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

On Tue, Mar 15, 2016 at 10:20:48AM +0100, Christophe Fergeau wrote: > 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. Misleading, even after the series port=0 is generated.

> 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); Instead of all these conditions, we can convert all the arguments to add a trailing comma and remove the last one by calling virBufferTrim(&opt, ",", -1); at the end.

> } > > + /* 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"); > + I would rather add this change to the commit that disables the first port=0.

Also, there is no need to go through the buffer, the string can be added directly to the virCommand.