Re: [PATCH v2 12/22] qga: conditionalize schema for commands only supported on Windows

[Konstantin Kostiuk <kkostiuk@redhat.com>]

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

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Thu, Jun 13, 2024 at 6:44 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> 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 non-Windows.
>
> The command will be rejected at QMP dispatch time instead,
> avoiding reimplementing rejection by blocking the stub commands.
> This changes the error message for affected commands from
>
>     {"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
>
> to
>
>     {"class": "CommandNotFound", "desc": "The command FOO has not been
> found"}
>
> 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-posix.c |  9 ---------
>  qga/qapi-schema.json | 15 ++++++++++-----
>  2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 838dc3cf98..b7f96aa005 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1207,8 +1207,6 @@ GList *ga_command_init_blockedrpcs(GList
> *blockedrpcs)
>      blockedrpcs = g_list_append(blockedrpcs, g_strdup("guest-fstrim"));
>  #endif
>
> -    blockedrpcs = g_list_append(blockedrpcs,
> g_strdup("guest-get-devices"));
> -
>      return blockedrpcs;
>  }
>
> @@ -1419,13 +1417,6 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
>      return info;
>  }
>
> -GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> -{
> -    error_setg(errp, QERR_UNSUPPORTED);
> -
> -    return NULL;
> -}
> -
>  #ifndef HOST_NAME_MAX
>  # ifdef _POSIX_HOST_NAME_MAX
>  #  define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 700c5baa5a..2704b814ab 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1527,7 +1527,8 @@
>  # @pci: PCI device
>  ##
>  { 'enum': 'GuestDeviceType',
> -  'data': [ 'pci' ] }
> +  'data': [ 'pci' ],
> +  'if': 'CONFIG_WIN32' }
>
>  ##
>  # @GuestDeviceIdPCI:
> @@ -1539,7 +1540,8 @@
>  # Since: 5.2
>  ##
>  { 'struct': 'GuestDeviceIdPCI',
> -  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' },
> +  'if': 'CONFIG_WIN32' }
>
>  ##
>  # @GuestDeviceId:
> @@ -1553,7 +1555,8 @@
>  { 'union': 'GuestDeviceId',
>    'base': { 'type': 'GuestDeviceType' },
>    'discriminator': 'type',
> -  'data': { 'pci': 'GuestDeviceIdPCI' } }
> +  'data': { 'pci': 'GuestDeviceIdPCI' },
> +  'if': 'CONFIG_WIN32' }
>
>  ##
>  # @GuestDeviceInfo:
> @@ -1574,7 +1577,8 @@
>        '*driver-date': 'int',
>        '*driver-version': 'str',
>        '*id': 'GuestDeviceId'
> -  } }
> +  },
> +  'if': 'CONFIG_WIN32' }
>
>  ##
>  # @guest-get-devices:
> @@ -1586,7 +1590,8 @@
>  # Since: 5.2
>  ##
>  { 'command': 'guest-get-devices',
> -  'returns': ['GuestDeviceInfo'] }
> +  'returns': ['GuestDeviceInfo'],
> +  'if': 'CONFIG_WIN32' }
>
>  ##
>  # @GuestAuthorizedKeys:
> --
> 2.45.1
>
>

[-- Attachment #2: Type: text/html, Size: 4469 bytes --]