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

[Marc-André Lureau]

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.