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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2847 bytes --]

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).

> +#
> +# @display: #optional The display device the vnc server is linked to.
> +#
> +# Since: 2.3
> +##
> +{ 'type': 'VncInfo2',
> +  'data': { 'id'       : 'str',
> +            'server'   : ['VncBasicInfo'],
> +            'clients'  : ['VncClientInfo'],
> +            'auth'     : 'str',
> +            '*display' : 'str' } }

Looks reasonable, other than my concern about 'auth'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]