Thomas Huth <th...@redhat.com> writes:
> On 07/02/2023 16.39, Markus Armbruster wrote:
>> We have a boatload of CONFIG_FOO macros that may only be used in
>> target-dependent code. We use generated config-poison.h to enforce.
>> This is a bit annoying in the QAPI schema. Let me demonstrate with an
>> example: QMP commands query-rocker, query-rocker-ports, and so forth.
>> These commands are useful only with "rocker" devices. They are
>> compile-time optional. hw/net/Kconfig:
>> config ROCKER
>> bool
>> default y if PCI_DEVICES
>> depends on PCI && MSI_NONBROKEN
>> The rocker device and QMP code is actually target-independent:
>> hw/net/meson.build puts it into softmmu_ss.
>> Disabling the "rocker" device type ideally disables the rocker QMP
>> commands, too. Should be easy enough: 'if': 'CONFIG_FOO' in the QAPI
>> schema.
>> Except that makes the entire code QAPI generates for rocker.json
>> device-dependent: it now contains #if defined(CONFIG_ROCKER), and
>> CONFIG_ROCKER is poisoned. The rocker code implementing monitor
>> commands also becomes device-dependent, because it includes generated
>> headers. We compile all that per target for no sane reason at all.
>
> Well, CONFIG_ROCKER depends on the target config, so that is a sane reason in
> my eyes. Depending on which target you currently compile, it's either set or
> not set - there is no reasonable way to compile generic code and still test
> the CONFIG_ROCKER macro.
>
> So adding such a switch to rocker.json itself just cannot work.
>
> But at the "make" level, we distinguish between config-all-devices.mak and
> *-softmmu-config-devices.mak , so if you want to disable the rocker QMP code
> depending on whether CONFIG_ROCKER is set in *any* target binary or not, you
> would need a similar mechanism to config-all-devices.mak here, e.g. something
> that would switch the inclusion of rocker.json on or off depending on the
> switch in config-all-devices.mak ...
> Would it be possible to add such a switch to qapi/meson.build ?
> ... and to qapi/qapi-schema.json, I guess, since the rocker.json gets
> included from there? Something like that:
>
> diff --git a/qapi/meson.build b/qapi/meson.build
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -59,9 +59,11 @@ if have_system
> 'qdev',
> 'pci',
> 'rdma',
> - 'rocker',
> 'tpm',
> ]
> + if config_all_devices.has_key('CONFIG_ROCKER')
> + qapi_all_modules += [ 'rocker', ]
> + endif
> endif
> if have_system or have_tools
> qapi_all_modules += [
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -72,7 +72,7 @@
> { 'include': 'job.json' }
> { 'include': 'net.json' }
> { 'include': 'rdma.json' }
> -{ 'include': 'rocker.json' }
> +{ 'include': 'rocker.json', 'if': 'CONFIG_ROCKER' }
> { 'include': 'tpm.json' }
> { 'include': 'ui.json' }
> { 'include': 'authz.json' }
>
> No clue whether the qapi parser could be extended like that, though...
Feels feasible.
I had vague ideas going this direction, too. Thanks for fleshing them
out for me!
Hmm, we have somewhat hairy code in qapi/meson.build to enumerate the
QAPI modules and generated source files, and to add them to the right
source sets. Perhaps we could have qapi-gen generate some of that.
