qemu: Split out code to generate SPICE command line

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

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.

src/qemu/qemu_command.c

@@ -5498,166 +5498,13 @@ cleanup:
     return ret;
 }
 
+
 static int
-qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
+qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
                                   virCommandPtr cmd,
-                             virDomainDefPtr def,
                                   virQEMUCapsPtr qemuCaps,
                                   virDomainGraphicsDefPtr graphics)
 {
-    int i;
-
-    if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
-        virBuffer opt = VIR_BUFFER_INITIALIZER;
-
-        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)) {
-            const char *listenNetwork;
-            const char *listenAddr = NULL;
-            char *netAddr = NULL;
-            bool escapeAddr;
-            int ret;
-
-            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");
-        }
-    } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
-        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_0_10) &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("sdl not supported by '%s'"),
-                           def->emulator);
-            goto error;
-        }
-
-        if (graphics->data.sdl.xauth)
-            virCommandAddEnvPair(cmd, "XAUTHORITY",
-                                 graphics->data.sdl.xauth);
-        if (graphics->data.sdl.display)
-            virCommandAddEnvPair(cmd, "DISPLAY",
-                                 graphics->data.sdl.display);
-        if (graphics->data.sdl.fullscreen)
-            virCommandAddArg(cmd, "-full-screen");
-
-        /* If using SDL for video, then we should just let it
-         * use QEMU's host audio drivers, possibly SDL too
-         * User can set these two before starting libvirtd
-         */
-        virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
-        virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
-
-        /* New QEMU has this flag to let us explicitly ask for
-         * SDL graphics. This is better than relying on the
-         * default, since the default changes :-( */
-        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL))
-            virCommandAddArg(cmd, "-sdl");
-
-    } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
     virBuffer opt = VIR_BUFFER_INITIALIZER;
     const char *listenNetwork;
     const char *listenAddr = NULL;
@@ -5666,6 +5513,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
     int defaultMode = graphics->data.spice.defaultMode;
     int port = graphics->data.spice.port;
     int tlsPort = graphics->data.spice.tlsPort;
+    int i;
 
     if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -5726,9 +5574,8 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
 
     VIR_FREE(netAddr);
 
-        int mm = graphics->data.spice.mousemode;
-        if (mm) {
-            switch (mm) {
+    if (graphics->data.spice.mousemode) {
+        switch (graphics->data.spice.mousemode) {
         case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
             virBufferAsprintf(&opt, ",agent-mouse=off");
             break;
@@ -5769,7 +5616,8 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
         case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
             if (!cfg->spiceTLS) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                                   _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf"));
+                               _("spice secure channels set in XML configuration, "
+                                 "but TLS is disabled in qemu.conf"));
                 goto error;
             }
             virBufferAsprintf(&opt, ",tls-channel=%s",
@@ -5817,6 +5665,174 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
      */
     virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
 
+    return 0;
+
+error:
+    VIR_FREE(netAddr);
+    virBufferFreeAndReset(&opt);
+    return -1;
+}
+
+static int
+qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
+                             virCommandPtr cmd,
+                             virDomainDefPtr def,
+                             virQEMUCapsPtr qemuCaps,
+                             virDomainGraphicsDefPtr graphics)
+{
+    if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
+        virBuffer opt = VIR_BUFFER_INITIALIZER;
+
+        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)) {
+            const char *listenNetwork;
+            const char *listenAddr = NULL;
+            char *netAddr = NULL;
+            bool escapeAddr;
+            int ret;
+
+            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");
+        }
+    } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
+        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_0_10) &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("sdl not supported by '%s'"),
+                           def->emulator);
+            goto error;
+        }
+
+        if (graphics->data.sdl.xauth)
+            virCommandAddEnvPair(cmd, "XAUTHORITY",
+                                 graphics->data.sdl.xauth);
+        if (graphics->data.sdl.display)
+            virCommandAddEnvPair(cmd, "DISPLAY",
+                                 graphics->data.sdl.display);
+        if (graphics->data.sdl.fullscreen)
+            virCommandAddArg(cmd, "-full-screen");
+
+        /* If using SDL for video, then we should just let it
+         * use QEMU's host audio drivers, possibly SDL too
+         * User can set these two before starting libvirtd
+         */
+        virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
+        virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
+
+        /* New QEMU has this flag to let us explicitly ask for
+         * SDL graphics. This is better than relying on the
+         * default, since the default changes :-( */
+        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL))
+            virCommandAddArg(cmd, "-sdl");
+
+    } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+        if (qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics) < 0)
+            goto error;
     } else {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("unsupported graphics type '%s'"),