Hey, After this patch series, the QEMU command line may not contain port nor tls-port if they both were set to '0'. However, QEMU versions older than 2.3.0 will error out because they don't have this commit: http://git.qemu.org/?p=qemu.git;a=commitdiff;h=cf7856adefebe86e0 I assume we want to keep supporting older QEMU binaries, and that this needs to be fixed on libvirt side? Christophe On Wed, Mar 16, 2016 at 05:45:03PM +0100, Christophe Fergeau wrote:
The end goal is to avoid adding -spice port=0,addr=127.0.0.1 to QEMU command line when no SPICE port is specified in libvirt XML.
Currently, the code relies on port=xx to always be present, so subsequent args can be unconditionally appended with a leading ','. Since port=0 will no longer be added in a subsequent commit, we append a ',' to every arg instead of prepending, and remove the last one before adding it to the arg list. --- src/qemu/qemu_command.c | 68 +++++++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 32d32b1..cd20a16 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7030,7 +7030,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, }
if (port > 0 || tlsPort <= 0) - virBufferAsprintf(&opt, "port=%u", port); + virBufferAsprintf(&opt, "port=%u,", port);
if (tlsPort > 0) { if (!cfg->spiceTLS) { @@ -7039,13 +7039,12 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, " but TLS is disabled in qemu.conf")); goto error; } - if (port > 0) - virBufferAddChar(&opt, ','); - virBufferAsprintf(&opt, "tls-port=%u", tlsPort); + + virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); }
if (cfg->spiceSASL) { - virBufferAddLit(&opt, ",sasl"); + virBufferAddLit(&opt, "sasl,");
if (cfg->spiceSASLdir) virCommandAddEnvPair(cmd, "SASL_CONF_PATH", @@ -7085,17 +7084,17 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, if (!listenAddr) listenAddr = cfg->spiceListen; if (listenAddr) - virBufferAsprintf(&opt, ",addr=%s", listenAddr); + 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"); + virBufferAddLit(&opt, "agent-mouse=off,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: - virBufferAddLit(&opt, ",agent-mouse=on"); + virBufferAddLit(&opt, "agent-mouse=on,"); break; default: break; @@ -7106,18 +7105,19 @@ 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) { + virBufferAddLit(&opt, "disable-ticketing,"); + }
if (tlsPort > 0) - virBufferAsprintf(&opt, ",x509-dir=%s", cfg->spiceTLSx509certdir); + virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir);
switch (defaultMode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAddLit(&opt, ",tls-channel=default"); + virBufferAddLit(&opt, "tls-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAddLit(&opt, ",plaintext-channel=default"); + virBufferAddLit(&opt, "plaintext-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: /* nothing */ @@ -7133,7 +7133,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, "but TLS port is not provided")); goto error; } - virBufferAsprintf(&opt, ",tls-channel=%s", + virBufferAsprintf(&opt, "tls-channel=%s,", virDomainGraphicsSpiceChannelNameTypeToString(i)); break;
@@ -7144,7 +7144,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, "configuration, but plain port is not provided")); goto error; } - virBufferAsprintf(&opt, ",plaintext-channel=%s", + virBufferAsprintf(&opt, "plaintext-channel=%s,", virDomainGraphicsSpiceChannelNameTypeToString(i)); break;
@@ -7175,30 +7175,36 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, } }
- if (graphics->data.spice.image) - virBufferAsprintf(&opt, ",image-compression=%s", + if (graphics->data.spice.image) { + 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) { + 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) { + 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) { + 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) { + 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) { + 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"); + virBufferAddLit(&opt, "disable-agent-file-xfer,"); } }
@@ -7211,7 +7217,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
/* 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)); }
@@ -7220,9 +7226,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, * 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,"); }
+ virBufferTrim(&opt, ",", -1); + virCommandAddArg(cmd, "-spice"); virCommandAddArgBuffer(cmd, &opt); if (graphics->data.spice.keymap) -- 2.5.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list