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

[Ján Tomko]

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.

Jan

>      virCommandAddArg(cmd, "-spice");
>      virCommandAddArgBuffer(cmd, &opt);
>      if (graphics->data.spice.keymap)
> -- 
> 2.5.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list