Reviewed-by: Konstantin Kostiuk <kkost...@redhat.com>
On Fri, Jul 12, 2024 at 4:25 PM Daniel P. Berrangé <berra...@redhat.com>
wrote:
> Some commands were blocked based on CONFIG_FSFREEZE, but their
> impl had nothing todo with CONFIG_FSFREEZE, and were instead
> either Linux-only, or Win+Linux-only.
>
> 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 stats and fsinfo commands on
> platforms that can't support them.
>
> 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é <berra...@redhat.com>
> ---
> qga/commands-bsd.c | 24 -----------------------
> qga/commands-posix.c | 30 ++---------------------------
> qga/qapi-schema.json | 45 +++++++++++++++++++++++++++-----------------
> 3 files changed, 30 insertions(+), 69 deletions(-)
>
> diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
> index 17bddda1cf..9ce48af311 100644
> --- a/qga/commands-bsd.c
> +++ b/qga/commands-bsd.c
> @@ -149,30 +149,6 @@ int qmp_guest_fsfreeze_do_thaw(Error **errp)
> }
> return ret;
> }
> -
> -GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> -
> -GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return 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;
> -}
> #endif /* CONFIG_FSFREEZE */
>
> #ifdef HAVE_GETIFADDRS
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 09d08ee2ca..838dc3cf98 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1146,12 +1146,6 @@ error:
>
> #if !defined(CONFIG_FSFREEZE)
>
> -GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return NULL;
> -}
> -
> GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
> {
> error_setg(errp, QERR_UNSUPPORTED);
> @@ -1181,25 +1175,6 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
>
> return 0;
> }
> -
> -GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> -{
> - error_setg(errp, QERR_UNSUPPORTED);
> - return 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;
> -}
> -
> #endif /* CONFIG_FSFREEZE */
>
> #if !defined(CONFIG_FSTRIM)
> @@ -1217,10 +1192,9 @@ GList *ga_command_init_blockedrpcs(GList
> *blockedrpcs)
> #if !defined(CONFIG_FSFREEZE)
> {
> const char *list[] = {
> - "guest-get-fsinfo", "guest-fsfreeze-status",
> + "guest-fsfreeze-status",
> "guest-fsfreeze-freeze", "guest-fsfreeze-freeze-list",
> - "guest-fsfreeze-thaw", "guest-get-fsinfo",
> - "guest-get-disks", NULL};
> + "guest-fsfreeze-thaw", NULL};
> char **p = (char **)list;
>
> while (*p) {
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 79ed4f0e21..9bd5aa53bc 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -870,7 +870,8 @@
> { 'enum': 'GuestDiskBusType',
> 'data': [ 'ide', 'fdc', 'scsi', 'virtio', 'xen', 'usb', 'uml', 'sata',
> 'sd', 'unknown', 'ieee1394', 'ssa', 'fibre', 'raid', 'iscsi',
> - 'sas', 'mmc', 'virtual', 'file-backed-virtual', 'nvme' ] }
> + 'sas', 'mmc', 'virtual', 'file-backed-virtual', 'nvme' ],
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
>
> ##
> @@ -888,7 +889,8 @@
> ##
> { 'struct': 'GuestPCIAddress',
> 'data': {'domain': 'int', 'bus': 'int',
> - 'slot': 'int', 'function': 'int'} }
> + 'slot': 'int', 'function': 'int'},
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @GuestCCWAddress:
> @@ -907,7 +909,8 @@
> 'data': {'cssid': 'int',
> 'ssid': 'int',
> 'subchno': 'int',
> - 'devno': 'int'} }
> + 'devno': 'int'},
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @GuestDiskAddress:
> @@ -936,7 +939,8 @@
> 'bus-type': 'GuestDiskBusType',
> 'bus': 'int', 'target': 'int', 'unit': 'int',
> '*serial': 'str', '*dev': 'str',
> - '*ccw-address': 'GuestCCWAddress'} }
> + '*ccw-address': 'GuestCCWAddress'},
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @GuestNVMeSmart:
> @@ -973,7 +977,8 @@
> 'media-errors-lo': 'uint64',
> 'media-errors-hi': 'uint64',
> 'number-of-error-log-entries-lo': 'uint64',
> - 'number-of-error-log-entries-hi': 'uint64' } }
> + 'number-of-error-log-entries-hi': 'uint64' },
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @GuestDiskSmart:
> @@ -987,7 +992,8 @@
> { 'union': 'GuestDiskSmart',
> 'base': { 'type': 'GuestDiskBusType' },
> 'discriminator': 'type',
> - 'data': { 'nvme': 'GuestNVMeSmart' } }
> + 'data': { 'nvme': 'GuestNVMeSmart' },
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @GuestDiskInfo:
> @@ -1012,7 +1018,8 @@
> { 'struct': 'GuestDiskInfo',
> 'data': {'name': 'str', 'partition': 'bool', '*dependencies': ['str'],
> '*address': 'GuestDiskAddress', '*alias': 'str',
> - '*smart': 'GuestDiskSmart'} }
> + '*smart': 'GuestDiskSmart'},
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @guest-get-disks:
> @@ -1025,7 +1032,8 @@
> # Since: 5.2
> ##
> { 'command': 'guest-get-disks',
> - 'returns': ['GuestDiskInfo'] }
> + 'returns': ['GuestDiskInfo'],
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @GuestFilesystemInfo:
> @@ -1051,7 +1059,8 @@
> { 'struct': 'GuestFilesystemInfo',
> 'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
> '*used-bytes': 'uint64', '*total-bytes': 'uint64',
> - '*total-bytes-privileged': 'uint64', 'disk':
> ['GuestDiskAddress']} }
> + '*total-bytes-privileged': 'uint64', 'disk':
> ['GuestDiskAddress']},
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @guest-get-fsinfo:
> @@ -1064,7 +1073,8 @@
> # Since: 2.2
> ##
> { 'command': 'guest-get-fsinfo',
> - 'returns': ['GuestFilesystemInfo'] }
> + 'returns': ['GuestFilesystemInfo'],
> + 'if': { 'any': [ 'CONFIG_WIN32', 'CONFIG_LINUX' ] } }
>
> ##
> # @guest-set-user-password:
> @@ -1703,7 +1713,8 @@
> '*ios-pgr': 'uint64',
> '*total-ticks': 'uint64',
> '*weight-ticks': 'uint64'
> - } }
> + },
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @GuestDiskStatsInfo:
> @@ -1721,7 +1732,7 @@
> 'major': 'uint64',
> 'minor': 'uint64',
> 'stats': 'GuestDiskStats' },
> - 'if': 'CONFIG_POSIX' }
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @guest-get-diskstats:
> @@ -1734,7 +1745,7 @@
> ##
> { 'command': 'guest-get-diskstats',
> 'returns': ['GuestDiskStatsInfo'],
> - 'if': 'CONFIG_POSIX'
> + 'if': 'CONFIG_LINUX'
> }
>
> ##
> @@ -1748,7 +1759,7 @@
> ##
> { 'enum': 'GuestCpuStatsType',
> 'data': [ 'linux' ],
> - 'if': 'CONFIG_POSIX' }
> + 'if': 'CONFIG_LINUX' }
>
>
> ##
> @@ -1794,7 +1805,7 @@
> '*guest': 'uint64',
> '*guestnice': 'uint64'
> },
> - 'if': 'CONFIG_POSIX' }
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @GuestCpuStats:
> @@ -1809,7 +1820,7 @@
> 'base': { 'type': 'GuestCpuStatsType' },
> 'discriminator': 'type',
> 'data': { 'linux': 'GuestLinuxCpuStats' },
> - 'if': 'CONFIG_POSIX' }
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @guest-get-cpustats:
> @@ -1822,5 +1833,5 @@
> ##
> { 'command': 'guest-get-cpustats',
> 'returns': ['GuestCpuStats'],
> - 'if': 'CONFIG_POSIX'
> + 'if': 'CONFIG_LINUX'
> }
> --
> 2.45.1
>
>
