On 4/25/25 11:21 PM, Markus Armbruster wrote:
Trouble is some uses of the second kind are in QAPI conditionals. I can
see three options:
(1) Drop these conditionals.
(2) Replace them by run-time checks.
(3) Have target-specific QAPI-generated code for multiple targets
coexist in the single binary.
As far as I can tell, your RFC series is an incomplete attempt at (2).
I gather you considered (3), but you dislike it for its bloat and
possibly other reasons. I sympathize; the QAPI-generated code is plenty
bloated as it is, in good part to early design decisions (not mine).
Your "no noticeable differences" goal precludes (1).
Back to (2). In C, replacing compile-time conditionals by run-time
checks means replacing #if FOO by if (foo). Such a transformation isn't
possible in the QAPI schema. To make it possible, we need to evolve the
QAPI schema language.
docs/devel/qapi-code-gen.rst describes what we have:
COND = STRING
| { 'all: [ COND, ... ] }
| { 'any: [ COND, ... ] }
| { 'not': COND }
[....]
The C code generated for the definition will then be guarded by an #if
preprocessing directive with an operand generated from that condition:
* STRING will generate defined(STRING)
* { 'all': [COND, ...] } will generate (COND && ...)
* { 'any': [COND, ...] } will generate (COND || ...)
* { 'not': COND } will generate !COND
So, conditions are expression trees where the leaves are preprocessor
symbols and the inner nodes are operators.
It's not quite obvious to me how to best evolve this to support run-time
checks.
After looking at the introspection code, I don't see any major blocker.
We need to keep some of existing "if", as they are based on config-host,
and should apply.
We can introduce a new "available_if" (or any other name), which
generates a runtime check when building the schema, or when serializing
a struct.
This way, by modifying the .json with:
- if: 'TARGET_I386'
+ available_if: 'target_i386()'
This way, we keep the possibility to have ifdef, and we can expose at
runtime based on available_if. So we can keep the exact same schema we
have today per target.
Whatever we choose should support generating Rust and Go as well. Why?
Rust usage in QEMU is growing, and we'll likely need to generate some
Rust from the QAPI schema. Victor Toso has been working on Go bindings
for use in Go QMP client software.
I don't see any blocker with that. If you mention generating Rust and Go
from qapi json definitions, it's already dependent on C preprocessor
because of ifdef constant. So it will have to be adapted anyway.
Having the same function (target_i386()) name through different
languages is not something hard to achieve.
The build system treats QAPI modules qapi/*-target.json as
target-specific. The .c files generated for them are compiled per
target. See qapi/meson.build.
Only such target-specific modules can can use target-specific QAPI
conditionals. Use in target-independent modules will generate C that
won't compile.
Poisoned macros used in qapi/*-target.json:
CONFIG_KVM
TARGET_ARM
TARGET_I386
TARGET_LOONGARCH64
TARGET_MIPS
TARGET_PPC
TARGET_RISCV
TARGET_S390X
What we try to do here is to build them only once
instead.
You're trying to eliminate target-specific QAPI conditionals. Correct?
Yes, but without impacting the list of commands exposed. Thus, it would
be needed to select at runtime to expose/register commands.
Conditionals affect more than just commands.
Thus, the proposal above to do the same for concerned struct members.
In the past, we identied that the best approach to solve this is to expose code
for all targets (thus removing all #if clauses), and stub missing
symbols for concerned targets.
This affects QAPI/QMP introspection, i.e. the value of query-qmp-schema.
Management applications can no longer use introspection to find out
whether target-specific things are available.
As asked on my previous email answering Daniel, would that be possible
to build the schema dynamically, so we can decide what to expose or not
introspection wise?
QAPI was designed to be compile-time static. Revising such fundamental
design assumptions is always fraught. I can't give you a confident
assessment now. All I can offer you is my willingness to explore
solutions. See "really fancy" below.
Fun fact: we used to generate the value of query-qmp-schema as a single
string. We switched to the current, more bloated representation to
support conditionals (commit 7d0f982bfbb).
It's nice to have this, and this is what would allow us to
conditionnally include or not various definitions/commands/fields. I was
a bit worried we would have a "static string", but was glad to find a
static list instead.
For instance, query-cpu-definitions is implemented for targets arm,
i386, loongarch, mips, ppc, riscv, and s390x. It initially was for
fewer targets, and more targets followed one by one. Still more may
follow in the future. Right now, management applications can use
introspection to find out whether it is available. That stops working
when you make it available for all targets, stubbed out for the ones
that don't (yet) implement it.
I will repeat, just to be clear, I don't think exposing all commands is
a good idea.
The current series *does not* do this, simply because I didn't want to
huge work for nothing.
Got it.
Management applications may have to be adjusted for this.
This is not an attempt to shoot down your approach. I'm merely
demonstrating limitations of your promise "if anyone notices a
difference, it will be a bug."
I stick to this promise :).
Now, we could get really fancy and try to keep introspection the same by
applying conditionals dynamically somehow. I.e. have the single binary
return different introspection values depending on the actual guest's
target.
This requires fixing the target before introspection. Unless this is
somehow completely transparent (wrapper scripts, or awful hacks based on
the binary's filename, perhaps), management applications may have to be
adjusted to actually do that.
Applies not just to introspection. Consider query-cpu-definitions
again. It currently returns CPU definitions for *the* target. What
would a single binary's query-cpu-definitions return? The CPU
definitions for *all* its targets? Management applications then receive
CPUs that won't work, which may upset them. To avoid noticable
difference, we again have to fix the target before we look.
Of course, "fixing the target" stops making sense once we move to
heterogeneous machines with multiple targets.
At this point, I don't have think about what should be the semantic when
we'll have multiple targets running simultaneously (expose the union,
restrict to the main arch, choose a third way).
We have to unless we make query-cpu-definitions fail or impossible to
send while the target is still undecided.
Making it fail would violate the "no observable differences" goal.
The only path to true "no observable differences" I can see is to fix
the target before the management application interacts with QEMU in any
way. This would make QMP commands (query-cpu-definitions,
query-qmp-schema, ...) impossible to send before the target is fixed.
The current target will be set at the entry of main() in QEMU, so before
the monitor is created. Thus, it will be unambiguous.
This series build QAPI generated code once, by removing all TARGET_{arch} and
CONFIG_KVM clauses. What it does *not* at the moment is:
- prevent target specific commands to be visible for all targets
(see TODO comment on patch 2 explaining how to address this)
- nothing was done to hide all this from generated documentation
For better or worse, generated documentation always contains everything.
Fine for me, it makes sense, as the official documentation published,
which is what people will consume primarily, is for all targets.
An argument could be made for stripping out documentation for the stuff
that isn't included in this build.
From what I understood, the only thing that matters is to limit qmp commands
visible. Exposing enums, structure, or events is not a problem, since they
won't be used/triggered for non concerned targets. Please correct me if this is
wrong, and if there are unexpected consequences for libvirt or other consumers.
I'm not sure what you mean by "to limit qmp commands visible".
QAPI/QMP introspection has all commands and events, and all types
reachable from them. query-qmp-schema returns an array, where each
array element describes one command, event, or type. When a command,
event, or type is conditional in the schema, the element is wrapped in
the #if generated for the condition.
After reading and answering to your valuable email, I definitely think
Thanks!
the introspection schema we expose should be adapted, independently of
how we build QAPI code (i.e. using #ifdef TARGET or not).
Is it something technically hard to achieve?
Unclear. See "fundamental design assumptions" and "need to evolve the
QAPI schema language" above.
If you want to learn more about introspection, I'd recommend
docs/devel/qapi-code-gen.rst section "Client JSON Protocol
introspection".
I'll give a try at conditioning all this by runtime checks, so you can
review which changes it would create.
[...]
Regards,
Pierrick
