[libvirt] [PATCH 1/3] qemu: Make all SPICE command-line args optional

[Christophe Fergeau]

The goal is to not add -spice port=0,addr=127.0.0.1 to QEMU command line
when no SPICE port is specified in libvirt XML.
Before this change, we could rely on port or tlsPort to always be
present, so subsequent args could be unconditionally appended with a
leading ','. Now that it's no longer the case, we need to always check
whether the arg we are about to append is the first one or not.
---
 src/qemu/qemu_command.c | 98 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 73 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 32d32b1..84db056 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7039,13 +7039,16 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
                              " but TLS is disabled in qemu.conf"));
             goto error;
         }
-        if (port > 0)
+
+        if (virBufferUse(&opt) != 0)
             virBufferAddChar(&opt, ',');
         virBufferAsprintf(&opt, "tls-port=%u", tlsPort);
     }
 
     if (cfg->spiceSASL) {
-        virBufferAddLit(&opt, ",sasl");
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAddLit(&opt, "sasl");
 
         if (cfg->spiceSASLdir)
             virCommandAddEnvPair(cmd, "SASL_CONF_PATH",
@@ -7084,18 +7087,25 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
 
     if (!listenAddr)
         listenAddr = cfg->spiceListen;
-    if (listenAddr)
-        virBufferAsprintf(&opt, ",addr=%s", listenAddr);
+    if (listenAddr) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAsprintf(&opt, "addr=%s", listenAddr);
+    }
 
     VIR_FREE(netAddr);
 
     if (graphics->data.spice.mousemode) {
         switch (graphics->data.spice.mousemode) {
         case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
-            virBufferAddLit(&opt, ",agent-mouse=off");
+            if (virBufferUse(&opt) != 0)
+                virBufferAddChar(&opt, ',');
+            virBufferAddLit(&opt, "agent-mouse=off");
             break;
         case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
-            virBufferAddLit(&opt, ",agent-mouse=on");
+            if (virBufferUse(&opt) != 0)
+                virBufferAddChar(&opt, ',');
+            virBufferAddLit(&opt, "agent-mouse=on");
             break;
         default:
             break;
@@ -7106,18 +7116,25 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
      * making it visible on CLI, so there's no use of password=XXX
      * in this bit of the code */
     if (!graphics->data.spice.auth.passwd &&
-        !cfg->spicePassword)
-        virBufferAddLit(&opt, ",disable-ticketing");
+        !cfg->spicePassword) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAddLit(&opt, "disable-ticketing");
+    }
 
     if (tlsPort > 0)
         virBufferAsprintf(&opt, ",x509-dir=%s", cfg->spiceTLSx509certdir);
 
     switch (defaultMode) {
     case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
-        virBufferAddLit(&opt, ",tls-channel=default");
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAddLit(&opt, "tls-channel=default");
         break;
     case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
-        virBufferAddLit(&opt, ",plaintext-channel=default");
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAddLit(&opt, "plaintext-channel=default");
         break;
     case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY:
         /* nothing */
@@ -7175,30 +7192,50 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
         }
     }
 
-    if (graphics->data.spice.image)
-        virBufferAsprintf(&opt, ",image-compression=%s",
+    if (graphics->data.spice.image) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAsprintf(&opt, "image-compression=%s",
                           virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image));
-    if (graphics->data.spice.jpeg)
-        virBufferAsprintf(&opt, ",jpeg-wan-compression=%s",
+    }
+    if (graphics->data.spice.jpeg) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAsprintf(&opt, "jpeg-wan-compression=%s",
                           virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg));
-    if (graphics->data.spice.zlib)
-        virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s",
+    }
+    if (graphics->data.spice.zlib) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAsprintf(&opt, "zlib-glz-wan-compression=%s",
                           virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib));
-    if (graphics->data.spice.playback)
-        virBufferAsprintf(&opt, ",playback-compression=%s",
+    }
+    if (graphics->data.spice.playback) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAsprintf(&opt, "playback-compression=%s",
                           virTristateSwitchTypeToString(graphics->data.spice.playback));
-    if (graphics->data.spice.streaming)
-        virBufferAsprintf(&opt, ",streaming-video=%s",
+    }
+    if (graphics->data.spice.streaming) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAsprintf(&opt, "streaming-video=%s",
                           virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming));
-    if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO)
-        virBufferAddLit(&opt, ",disable-copy-paste");
+    }
+    if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
+        virBufferAddLit(&opt, "disable-copy-paste");
+    }
     if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO) {
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) {
            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                           _("This QEMU can't disable file transfers through spice"));
             goto error;
         } else {
-            virBufferAddLit(&opt, ",disable-agent-file-xfer");
+            if (virBufferUse(&opt) != 0)
+                virBufferAddChar(&opt, ',');
+            virBufferAddLit(&opt, "disable-agent-file-xfer");
         }
     }
 
@@ -7209,20 +7246,31 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
             goto error;
         }
 
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
         /* spice.gl is a TristateBool, but qemu expects on/off: use
          * TristateSwitch helper */
-        virBufferAsprintf(&opt, ",gl=%s",
+        virBufferAsprintf(&opt, "gl=%s",
                           virTristateSwitchTypeToString(graphics->data.spice.gl));
     }
 
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION)) {
+        if (virBufferUse(&opt) != 0)
+            virBufferAddChar(&opt, ',');
         /* If qemu supports seamless migration turn it
          * unconditionally on. If migration destination
          * doesn't support it, it fallbacks to previous
          * migration algorithm silently. */
-        virBufferAddLit(&opt, ",seamless-migration=on");
+        virBufferAddLit(&opt, "seamless-migration=on");
     }
 
+    /* If we did not add any SPICE arguments, add a dummy 'port=0' one
+     * as -spice alone is not allowed on QEMU command line and will be
+     * ignored by libvirt
+     */
+    if (virBufferUse(&opt) == 0)
+      virBufferAddLit(&opt, "port=0");
+
     virCommandAddArg(cmd, "-spice");
     virCommandAddArgBuffer(cmd, &opt);
     if (graphics->data.spice.keymap)
-- 
2.5.0