On 16/7/25 11:24, Daniel P. Berrangé wrote:
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.
Indeed, I now remembered it:
https://lore.kernel.org/qemu-devel/87r1haasht....@dusky.pond.sub.org/
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
I agree, the same was suggested as comment in my previous patch
https://lore.kernel.org/qemu-devel/20210609184955.1193081-2-phi...@redhat.com/
Markus, WDYT?
Regards,
Phil.
