3:55 a.m.
On Fri, Apr 25, 2025 at 17:38:44 +0200, Markus Armbruster via Devel wrote:
Pierrick Bouvier <pierrick.bouvier(a)linaro.org> writes:
[...]
To be precise: conditionals that use macros restricted to
target-specific code, i.e. the ones poisoned by exec/poison.h. Let's
call them target-specific QAPI conditionals.
The QAPI generator is blissfully unaware of all this.
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
I've had a look at what bits of the QMP schema are depending on the
above defines which libvirt uses.
In many cases libvirt could restrict the use of given command/property
to only supported architectures. We decided to simply probe the presence
of the command because it's convenient to not have to filter them any
more
- query-gic-capabilities
- libvirt already calls this only for ARM guests based on the
definition
- query-sev and friends
- libvirt uses presence of 'query-sev' to decide if the binary
supports it; patching in a platofrm check is possible although
inconvenient
- query-sgx and friends
- similar to sev
-query-cpu-definitions and friends
- see below
> What we try to do here is to build them only once
instead.
You're trying to eliminate target-specific QAPI conditionals. Correct?
> 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.
Indeed and libvirt already uses this in few cases as noded above.
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.
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."
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.
I wonder how this will work if libvirt is probing a binary. Libvirt does
not look at the filename. It can't because it can be a
user-specified/compiled binary, override script, or a distro that chose
to rename the binary.
The second thing that libvirt does after 'query-version' is
'query-target'.
So what should libvirt do once multiple targets are supported?
How do we query CPUs for each of the supported targets?
Will the result be the same if we query them one at a time or all at
once?
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.
As noted filename will not work. Users can specify any filename and
create override scripts or rename the binary.
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.
Ah I see you had a similar question :D
Of course, "fixing the target" stops making sense once we move to
heterogeneous machines with multiple targets.
> 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