| Message ID | 20240613154406.1365469-7-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-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(-) 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-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 > >
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:
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(-)