Re: [Qemu-devel] [PATCH v12 0/4] qapi: Allow modularization of QAPI schema files

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 05/06/2014 07:27 AM, Luiz Capitulino wrote:
>> On Tue, 6 May 2014 15:07:40 +0200
>> Benoît Canet <address@hidden> wrote:
>> 
>>> I am trying to use this series to modularise the block API.
>>>
>>> Here are my finding.
>>>
>>> I tried to make a qmp/block.json including VM state related API.
>>> block.json include a qmp/block-core.json containing only true block stuff.
>>>
>>> When generating and compiling block-core.json to link it with qemu-nbd
>>> I saw that some of the block stuff needed ErrorClass so I went the route
>>> of creating a qmp/common.json containing ErrorClass.
>>>
>>> common.json being included in block-core.json and in qapi-schema.json it
>>> quickly lead some code being generated in double and the
>>> compilation to choke.
>>>
>>> What do you think would be the best solution to fix this ?
>>> (Fix the generator ? Make include ignore second inclusion of the same file 
>>> ?)
>> 
>> Make qapi-schema.json a sort of master file and include everything?
>
> Won't cut it, if we want to support a subset of files in other contexts.
>  You either have to do:
>
> qemu-schema.json:
>   include common
>   include block
>   include other
>
> qemu-block:
>   include common
>   include block
>
> where block does not work in isolation, and has to be wrapped; but now
> we have two different wrappers depending on the two different clients
> that want a different subset.

Won't win beauty contests, but it isn't exactly terrible, either.

>                                Or you do:
>
> qemu-schema.json:
>   include common
>   include block
>   include other
> block.json:
>   include common
>
> now block.json is standalone, and qemu-schema.json ends up including
> common through two different paths.

Yes.

>> Eventually, we might want to have if/defs and whatnot. But having a master
>> file seems a reasonable first step to me. I actually thought this was the
>> intention. Unless I got it wrong, of course.
>
> Ifdefs may be a bit much.  If we add them, then we can worry about
> explicit include guards, the same as the C preprocessor.  But for now,
> I'd be perfectly fine with a followup patch that includes a file's
> contents exactly once, no matter how many times it is included (that is,
> act as if include guards were implicitly present, since we lack
> conditionals, so include files are currently idempotent).

As long as the meaning of an include doesn't depend on its environment
(and I don't see that change for QAPI schemata), making the include
idempotent is the right thing.

Benoît, Lluís, would either of you be willing to tackle this?