[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat un

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union
Date: Tue, 8 Mar 2016 09:29:46 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 03/08/2016 09:23 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>> Rather than requiring all flat unions to explicitly create
>> a separate base struct, we can allow the qapi schema to specify
>> the common members via an inline dictionary. This is similar to
>> how commands can specify an inline anonymous type for its 'data'.
>> We already have several struct types that only exist to serve as
>> a single flat union's base; the next commit will clean them up.

> I think you also
> * fixed a missing allow_optional=True in check_union()

More on that below.

> * fixed a missing "non-optional" in qapi-code-gen.txt (mention in commit
>   message?  separate patch?)
> * renamed readonly to read-only in an example in qapi-code-gen.txt to be
>   closer to the code (separate patch?)

Could separate those two cleanups if it helps.

>> @@ -560,9 +562,10 @@ def check_union(expr, expr_info):
>>      # Else, it's a flat union.
>>      else:
>> -        # The object must have a string member 'base'.
>> +        # The object must have a string or dictionary 'base'.
>>          check_type(expr_info, "'base' for union '%s'" % name,
>> -                   base, allow_metas=['struct'])
>> +                   base, allow_dict=True, allow_optional=True,
>> +                   allow_metas=['struct'])
> This is where you added allow_optional=True.

allow_optional only matters if allow_dict is True.  We have places where
we allow a dict but do not allow optionals (namely, simple unions); but
where we are creating an anonymous type, we want to allow optionals at
the same time we allow a dict.  I think this is just a case where the
commit message needs to be beefed up.

>> +A flat union definition avoids nesting on the wire, and specifies a
>> +set of common members that occur in all variants of the union.  The
>> +'base' key must specifiy either a type name (the type must be a
>> +struct, not a union), or a dictionary representing an anonymous type.
>> +All branches of the union must be complex types, and the top-level
>> +members of the union dictionary on the wire will be combination of
>> +members from both the base type and the appropriate branch type (when
>> +merging two dictionaries, there must be no keys in common).  The
>> +'discriminator' member must be the name of a non-optional enum-typed
> This is where you supplied the missing "non-optional".
>> +member of the base struct.
> And below, you rename readonly to read-only.

They touch the same paragraph, but I can separate them if it would make
this patch feel cleaner.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]