Daniel P. Berrangé <berra...@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.
The commit message should mention that the value of "error" in the error
response changes from
{"class": "GenericError, "desc": "this feature or command is not currently
supported"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
> This fixes inconsistency where some commands are implemented
> as stubs, yet not added to the blockedrpc list.
Example?
> This has the additional benefit that the QGA protocol reference
> now documents what conditions enable use of the command.
Yes!
> Signed-off-by: Daniel P. Berrangé <berra...@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
> @@ -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;
Can you make @p const and drop the cast?
>
> 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;
> -}
Reducing use of QERR_UNSUPPORTED is lovely.
[Schema patch snipped; it looks good to me...]
