[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



On Wed, May 11, 2016 at 08:14:43PM +0200, Marc-André Lureau wrote:
> 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.

That's a good question.  I didn't find this anywhere, but I'll try it.

> > +        </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)

qemu doesn't allow to start if there is spice without arguments.  So in case
that there isn't anything else to add, like the gl=on we need to add port=0 in
order to start qemu with spice.  This actually probably doesn't happen in the
real world as libvirt always adds some arguments for spice.  The code I'm
removing few lines below was introduce to start a guest with spice that doesn't
listen for any connection.  After this patch the only case where we could end
up with spice without any arguments is the <listen type='none'>.  For
consistency I would rather put port=0 for <listen type='none'> even if there are
other arguments.

Pavel


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