[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v6 14/15] qapi: add conditions to VNC type/comma

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v6 14/15] qapi: add conditions to VNC type/commands/events on the schema
Date: Tue, 03 Jul 2018 17:04:04 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Marc-André Lureau <address@hidden> writes:

> Add #if defined(CONFIG_VNC) in generated code, and adjust the
> qmp/hmp code accordingly.
> query-qmp-schema no longer reports the command/events etc as
> available when disabled at compile.
> Commands made conditional:
> * query-vnc, query-vnc-servers, change-vnc-password
>   Before the patch, the commands for !CONFIG_VNC are stubs that fail
>   like this:
>     {"error": {"class": "GenericError",
>                "desc": "The feature 'vnc' is not enabled"}}
>   Afterwards, they fail like this:
>     {"error": {"class": "CommandNotFound",
>                "desc": "The command FOO has not been found"}}
>   I call that an improvement, because it lets clients distinguish
>   between command unavailable (class CommandNotFound) and command failed
>   (class GenericError).
> Events made conditional:
> HMP change:
> * info vnc
>   Will return "unknown command: 'info vnc'" when VNC is compiled
>   out (same as error for spice when --disable-spice)
> Occurrences of VNC (case insensitive) in the schema that aren't
> covered by this change:
> * add_client
>   Command has other uses, including "socket bases character devices".
>   These are unconditional as far as I can tell.
> * set_password, expire_password
>   In theory, these commands could be used for managing any service's
>   password.  In practice, they're used for VNC and SPICE services.
>   They're documented for "remote display session" / "remote display
>   server".
>   The service is selected by argument @protocol.  The code special-cases
>   protocol-specific argument checking, then calls a protocol-specific
>   function to do the work.  If it fails, the command fails with "Could
>   not set password".  It does when the service isn't compiled in (it's a
>   stub then).
>   We could make these commands conditional on the conjunction of all
>   services [currently: defined(CONFIG_VNC) || defined(CONFIG_SPICE)],
>   but I doubt it's worthwhile.
> * change
>   Command has other uses, namely changing media.
>   This patch inlines a stub; no functional change.
> Signed-off-by: Marc-André Lureau <address@hidden>
> Reviewed-by: Gerd Hoffmann <address@hidden>

Reviewed-by: Markus Armbruster <address@hidden>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]