Re: [PATCH v5 28/69] qapi: Move definitions related to accelerators in their own file

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Markus,

On 3/7/25 12:54, Philippe Mathieu-Daudé wrote:
> Extract TCG and KVM definitions from machine.json to accelerator.json.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   MAINTAINERS                |  1 +
>   qapi/accelerator.json      | 57 ++++++++++++++++++++++++++++++++++++++
>   qapi/machine.json          | 47 -------------------------------
>   qapi/qapi-schema.json      |  1 +
>   accel/tcg/monitor.c        |  2 +-
>   hw/core/machine-hmp-cmds.c |  1 +
>   hw/core/machine-qmp-cmds.c |  1 +
>   qapi/meson.build           |  1 +
>   8 files changed, 63 insertions(+), 48 deletions(-)
>   create mode 100644 qapi/accelerator.json
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b1cbfe115bc..c3ce0d37779 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -507,6 +507,7 @@ F: accel/Makefile.objs
>   F: accel/stubs/Makefile.objs
>   F: cpu-common.c
>   F: cpu-target.c
> +F: qapi/accelerator.json
>   F: system/cpus.c
>   
>   Apple Silicon HVF CPUs
> diff --git a/qapi/accelerator.json b/qapi/accelerator.json
> new file mode 100644
> index 00000000000..00d25427059
> --- /dev/null
> +++ b/qapi/accelerator.json
> @@ -0,0 +1,57 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +##
> +# = Accelerators
> +##
> +
> +{ 'include': 'common.json' }

common.json defines @HumanReadableText, ...

[...]

> +##
> +# @x-query-jit:
> +#
> +# Query TCG compiler statistics
> +#
> +# Features:
> +#
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: TCG compiler statistics
> +#
> +# Since: 6.2
> +##
> +{ 'command': 'x-query-jit',
> +  'returns': 'HumanReadableText',
> +  'if': 'CONFIG_TCG',

... which is *optionally* used here, triggering when
TCG is not built in:

qapi/qapi-commands-accelerator.c:85:13: error: 
‘qmp_marshal_output_HumanReadableText’ defined but not used 
[-Werror=unused-function]
    85 | static void 
qmp_marshal_output_HumanReadableText(HumanReadableText *ret_in,
       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

We previously discussed that issue:
https://mail.gnu.org/archive/html/qemu-devel/2021-06/msg02667.html

where you said:

"conditional commands returning an unconditional type is a bit
of a code smell". Is it however a "non-smelly instances of this pattern"?

Regards,

Phil.