[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 22/24] qapi: Finish converting to new qapi u
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v11 22/24] qapi: Finish converting to new qapi union layout |
Date: |
Tue, 27 Oct 2015 08:33:29 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 10/26/2015 04:35 PM, Eric Blake wrote:
> We have two issues with our qapi union layout:
> 1) Even though the QMP wire format spells the tag 'type', the
> C code spells it 'kind', requiring some hacks in the generator.
> 2) The C struct uses an anonymous union, which places all tag
> values in the same namespace as all non-variant members. This
> leads to spurious collisions if a tag value matches a QMP name.
You asked for an updated commit message (but that request was made
against v10:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06216.html).
There, you suggested "a non-variant member name" rather than "QMP name"
- it works for me, but you'd want to make that change for all of
13-22/25 (since I copy-pasted the same intro to each). Or decide which
ones you want to squash together.
For your other comment in that mail (mentioning an example test), I
think I already got close to what you asked for, but have one tweak below:
>
> This patch is the back end for a series that converts to a
> saner qapi union layout. Now that all clients have been
> converted to use 'type' and 'obj->u.value', we can drop the
> temporary parallel support for 'kind' and 'obj->value'.
>
> Given a simple union qapi type:
>
> { 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } }
>
> this is the overall effect, when compared to the state before
> this series of patches:
>
> | struct Foo {
> |- FooKind kind;
> |- union { /* union tag is @kind */
> |+ FooKind type;
> |+ union { /* union tag is @type */
> | void *data;
> | int64_t a;
> | bool b;
> |- };
> |+ } u;
> | };
>
> There are still some further changes that can be made to the
> testsuite now that there is no longer a collision between
> enum tag values and QMP names, as well as adding a reservation
> of 'u' to avoid a collision between QMP and our choice of union
> naming, but those will be later patches.
Change this paragraph to something along these lines:
The testsuite still contains some examples of artificial restrictions
(see flat-union-clash-type.json, for example) that are no longer
technically necessary, now that there is no longer a collision between
enum tag values and non-variant member names; but fixing this will be
done in later patches, in part because some further changes are required
to keep QAPISchema*.check() from asserting. Also, a later patch will
add a reservation for the member name 'u' to avoid a collision between a
user's non-variant names and our internal choice of C union name.
>
> Note, however, that we do not rename the generated enum, which
> is still 'FooKind'. A further patch could generate implicit
> enums as 'FooType', but while the generator already reserved
> the '*Kind' namespace (commit 4dc2e69), there are already QMP
> constructs with '*Type' naming, which means changing our
> reservation namespace would have lots of churn to C code to
> deal with a forced name change.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v11 15/24] block: Convert to new qapi union layout, Eric Blake, 2015/10/26
[Qemu-devel] [PATCH v11 03/24] qapi: More robust conditions for when labels are needed, Eric Blake, 2015/10/26
[Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name, Eric Blake, 2015/10/26
[Qemu-devel] [PATCH v11 16/24] sockets: Convert to new qapi union layout, Eric Blake, 2015/10/26