Reviewed-by: Konstantin Kostiuk <kkost...@redhat.com>
On Thu, Jun 13, 2024 at 6:44 PM Daniel P. Berrangé <berra...@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 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 also fixes an accidental inconsistency where some commands
> (guest-get-diskstats & guest-get-cpustats) are implemented as
> stubs, yet not added to the blockedrpc list. Those change their
> error message from
>
> {"class": "GenericError, "desc": "this feature or command is not
> currently supported"}
>
> to
>
> {"class": "CommandNotFound", "desc": "The command FOO has not been
> found"}
>
> The final additional benefit is that the QGA protocol reference
> now documents what conditions enable use of the command.
>
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
> qga/commands-posix.c | 2 +-
> qga/commands-win32.c | 56 +-------------------------------------------
> qga/qapi-schema.json | 45 +++++++++++++++++++++++------------
> 3 files changed, 32 insertions(+), 71 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 2a3bef7445..0dd8555867 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1280,7 +1280,7 @@ GList *ga_command_init_blockedrpcs(GList
> *blockedrpcs)
> "guest-get-memory-blocks", "guest-set-memory-blocks",
> "guest-get-memory-block-info",
> NULL};
> - char **p = (char **)list;
> + const char **p = list;
>
> while (*p) {
> blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
> 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
> @@ -1494,11 +1494,6 @@ out:
> }
> }
>
> -void qmp_guest_suspend_hybrid(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> -}
> -
> static IP_ADAPTER_ADDRESSES *guest_get_adapters_addresses(Error **errp)
> {
> IP_ADAPTER_ADDRESSES *adptr_addrs = NULL;
> @@ -1862,12 +1857,6 @@ GuestLogicalProcessorList
> *qmp_guest_get_vcpus(Error **errp)
> return NULL;
> }
>
> -int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error
> **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return -1;
> -}
> -
> static gchar *
> get_net_error_message(gint error)
> {
> @@ -1969,46 +1958,15 @@ done:
> g_free(rawpasswddata);
> }
>
> -GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> -
> -GuestMemoryBlockResponseList *
> -qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> -
> -GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> -
> /* 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++));
> @@ -2505,15 +2463,3 @@ char *qga_get_host_name(Error **errp)
>
> return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
> }
> -
> -GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> -
> -GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index b3de1fb6b3..b91456e9ad 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -636,7 +636,8 @@
> #
> # Since: 1.1
> ##
> -{ 'command': 'guest-suspend-hybrid', 'success-response': false }
> +{ 'command': 'guest-suspend-hybrid', 'success-response': false,
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestIpAddressType:
> @@ -806,7 +807,8 @@
> ##
> { 'command': 'guest-set-vcpus',
> 'data': {'vcpus': ['GuestLogicalProcessor'] },
> - 'returns': 'int' }
> + 'returns': 'int',
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestDiskBusType:
> @@ -1099,7 +1101,8 @@
> { 'struct': 'GuestMemoryBlock',
> 'data': {'phys-index': 'uint64',
> 'online': 'bool',
> - '*can-offline': 'bool'} }
> + '*can-offline': 'bool'},
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @guest-get-memory-blocks:
> @@ -1115,7 +1118,8 @@
> # Since: 2.3
> ##
> { 'command': 'guest-get-memory-blocks',
> - 'returns': ['GuestMemoryBlock'] }
> + 'returns': ['GuestMemoryBlock'],
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestMemoryBlockResponseType:
> @@ -1138,7 +1142,8 @@
> ##
> { 'enum': 'GuestMemoryBlockResponseType',
> 'data': ['success', 'not-found', 'operation-not-supported',
> - 'operation-failed'] }
> + 'operation-failed'],
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestMemoryBlockResponse:
> @@ -1156,7 +1161,8 @@
> { 'struct': 'GuestMemoryBlockResponse',
> 'data': { 'phys-index': 'uint64',
> 'response': 'GuestMemoryBlockResponseType',
> - '*error-code': 'int' }}
> + '*error-code': 'int' },
> + 'if': 'CONFIG_POSIX'}
>
> ##
> # @guest-set-memory-blocks:
> @@ -1187,7 +1193,8 @@
> ##
> { 'command': 'guest-set-memory-blocks',
> 'data': {'mem-blks': ['GuestMemoryBlock'] },
> - 'returns': ['GuestMemoryBlockResponse'] }
> + 'returns': ['GuestMemoryBlockResponse'],
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestMemoryBlockInfo:
> @@ -1199,7 +1206,8 @@
> # Since: 2.3
> ##
> { 'struct': 'GuestMemoryBlockInfo',
> - 'data': {'size': 'uint64'} }
> + 'data': {'size': 'uint64'},
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @guest-get-memory-block-info:
> @@ -1211,7 +1219,8 @@
> # Since: 2.3
> ##
> { 'command': 'guest-get-memory-block-info',
> - 'returns': 'GuestMemoryBlockInfo' }
> + 'returns': 'GuestMemoryBlockInfo',
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestExecStatus:
> @@ -1702,7 +1711,8 @@
> 'data': {'name': 'str',
> 'major': 'uint64',
> 'minor': 'uint64',
> - 'stats': 'GuestDiskStats' } }
> + 'stats': 'GuestDiskStats' },
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @guest-get-diskstats:
> @@ -1714,7 +1724,8 @@
> # Since: 7.1
> ##
> { 'command': 'guest-get-diskstats',
> - 'returns': ['GuestDiskStatsInfo']
> + 'returns': ['GuestDiskStatsInfo'],
> + 'if': 'CONFIG_POSIX'
> }
>
> ##
> @@ -1727,7 +1738,8 @@
> # Since: 7.1
> ##
> { 'enum': 'GuestCpuStatsType',
> - 'data': [ 'linux' ] }
> + 'data': [ 'linux' ],
> + 'if': 'CONFIG_POSIX' }
>
>
> ##
> @@ -1772,7 +1784,8 @@
> '*steal': 'uint64',
> '*guest': 'uint64',
> '*guestnice': 'uint64'
> - } }
> + },
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @GuestCpuStats:
> @@ -1786,7 +1799,8 @@
> { 'union': 'GuestCpuStats',
> 'base': { 'type': 'GuestCpuStatsType' },
> 'discriminator': 'type',
> - 'data': { 'linux': 'GuestLinuxCpuStats' } }
> + 'data': { 'linux': 'GuestLinuxCpuStats' },
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @guest-get-cpustats:
> @@ -1798,5 +1812,6 @@
> # Since: 7.1
> ##
> { 'command': 'guest-get-cpustats',
> - 'returns': ['GuestCpuStats']
> + 'returns': ['GuestCpuStats'],
> + 'if': 'CONFIG_POSIX'
> }
> --
> 2.45.1
>
>
