Re: [PATCH v2 14/22] qga: conditionalize schema for commands requiring fstrim

[Konstantin Kostiuk <kkostiuk@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2966 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 filesystem trimming commands
> on POSIX platforms lacking required APIs.
>
> 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 | 13 -------------
>  qga/qapi-schema.json |  9 ++++++---
>  2 files changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 9207cb7a8f..d92fa0ec87 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1144,22 +1144,9 @@ error:
>
>  #endif /* HAVE_GETIFADDRS */
>
> -#if !defined(CONFIG_FSTRIM)
> -GuestFilesystemTrimResponse *
> -qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
> -{
> -    error_setg(errp, QERR_UNSUPPORTED);
> -    return NULL;
> -}
> -#endif
> -
>  /* add unsupported commands to the list of blocked RPCs */
>  GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
>  {
> -#if !defined(CONFIG_FSTRIM)
> -    blockedrpcs = g_list_append(blockedrpcs, g_strdup("guest-fstrim"));
> -#endif
> -
>      return blockedrpcs;
>  }
>
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 098fa7a08b..0f27375ea0 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -509,7 +509,8 @@
>  ##
>  { 'struct': 'GuestFilesystemTrimResult',
>    'data': {'path': 'str',
> -           '*trimmed': 'int', '*minimum': 'int', '*error': 'str'} }
> +           '*trimmed': 'int', '*minimum': 'int', '*error': 'str'},
> +  'if': { 'any': ['CONFIG_WIN32', 'CONFIG_FSTRIM'] } }
>
>  ##
>  # @GuestFilesystemTrimResponse:
> @@ -519,7 +520,8 @@
>  # Since: 2.4
>  ##
>  { 'struct': 'GuestFilesystemTrimResponse',
> -  'data': {'paths': ['GuestFilesystemTrimResult']} }
> +  'data': {'paths': ['GuestFilesystemTrimResult']},
> +  'if': { 'any': ['CONFIG_WIN32', 'CONFIG_FSTRIM'] } }
>
>  ##
>  # @guest-fstrim:
> @@ -541,7 +543,8 @@
>  ##
>  { 'command': 'guest-fstrim',
>    'data': { '*minimum': 'int' },
> -  'returns': 'GuestFilesystemTrimResponse' }
> +  'returns': 'GuestFilesystemTrimResponse',
> +  'if': { 'any': ['CONFIG_WIN32', 'CONFIG_FSTRIM'] } }
>
>  ##
>  # @guest-suspend-disk:
> --
> 2.45.1
>
>

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