Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common

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.

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.

>> 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.

>>> 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).

>> 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.

>>> 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".

[...]