Re: [Qemu-devel] [PATCH v3 12/15] monitor: use qmp_dispatch()

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Hi
>
> ----- Original Message -----
>> Marc-André Lureau <address@hidden> writes:
>> 
>> > Hi
>> >
>> > ----- Original Message -----
>> >> address@hidden writes:
>> >> 
>> >> > From: Marc-André Lureau <address@hidden>
>> >> >
>> >> > Replace the old manual dispatch and validation code by the generic one
>> >> > provided by qapi common code.
>> >> >
>> >> > Note that it is now possible to call the following commands that used to
>> >> > be disabled by compile-time conditionals:
>> >> > - dump-skeys
>> >> > - query-spice
>> >> > - rtc-reset-reinjection
>> >> > - query-gic-capabilities
>> >> >
>> >> > Their fallback functions return an appropriate "feature disabled" error.
>> >> >
>> >> > Signed-off-by: Marc-André Lureau <address@hidden>
>> >> 
>> >> Means query-qmp-schema no longer shows whether these commands are
>> >> supported, doesn't it?
>> >> 
>> >> Eric, could this create difficulties for libvirt or other introspection
>> >> users?
>> >
>> > Thinking a bit about this, I guess it would be fairly straightforward
>> > to have a new key "c-conditional" : "#ifdef CONFIG_SPICE" that would
>> > prepend it in C generated files, with a corresponding "#endif". Would
>> > that be acceptable?
>> 
>> Not exactly pretty, but the only alternative I can think of right now
>> would be conditional qapi generation, i.e. something like
>> 
>> { 'if': 'CONFIG_SPICE'
>>   'then': { 'command': 'query-spice', 'returns': 'SpiceInfo' } }
>> 
>> More general, but *much* more work.  Let's not go there now.
>
> That looks quite unnecessarily complicated to me, and not so declarative.
>
>> 
>> The value of key 'c-conditional' must be a preprocessing directive that
>> pairs with #endif.  Hmm.
>> 
>> Could make it an expression instead, and call the key just
>> 'conditional'.  If given, wrap the code generated for the QAPI
>> definition in
>> 
>>     #if <value of conditional>
>>     ...
>>     #endif
>> 
>
> Sure, we could make it a preprocessor expression instead, so it would have to 
> match with the automatically appened "#endif".
>
>> Feels cleaner, but to avoid -Wundef warnings, we'd have to say
>> 'defined(CONFIG_SPICE)'.
>
> Yes, why not? I can work on a patch see how well it fits.

Yes, please.