On 4/29/25 12:43 AM, Markus Armbruster wrote:
Pierrick Bouvier <pierrick.bouvier(a)linaro.org> writes:
> 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.
The name is ugly. Naming is hard. No need to worry about it right now.
Sure, when I'll work on a v2, I'll use "whatever_if". Meanwhile, pick a
name you like to describe a runtime vs compile time check, and I'll do
the sed.
Semantics of having both 'if' and 'available_if'? To
work out an
answer, let's consider how to convert conditionals:
* 'if': STRING
If STRING is a target-specific macro, replace by 'available_if': PRED,
where PRED is the equivalent run-time predicate.
Else, no change.
* 'if': { 'all': [COND, ...] }
If COND contains only target-specific macros, replace by
'available_if': { 'all': [PRED, ...] }, where the PRED are the
equivalent run-time predicates.
If COND contains no target-specific macros, no change.
What if it contains both?
- If each COND contains either only target-specific macros, or no
target-specific macros, we could split the target-specific ones off
into an additional 'available_if'. This requires defining the
semantics of having both 'if' and 'available_if' as "both
conditions
must be satisfied".
- What if this isn't the case?
* 'if' { 'any': [COND, ...] }
Similar, but to be able to split the COND we need "either condition
must be satisfied".
I don't see any reason to block having both. You may want to condition
it by a define derived from config-host.h, and add a runtime check as
well. Both "checks" won't apply in the same locations.
We can add any restriction on having both at the same time, works for me
also. No strong opinion here.
Even if we can make this work somehow, it would likely be a royal
mess
to explain in qapi-code-gen.rst.
We currently don't have "mixed" conditionals. So we could sidestep the
problem: you can have either 'if' or 'available_if', but not both.
Feels like a cop out to me.
What if we move the "is dynamic" bit from the root of the conditional to
its leaves? So far, the leaves are macro names. What if we
additionally permit a function name?
Function name, not C expression, to not complicate generating code in
languages other than C too much.
I don't think we should think too much ahead for languages other than C,
for one, two, and even three reasons :)
- First, it's already broken because we rely on ifdef that won't be
there in Rust or Go.
- Second, it's code, we can just change it later if needed.
- Third, those json are consumed only by QEMU (right?), so we are free
to write/modify them as we want.
The only thing that must stay the same is what we expose to the consumer
in the schema, and which commands we expose in qemu.
Ignore the question of syntax for now, i.e. how to decide whether a
leaf
is a macro or a function name.
>> 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.
I can't see concrete blockers at this time. I just wanted to make you
aware of the emerging need to support other languages.
[...]
>> 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.
I'm mildly unhappy about the bloat, but not enough to fix it.
[...]
>> 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.
I reiterate my dislike for having behavior depend on argv[0]. We'll
figure out something.
I'll answer on the dedicated subthread.
[...]