Re: [Qemu-devel] [PATCH v2 09/10] monitor: add query-vnc2 command

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Dec 10, 2014 at 09:52:05AM -0700, Eric Blake wrote:
> On 12/10/2014 02:37 AM, Gerd Hoffmann wrote:
> > Add new query vnc qmp command, for the lack of better ideas just name it
> > "query-vnc2".  Changes over query-vnc:
> > 
> >  * It returns a list of vnc servers, so multiple vnc server instances
> >    are covered.
> >  * Each vnc server returns a list of server sockets.  Followup patch
> >    will use that to also report websockets.  In case we add support for
> >    multiple server sockets server sockets (to better support ipv4+ipv6
> >    dualstack) we can add them to the list too.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> 
> > +# @VncInfo2:
> > +#
> > +# Information about a vnc server
> > +#
> > +# @id: vnc server name.
> > +#
> > +# @server: A list of @VncBasincInfo describing all listening sockets.
> > +#          The list can be empty (in case the vnc server is disabled).
> > +#          It also may have multiple entries: normal + websocket,
> > +#          possibly also ipv4 + ipv6 in the future.
> > +#
> > +# @clients: A list of @VncClientInfo of all currently connected clients.
> > +#           The list can be empty, for obvious reasons.
> 
> Seems okay.
> 
> > +#
> > +# @auth: The current authentication type used by the server
> > +#        'none' if no authentication is being used
> > +#        'vnc' if VNC authentication is being used
> > +#        'vencrypt+plain' if VEncrypt is used with plain text authentication
> > +#        'vencrypt+tls+none' if VEncrypt is used with TLS and no authentication
> > +#        'vencrypt+tls+vnc' if VEncrypt is used with TLS and VNC authentication
> > +#        'vencrypt+tls+plain' if VEncrypt is used with TLS and plain text auth
> > +#        'vencrypt+x509+none' if VEncrypt is used with x509 and no auth
> > +#        'vencrypt+x509+vnc' if VEncrypt is used with x509 and VNC auth
> > +#        'vencrypt+x509+plain' if VEncrypt is used with x509 and plain text auth
> > +#        'vencrypt+tls+sasl' if VEncrypt is used with TLS and SASL auth
> > +#        'vencrypt+x509+sasl' if VEncrypt is used with x509 and SASL auth
> 
> This feels like an open-coded string that should instead be an array of
> enum values.  That is,
> 
> { 'enum': 'VncAuth', 'data', [ 'none', 'vnc', 'vencrypt', 'plain',
> 'tls', 'x509' ] }
> ... 'auth': ['VcnAuth']
> 
> might be friendlier to applications (having to post-parse the '+' is not
> friendly).

That's not a correct interpretation of the auth values - tls and x509 are
not separate auth codes. VNC has one set of primary auth codes really

  none, vnc, vencrypt

If using the vencrypt option there are a number of sub-auth codes

   tls-none, tls-vnc, tls-plain, tls-sasl x509-none, x509-vnc, x509-plain, x509-sasl

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|