| Message ID | 20240613154406.1365469-4-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qga: clean up command source locations and conditionals | expand |
On 13/6/24 17:43, Daniel P. Berrangé 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-Linux POSIX > platforms > > 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 | 66 -------------------------------------------- > qga/qapi-schema.json | 30 +++++++++++--------- > 2 files changed, 17 insertions(+), 79 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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-Linux POSIX > platforms > > 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 | 66 -------------------------------------------- > qga/qapi-schema.json | 30 +++++++++++--------- > 2 files changed, 17 insertions(+), 79 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 0dd8555867..559d71ffae 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -887,56 +887,6 @@ void qmp_guest_set_user_password(const char *username, > } > #endif /* __linux__ || __FreeBSD__ */ > > -#ifndef __linux__ > - > -void qmp_guest_suspend_disk(Error **errp) > -{ > - error_setg(errp, QERR_UNSUPPORTED); > -} > - > -void qmp_guest_suspend_ram(Error **errp) > -{ > - error_setg(errp, QERR_UNSUPPORTED); > -} > - > -void qmp_guest_suspend_hybrid(Error **errp) > -{ > - error_setg(errp, QERR_UNSUPPORTED); > -} > - > -GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) > -{ > - error_setg(errp, QERR_UNSUPPORTED); > - return NULL; > -} > - > -int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error > **errp) > -{ > - error_setg(errp, QERR_UNSUPPORTED); > - return -1; > -} > - > -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; > -} > - > -#endif > - > #ifdef HAVE_GETIFADDRS > static GuestNetworkInterface * > guest_find_interface(GuestNetworkInterfaceList *head, > @@ -1272,22 +1222,6 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, > Error **errp) > /* add unsupported commands to the list of blocked RPCs */ > GList *ga_command_init_blockedrpcs(GList *blockedrpcs) > { > -#if !defined(__linux__) > - { > - const char *list[] = { > - "guest-suspend-disk", "guest-suspend-ram", > - "guest-suspend-hybrid", "guest-get-vcpus", "guest-set-vcpus", > - "guest-get-memory-blocks", "guest-set-memory-blocks", > - "guest-get-memory-block-info", > - NULL}; > - const char **p = list; > - > - while (*p) { > - blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++)); > - } > - } > -#endif > - > #if !defined(HAVE_GETIFADDRS) > blockedrpcs = g_list_append(blockedrpcs, > g_strdup("guest-network-get-interfaces")); > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index b91456e9ad..d164c30ec3 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -565,7 +565,8 @@ > # > # Since: 1.1 > ## > -{ 'command': 'guest-suspend-disk', 'success-response': false } > +{ 'command': 'guest-suspend-disk', 'success-response': false, > + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } > > ## > # @guest-suspend-ram: > @@ -601,7 +602,8 @@ > # > # Since: 1.1 > ## > -{ 'command': 'guest-suspend-ram', 'success-response': false } > +{ 'command': 'guest-suspend-ram', 'success-response': false, > + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } > > ## > # @guest-suspend-hybrid: > @@ -637,7 +639,7 @@ > # Since: 1.1 > ## > { 'command': 'guest-suspend-hybrid', 'success-response': false, > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @GuestIpAddressType: > @@ -750,7 +752,8 @@ > { 'struct': 'GuestLogicalProcessor', > 'data': {'logical-id': 'int', > 'online': 'bool', > - '*can-offline': 'bool'} } > + '*can-offline': 'bool'}, > + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } > > ## > # @guest-get-vcpus: > @@ -765,7 +768,8 @@ > # Since: 1.5 > ## > { 'command': 'guest-get-vcpus', > - 'returns': ['GuestLogicalProcessor'] } > + 'returns': ['GuestLogicalProcessor'], > + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } > > ## > # @guest-set-vcpus: > @@ -808,7 +812,7 @@ > { 'command': 'guest-set-vcpus', > 'data': {'vcpus': ['GuestLogicalProcessor'] }, > 'returns': 'int', > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @GuestDiskBusType: > @@ -1102,7 +1106,7 @@ > 'data': {'phys-index': 'uint64', > 'online': 'bool', > '*can-offline': 'bool'}, > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @guest-get-memory-blocks: > @@ -1119,7 +1123,7 @@ > ## > { 'command': 'guest-get-memory-blocks', > 'returns': ['GuestMemoryBlock'], > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @GuestMemoryBlockResponseType: > @@ -1143,7 +1147,7 @@ > { 'enum': 'GuestMemoryBlockResponseType', > 'data': ['success', 'not-found', 'operation-not-supported', > 'operation-failed'], > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @GuestMemoryBlockResponse: > @@ -1162,7 +1166,7 @@ > 'data': { 'phys-index': 'uint64', > 'response': 'GuestMemoryBlockResponseType', > '*error-code': 'int' }, > - 'if': 'CONFIG_POSIX'} > + 'if': 'CONFIG_LINUX'} > > ## > # @guest-set-memory-blocks: > @@ -1194,7 +1198,7 @@ > { 'command': 'guest-set-memory-blocks', > 'data': {'mem-blks': ['GuestMemoryBlock'] }, > 'returns': ['GuestMemoryBlockResponse'], > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @GuestMemoryBlockInfo: > @@ -1207,7 +1211,7 @@ > ## > { 'struct': 'GuestMemoryBlockInfo', > 'data': {'size': 'uint64'}, > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @guest-get-memory-block-info: > @@ -1220,7 +1224,7 @@ > ## > { 'command': 'guest-get-memory-block-info', > 'returns': 'GuestMemoryBlockInfo', > - 'if': 'CONFIG_POSIX' } > + 'if': 'CONFIG_LINUX' } > > ## > # @GuestExecStatus: > -- > 2.45.1 > >
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 0dd8555867..559d71ffae 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -887,56 +887,6 @@ void qmp_guest_set_user_password(const char *username, } #endif /* __linux__ || __FreeBSD__ */ -#ifndef __linux__ - -void qmp_guest_suspend_disk(Error **errp) -{ - error_setg(errp, QERR_UNSUPPORTED); -} - -void qmp_guest_suspend_ram(Error **errp) -{ - error_setg(errp, QERR_UNSUPPORTED); -} - -void qmp_guest_suspend_hybrid(Error **errp) -{ - error_setg(errp, QERR_UNSUPPORTED); -} - -GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) -{ - error_setg(errp, QERR_UNSUPPORTED); - return NULL; -} - -int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) -{ - error_setg(errp, QERR_UNSUPPORTED); - return -1; -} - -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; -} - -#endif - #ifdef HAVE_GETIFADDRS static GuestNetworkInterface * guest_find_interface(GuestNetworkInterfaceList *head, @@ -1272,22 +1222,6 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) /* add unsupported commands to the list of blocked RPCs */ GList *ga_command_init_blockedrpcs(GList *blockedrpcs) { -#if !defined(__linux__) - { - const char *list[] = { - "guest-suspend-disk", "guest-suspend-ram", - "guest-suspend-hybrid", "guest-get-vcpus", "guest-set-vcpus", - "guest-get-memory-blocks", "guest-set-memory-blocks", - "guest-get-memory-block-info", - NULL}; - const char **p = list; - - while (*p) { - blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++)); - } - } -#endif - #if !defined(HAVE_GETIFADDRS) blockedrpcs = g_list_append(blockedrpcs, g_strdup("guest-network-get-interfaces")); diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index b91456e9ad..d164c30ec3 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -565,7 +565,8 @@ # # Since: 1.1 ## -{ 'command': 'guest-suspend-disk', 'success-response': false } +{ 'command': 'guest-suspend-disk', 'success-response': false, + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } ## # @guest-suspend-ram: @@ -601,7 +602,8 @@ # # Since: 1.1 ## -{ 'command': 'guest-suspend-ram', 'success-response': false } +{ 'command': 'guest-suspend-ram', 'success-response': false, + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } ## # @guest-suspend-hybrid: @@ -637,7 +639,7 @@ # Since: 1.1 ## { 'command': 'guest-suspend-hybrid', 'success-response': false, - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @GuestIpAddressType: @@ -750,7 +752,8 @@ { 'struct': 'GuestLogicalProcessor', 'data': {'logical-id': 'int', 'online': 'bool', - '*can-offline': 'bool'} } + '*can-offline': 'bool'}, + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } ## # @guest-get-vcpus: @@ -765,7 +768,8 @@ # Since: 1.5 ## { 'command': 'guest-get-vcpus', - 'returns': ['GuestLogicalProcessor'] } + 'returns': ['GuestLogicalProcessor'], + 'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] } } ## # @guest-set-vcpus: @@ -808,7 +812,7 @@ { 'command': 'guest-set-vcpus', 'data': {'vcpus': ['GuestLogicalProcessor'] }, 'returns': 'int', - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @GuestDiskBusType: @@ -1102,7 +1106,7 @@ 'data': {'phys-index': 'uint64', 'online': 'bool', '*can-offline': 'bool'}, - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @guest-get-memory-blocks: @@ -1119,7 +1123,7 @@ ## { 'command': 'guest-get-memory-blocks', 'returns': ['GuestMemoryBlock'], - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @GuestMemoryBlockResponseType: @@ -1143,7 +1147,7 @@ { 'enum': 'GuestMemoryBlockResponseType', 'data': ['success', 'not-found', 'operation-not-supported', 'operation-failed'], - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @GuestMemoryBlockResponse: @@ -1162,7 +1166,7 @@ 'data': { 'phys-index': 'uint64', 'response': 'GuestMemoryBlockResponseType', '*error-code': 'int' }, - 'if': 'CONFIG_POSIX'} + 'if': 'CONFIG_LINUX'} ## # @guest-set-memory-blocks: @@ -1194,7 +1198,7 @@ { 'command': 'guest-set-memory-blocks', 'data': {'mem-blks': ['GuestMemoryBlock'] }, 'returns': ['GuestMemoryBlockResponse'], - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @GuestMemoryBlockInfo: @@ -1207,7 +1211,7 @@ ## { 'struct': 'GuestMemoryBlockInfo', 'data': {'size': 'uint64'}, - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @guest-get-memory-block-info: @@ -1220,7 +1224,7 @@ ## { 'command': 'guest-get-memory-block-info', 'returns': 'GuestMemoryBlockInfo', - 'if': 'CONFIG_POSIX' } + 'if': 'CONFIG_LINUX' } ## # @GuestExecStatus:
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-Linux POSIX platforms 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 | 66 -------------------------------------------- qga/qapi-schema.json | 30 +++++++++++--------- 2 files changed, 17 insertions(+), 79 deletions(-)