[libvirt] [PATCH v6 09/10] spice: introduce listen type none

[Ján Tomko]

On Wed, Jun 08, 2016 at 05:25:47PM +0200, Pavel Hrdina wrote:
> This new listen type is currently supported only by spice graphics.
> It's introduced to make it easier and clearer specify to not listen
> anywhere in order to start a guest with OpenGL support.
> 
> The old way to do this was set spice graphics autoport='no' and don't
> specify any ports.  The new way is to use <listen type='none'/>.  In
> order to be able to migrate to old libvirt the migratable XML will be
> generated without the listen element and with autoport='no'.  Also the
> old configuration will be automatically converted to the this listen
> type.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  docs/formatdomain.html.in                          | 11 ++++
>  docs/schemas/domaincommon.rng                      |  5 ++
>  src/conf/domain_conf.c                             | 77 ++++++++++++++++++----
>  src/qemu/qemu_command.c                            | 13 ++--
>  .../qemuxml2argv-video-virtio-gpu-spice-gl.args    |  2 +-
>  .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml   |  4 +-
>  6 files changed, 89 insertions(+), 23 deletions(-)
> 

ACK

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index eaa0770..d1539d7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3921,6 +3921,33 @@ virDomainDefPostParseTimer(virDomainDefPtr def)
>  }
>  
>  
> +static void
> +virDomainDefPostParseGraphics(virDomainDef *def)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->ngraphics; i++) {
> +        virDomainGraphicsDefPtr graphics = def->graphics[i];
> +
> +        /* If spice graphics is configured without ports and with autoport='no'
> +         * then we start qemu with Spice to not listen anywhere.  Let's convert
> +         * this configuration to the new listen type='none' which does the
> +         * same. */
> +        if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +            virDomainGraphicsListenDefPtr glisten = &graphics->listens[0];
> +
> +            if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS &&
> +                graphics->data.spice.port == 0 &&
> +                graphics->data.spice.tlsPort == 0 &&
> +                !graphics->data.spice.autoport) {
> +                VIR_FREE(glisten->address);
> +                glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE;

Fun!

> +            }
> +        }
> +    }
> +}
> +
> +

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1b46012..d03c6e0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7392,6 +7392,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>          break;
>  
>      case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE:
> +        /* QEMU requires either port or tls-port to be specified if there is no
> +         * other argument. Use a dummy port=0. */
> +        virBufferAddLit(&opt, "port=0,");
> +        hasInsecure = true;
> +        break;
>      case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST:
>          break;
>      }
> @@ -7539,13 +7544,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>      virBufferTrim(&opt, ",", -1);
>  
>      virCommandAddArg(cmd, "-spice");
> -    /* If we did not add any SPICE arguments, add a dummy 'port=0' one
> -     * as -spice alone is not allowed on QEMU command line
> -     */
> -    if (virBufferUse(&opt) == 0)
> -        virCommandAddArg(cmd, "port=0");
> -    else
> -        virCommandAddArgBuffer(cmd, &opt);
> +    virCommandAddArgBuffer(cmd, &opt);
>      if (graphics->data.spice.keymap)
>          virCommandAddArgList(cmd, "-k",
>                               graphics->data.spice.keymap, NULL);

This essentially reverts commit 6d28ef91:
    qemu: Don't add -spice port=0 when no port is specified

I think that change should be proposed in a separate patch which mentions
reverting commit 6d28ef91 with a justification.

Personally, I think not mentioning any ports or addrs is a better
representation of 'none' than port=0.

On the other hand, QEMU's help for -spice still mentions:
   at least one of {port, tls-port} is mandatory

Jan