Re: [libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line

2013-04-25 Thread Christophe Fergeau
On Tue, Apr 23, 2013 at 03:46:09PM +0200, Peter Krempa wrote:
 Decrease size of qemuBuildGraphicsCommandLine() by splitting out
 spice-related code into qemuBuildGraphicsVNCCommandLine().

I guess you mean VNC-related code here.

Christophe

 
 This patch also fixes 2 possible memory leaks on error path in the code
 that was split-out. The buffer containing the already generated options
 and a listen address string could be leaked.
 
 Also break a few very long lines and reflow code that fits now.
 ---
  src/qemu/qemu_command.c | 244 
 +---
  1 file changed, 125 insertions(+), 119 deletions(-)
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index d6d782c..66b2ba8 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -5419,6 +5419,130 @@ cleanup:
 
 
  static int
 +qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
 +virCommandPtr cmd,
 +virDomainDefPtr def,
 +virQEMUCapsPtr qemuCaps,
 +virDomainGraphicsDefPtr graphics)
 +{
 +virBuffer opt = VIR_BUFFER_INITIALIZER;
 +const char *listenNetwork;
 +const char *listenAddr = NULL;
 +char *netAddr = NULL;
 +bool escapeAddr;
 +int ret;
 +
 +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(vnc graphics are not supported with this QEMU));
 +goto error;
 +}
 +
 +if (graphics-data.vnc.socket || cfg-vncAutoUnixSocket) {
 +if (!graphics-data.vnc.socket 
 +virAsprintf(graphics-data.vnc.socket,
 +%s/%s.vnc, cfg-libDir, def-name) == -1)
 +goto no_memory;
 +
 +virBufferAsprintf(opt, unix:%s, graphics-data.vnc.socket);
 +
 +} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
 +switch (virDomainGraphicsListenGetType(graphics, 0)) {
 +case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
 +listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
 +break;
 +
 +case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
 +listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
 +if (!listenNetwork)
 +break;
 +ret = networkGetNetworkAddress(listenNetwork, netAddr);
 +if (ret = -2) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   %s, _(network-based listen not possible, 
 +   network driver not present));
 +goto error;
 +}
 +if (ret  0) {
 +virReportError(VIR_ERR_XML_ERROR,
 +   _(listen network '%s' had no usable 
 address),
 +   listenNetwork);
 +goto error;
 +}
 +listenAddr = netAddr;
 +/* store the address we found in the graphics element so it 
 will
 + * show up in status. */
 +if (virDomainGraphicsListenSetAddress(graphics, 0,
 +  listenAddr, -1, false)  0)
 +   goto error;
 +break;
 +}
 +
 +if (!listenAddr)
 +listenAddr = cfg-vncListen;
 +
 +escapeAddr = strchr(listenAddr, ':') != NULL;
 +if (escapeAddr)
 +virBufferAsprintf(opt, [%s], listenAddr);
 +else
 +virBufferAdd(opt, listenAddr, -1);
 +virBufferAsprintf(opt, :%d,
 +  graphics-data.vnc.port - 5900);
 +
 +VIR_FREE(netAddr);
 +} else {
 +virBufferAsprintf(opt, %d,
 +  graphics-data.vnc.port - 5900);
 +}
 +
 +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
 +if (graphics-data.vnc.auth.passwd || cfg-vncPassword)
 +virBufferAddLit(opt, ,password);
 +
 +if (cfg-vncTLS) {
 +virBufferAddLit(opt, ,tls);
 +if (cfg-vncTLSx509verify)
 +virBufferAsprintf(opt, ,x509verify=%s, 
 cfg-vncTLSx509certdir);
 +else
 +virBufferAsprintf(opt, ,x509=%s, cfg-vncTLSx509certdir);
 +}
 +
 +if (cfg-vncSASL) {
 +virBufferAddLit(opt, ,sasl);
 +
 +if (cfg-vncSASLdir)
 +virCommandAddEnvPair(cmd, SASL_CONF_DIR, cfg-vncSASLdir);
 +
 +/* TODO: Support ACLs later */
 +}
 +}
 +
 +virCommandAddArg(cmd, -vnc);
 +virCommandAddArgBuffer(cmd, opt);
 +if (graphics-data.vnc.keymap)
 +virCommandAddArgList(cmd, -k, graphics-data.vnc.keymap, NULL);
 +
 +/* Unless user requested it, set the audio backend to none, to
 + * prevent it opening the host OS audio devices, since that causes
 + * security issues and might not work when using VNC.
 + */
 

Re: [libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line

2013-04-25 Thread Peter Krempa

On 04/25/13 12:34, Christophe Fergeau wrote:

On Tue, Apr 23, 2013 at 03:46:09PM +0200, Peter Krempa wrote:

Decrease size of qemuBuildGraphicsCommandLine() by splitting out
spice-related code into qemuBuildGraphicsVNCCommandLine().


I guess you mean VNC-related code here.


Ah, yes. I borrowed the commit message from the patch moving the spice 
code and forgot to update that. Unfortunately I already pushed this patch.




Christophe



This patch also fixes 2 possible memory leaks on error path in the code
that was split-out. The buffer containing the already generated options
and a listen address string could be leaked.

Also break a few very long lines and reflow code that fits now.
---
  src/qemu/qemu_command.c | 244 +---
  1 file changed, 125 insertions(+), 119 deletions(-)



Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line

2013-04-23 Thread Peter Krempa
Decrease size of qemuBuildGraphicsCommandLine() by splitting out
spice-related code into qemuBuildGraphicsVNCCommandLine().

This patch also fixes 2 possible memory leaks on error path in the code
that was split-out. The buffer containing the already generated options
and a listen address string could be leaked.

Also break a few very long lines and reflow code that fits now.
---
 src/qemu/qemu_command.c | 244 +---
 1 file changed, 125 insertions(+), 119 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6d782c..66b2ba8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5419,6 +5419,130 @@ cleanup:


 static int
+qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
+virCommandPtr cmd,
+virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps,
+virDomainGraphicsDefPtr graphics)
+{
+virBuffer opt = VIR_BUFFER_INITIALIZER;
+const char *listenNetwork;
+const char *listenAddr = NULL;
+char *netAddr = NULL;
+bool escapeAddr;
+int ret;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(vnc graphics are not supported with this QEMU));
+goto error;
+}
+
+if (graphics-data.vnc.socket || cfg-vncAutoUnixSocket) {
+if (!graphics-data.vnc.socket 
+virAsprintf(graphics-data.vnc.socket,
+%s/%s.vnc, cfg-libDir, def-name) == -1)
+goto no_memory;
+
+virBufferAsprintf(opt, unix:%s, graphics-data.vnc.socket);
+
+} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
+switch (virDomainGraphicsListenGetType(graphics, 0)) {
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
+listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
+break;
+
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
+listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
+if (!listenNetwork)
+break;
+ret = networkGetNetworkAddress(listenNetwork, netAddr);
+if (ret = -2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(network-based listen not possible, 
+   network driver not present));
+goto error;
+}
+if (ret  0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(listen network '%s' had no usable address),
+   listenNetwork);
+goto error;
+}
+listenAddr = netAddr;
+/* store the address we found in the graphics element so it will
+ * show up in status. */
+if (virDomainGraphicsListenSetAddress(graphics, 0,
+  listenAddr, -1, false)  0)
+   goto error;
+break;
+}
+
+if (!listenAddr)
+listenAddr = cfg-vncListen;
+
+escapeAddr = strchr(listenAddr, ':') != NULL;
+if (escapeAddr)
+virBufferAsprintf(opt, [%s], listenAddr);
+else
+virBufferAdd(opt, listenAddr, -1);
+virBufferAsprintf(opt, :%d,
+  graphics-data.vnc.port - 5900);
+
+VIR_FREE(netAddr);
+} else {
+virBufferAsprintf(opt, %d,
+  graphics-data.vnc.port - 5900);
+}
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
+if (graphics-data.vnc.auth.passwd || cfg-vncPassword)
+virBufferAddLit(opt, ,password);
+
+if (cfg-vncTLS) {
+virBufferAddLit(opt, ,tls);
+if (cfg-vncTLSx509verify)
+virBufferAsprintf(opt, ,x509verify=%s, 
cfg-vncTLSx509certdir);
+else
+virBufferAsprintf(opt, ,x509=%s, cfg-vncTLSx509certdir);
+}
+
+if (cfg-vncSASL) {
+virBufferAddLit(opt, ,sasl);
+
+if (cfg-vncSASLdir)
+virCommandAddEnvPair(cmd, SASL_CONF_DIR, cfg-vncSASLdir);
+
+/* TODO: Support ACLs later */
+}
+}
+
+virCommandAddArg(cmd, -vnc);
+virCommandAddArgBuffer(cmd, opt);
+if (graphics-data.vnc.keymap)
+virCommandAddArgList(cmd, -k, graphics-data.vnc.keymap, NULL);
+
+/* Unless user requested it, set the audio backend to none, to
+ * prevent it opening the host OS audio devices, since that causes
+ * security issues and might not work when using VNC.
+ */
+if (cfg-vncAllowHostAudio)
+virCommandAddEnvPass(cmd, QEMU_AUDIO_DRV);
+else
+virCommandAddEnvString(cmd, QEMU_AUDIO_DRV=none);
+
+return 0;
+
+no_memory:
+virReportOOMError();
+error:
+VIR_FREE(netAddr);
+

Re: [libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line

2013-04-23 Thread Daniel P. Berrange
On Tue, Apr 23, 2013 at 03:46:09PM +0200, Peter Krempa wrote:
 Decrease size of qemuBuildGraphicsCommandLine() by splitting out
 spice-related code into qemuBuildGraphicsVNCCommandLine().
 
 This patch also fixes 2 possible memory leaks on error path in the code
 that was split-out. The buffer containing the already generated options
 and a listen address string could be leaked.
 
 Also break a few very long lines and reflow code that fits now.
 ---
  src/qemu/qemu_command.c | 244 
 +---
  1 file changed, 125 insertions(+), 119 deletions(-)

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list