[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 12/12] spice: introduce listen type none



Hi

On Wed, May 11, 2016 at 5:08 PM, Pavel Hrdina <phrdina redhat com> 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'.
>
> Signed-off-by: Pavel Hrdina <phrdina redhat com>
> ---
>  docs/formatdomain.html.in                          | 11 ++++
>  docs/schemas/domaincommon.rng                      |  5 ++
>  src/conf/domain_conf.c                             | 62 +++++++++++++++++-----
>  src/qemu/qemu_command.c                            | 11 ++--
>  .../qemuxml2argv-video-virtio-gpu-spice-gl.args    |  2 +-
>  .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml   |  4 +-
>  6 files changed, 72 insertions(+), 23 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 8f3e17f..35bbf3f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5375,6 +5375,17 @@ qemu-kvm -net nic,model=? /dev/null
>            attribute all <code>listen</code> elements are ignored.
>          </p>
>        </dd>
> +      <dt><code>none</code> <span class="since">since 1.3.5</span></dt>
> +      <dd>
> +        <p>
> +          This listen type doesn't have any other attribute. Libvirt supports
> +          passing a file descriptor through our APIs virDomainOpenGraphics() and
> +          virDomainOpenGraphicsFD(). No other listen types are allowed if this
> +          one is used and the graphics device doesn't listen anywhere. You need
> +          to use one of the two APIs to pass a FD to QEMU in order to connect to
> +          this graphics device. Supported only by <code>spice</code>.

I wonder if qemu would let you connect to the vnc server if started
with -vnc none.

> +        </p>
> +      </dd>
>      </dl>
>
>      <h4><a name="elementsVideo">Video devices</a></h4>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index e3dbcc6..c1a26a8 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3024,6 +3024,11 @@
>                </attribute>
>              </optional>
>            </group>
> +          <group>
> +            <attribute name="type">
> +              <value>none</value>
> +            </attribute>
> +          </group>
>          </choice>
>        </element>
>      </zeroOrMore>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 86b211c..99712ff 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10809,13 +10809,28 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>      }
>      def->type = typeVal;
>
> -    if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET &&
> -        graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> -        graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("listen type 'socket' is not available for "
> -                         "graphics type '%s'"), graphicsType);
> -        goto error;
> +    switch (def->type) {
> +    case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET:
> +        if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> +            graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("listen type 'socket' is not available for "
> +                             "graphics type '%s'"), graphicsType);
> +            goto error;
> +        }
> +        break;
> +    case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE:
> +        if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("listen type 'none' is not available for "
> +                             "graphics type '%s'"), graphicsType);
> +            goto error;
> +        }
> +        break;
> +    case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> +    case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> +    case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST:
> +        break;
>      }
>
>      if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) {
> @@ -10900,6 +10915,7 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def,
>      xmlNodePtr *listenNodes = NULL;
>      xmlNodePtr save = ctxt->node;
>      virDomainGraphicsListenDef newListen = {0};
> +    virDomainGraphicsListenDefPtr glisten = NULL;
>      char *socketPath = NULL;
>      int nListens;
>      int ret = -1;
> @@ -10953,6 +10969,19 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def,
>              goto error;
>      }
>
> +    /* 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 (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +        glisten = &def->listens[0];
> +
> +        if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS &&
> +            glisten->port == 0 && glisten->tlsPort == 0 && !glisten->autoport) {
> +            VIR_FREE(glisten->address);
> +            glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE;
> +        }
> +    }
> +
>      ret = 0;
>   error:
>      if (ret < 0)
> @@ -21429,10 +21458,8 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf,
>          return;
>
>      virBufferAddLit(buf, "<listen");
> -    if (def->type) {
> -        virBufferAsprintf(buf, " type='%s'",
> -                          virDomainGraphicsListenTypeToString(def->type));
> -    }
> +    virBufferAsprintf(buf, " type='%s'",
> +                      virDomainGraphicsListenTypeToString(def->type));
>
>      if (def->address &&
>          (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS ||
> @@ -21609,6 +21636,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>              break;
>
>          case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE:
> +            if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)
> +                virBufferAddStr(buf, " autoport='no'");
> +            break;
> +
>          case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET:
>          case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST:
>              break;
> @@ -21630,8 +21661,6 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>      }
>
>      for (i = 0; i < def->nListens; i++) {
> -        if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE)
> -            continue;
>          if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) {
>              if (def->listens[i].fromConfig)
>                  continue;
> @@ -21644,6 +21673,13 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>                  def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET &&
>                  !def->listens[i].autogenerated)
>                  continue;
> +
> +            /* The new listen type none is in the migratable XML represented as
> +             * port=0 and autoport=no because old libvirt support this
> +             * configuration for spice. */
> +            if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
> +                def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE)
> +                continue;
>          }
>          if (!children) {
>              virBufferAddLit(buf, ">\n");
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index b911076..64b383d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7635,6 +7635,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>          break;
>
>      case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE:
> +        /* QEMU requires either port or tls-port to be specified */
> +        virBufferAddLit(&opt, "port=0,");

Does it?
qemu-system-x86_64 -spice gl=on starts fine. It needs at least an
argument though (that looks like a bug)

> +        break;
>      case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST:
>          break;
>      }
> @@ -7755,13 +7758,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);
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
> index b80ad16..edecca1 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
> @@ -19,6 +19,6 @@ QEMU_AUDIO_DRV=spice \
>  -drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
>  id=drive-ide0-0-0,cache=none \
>  -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> --spice gl=on \
> +-spice port=0,gl=on \
>  -device virtio-vga,id=video0,virgl=on,bus=pci.0,addr=0x2 \
>  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml
> index fd260ea..9fb533a 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml
> @@ -29,8 +29,8 @@
>      <controller type='pci' index='0' model='pci-root'/>
>      <input type='mouse' bus='ps2'/>
>      <input type='keyboard' bus='ps2'/>
> -    <graphics type='spice' autoport='no'>
> -      <listen type='address' autoport='no'/>
> +    <graphics type='spice'>
> +      <listen type='none'/>
>        <gl enable='yes'/>
>      </graphics>
>      <video>
> --
> 2.8.2
>
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

Other than that, looks good to me

-- 
Marc-André Lureau


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]