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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list