On Wed, Jul 16, 2025 at 10:23:26AM +0200, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <phi...@linaro.org> writes:
>
> > Hi Markus,
>
> I missed this one, sorry!
>
> > 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é <phi...@linaro.org>
> >> Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
> >> Reviewed-by: Zhao Liu <zhao1....@intel.com>
>
> [...]
>
> >> 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
>
> This is a defect in the QAPI code generator. More below.
>
> > 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"?
>
> The instance discussed there wasn't.
>
> You ran into it when you made TPM commands conditional on CONFIG_TPM
> without also making the types they return conditional. The proper
> solution was to make the types conditional, too. Avoided generating
> dead code. I told you "The user is responsible for making T's 'if' the
> conjunction of the commands'."
>
> Some of the commands returning HumanReadableText are unconditional, so
> said conjunction is also unconditional. So how do we end up with unused
> qmp_marshal_output_HumanReadableText()?
>
> A qmp_marshal_output_T() is only ever called by qmp_marshal_C() for a
> command C that returns T.
>
> We've always generated it as a static function on demand, i.e. when we
> generate a call.
..snip..
> I need to ponder this to decide on a solution.
Functionally the redundat function is harmless, so the least effort
option is to change the generated QAPI headers to look like
#pragma GCC diagnostic push
#pragma GCC ignored "-Wunused-function"
... rest of QAPI header...
#pragma GCC diagnostic pop
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
