Re: [PATCH 08/20] qga: conditionalize schema for commands unsupported on Windows

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Rather than creating stubs for every command that just return
> QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
> fully exclude generation of the commands on Windows.
>
> The command will be rejected at QMP dispatch time instead,
> avoiding reimplementing rejection by blocking the stub commands.
>
> This fixes inconsistency where some commands are implemented
> as stubs, yet not added to the blockedrpc list.
>
> This has the additional benefit that the QGA protocol reference
> now documents what conditions enable use of the command.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qga/commands-win32.c | 56 +-------------------------------------------
>  qga/qapi-schema.json | 45 +++++++++++++++++++++++------------
>  2 files changed, 31 insertions(+), 70 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 9fe670d5b4..2533e4c748 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c

[...]

>  /* add unsupported commands to the list of blocked RPCs */
>  GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
>  {
> -    const char *list_unsupported[] = {
> -        "guest-suspend-hybrid",
> -        "guest-set-vcpus",
> -        "guest-get-memory-blocks", "guest-set-memory-blocks",
> -        "guest-get-memory-block-info",
> -        NULL};
> -    char **p = (char **)list_unsupported;
> -
> -    while (*p) {
> -        blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
> -    }
> -
>      if (!vss_init(true)) {
>          g_debug("vss_init failed, vss commands are going to be disabled");
>          const char *list[] = {
>              "guest-get-fsinfo", "guest-fsfreeze-status",
>              "guest-fsfreeze-freeze", "guest-fsfreeze-thaw", NULL};
> -        p = (char **)list;
> +        char **p = (char **)list;
>  
>          while (*p) {
>              blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
           }
       }

       return blockedrpcs;
   }

Four commands get disabled when vss_init() fails, i.e. when qga-vss.dll
can't be loaded and initialized.

Three of the four commands do this first:

        if (!vss_initialized()) {
            error_setg(errp, QERR_UNSUPPORTED);
            return 0;
        }

The execption is qmp_guest_get_fsinfo().

vss_initialized() returns true between successful vss_init() and
vss_deinit().

Aside: we call vss_init() in three places.  Two of them init, call
something, then deinit.  Weird.  Moving on.

If these commands are meant to be only available when the DLL is, then
having them check vss_initialized() is useless.

Conversely, if the check isn't useless, then the "make it available
only" business is.

Opportunity for further cleanup?

[...]