[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers |
Date: |
Tue, 08 Mar 2016 16:59:53 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Simple unions were carrying a special case that hid their 'data'
> QMP member from the resulting C struct, via the hack method
> QAPISchemaObjectTypeVariant.simple_union_type(). But by using
> the work we started by unboxing flat union and alternate
> branches, coupled with the ability to visit the members of an
> implicit type, we can now expose the simple union's implicit
> type in qapi-types.h:
>
> | struct _obj_ImageInfoSpecificQCow2_wrapper {
> | ImageInfoSpecificQCow2 *data;
> | };
> |
> | struct _obj_ImageInfoSpecificVmdk_wrapper {
> | ImageInfoSpecificVmdk *data;
> | };
> ...
> | struct ImageInfoSpecific {
> | ImageInfoSpecificKind type;
> | union { /* union tag is @type */
> | void *data;
> |- ImageInfoSpecificQCow2 *qcow2;
> |- ImageInfoSpecificVmdk *vmdk;
> |+ _obj_ImageInfoSpecificQCow2_wrapper qcow2;
> |+ _obj_ImageInfoSpecificVmdk_wrapper vmdk;
> | } u;
> | };
>
> Doing this removes asymmetry between QAPI's QMP side and its
> C side (both sides now expose 'data'), and means that the
> treatment of a simple union as sugar for a flat union is now
> equivalent in both languages (previously the two approaches used
> a different layer of dereferencing, where the simple union could
> be converted to a flat union with equivalent C layout but
> different {} on the wire, or to an equivalent QMP wire form
> but with different C representation). Using the implicit type
> also lets us get rid of the simple_union_type() hack.
>
> Of course, now all clients of simple unions have to adjust from
> using su->u.member to using su->u.member.data; while this touches
> a number of files in the tree, some earlier cleanup patches
> helped minimize the change to the initialization of a temporary
> variable rather than every single member access. The generated
> qapi-visit.c code is also affected by the layout change:
>
> |@@ -7393,10 +7393,10 @@ void visit_type_ImageInfoSpecific_member
> | }
> | switch (obj->type) {
> | case IMAGE_INFO_SPECIFIC_KIND_QCOW2:
> |- visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err);
> |+ visit_type__obj_ImageInfoSpecificQCow2_wrapper_members(v,
> &obj->u.qcow2, &err);
> | break;
> | case IMAGE_INFO_SPECIFIC_KIND_VMDK:
> |- visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err);
> |+ visit_type__obj_ImageInfoSpecificVmdk_wrapper_members(v,
> &obj->u.vmdk, &err);
> | break;
> | default:
> | abort();
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v4: improve commit message, rebase onto exposed implicit types
> [no v3]
> v2: rebase onto s/fields/members/ change, changes in master; pick
> up missing net/ files
> ---
> scripts/qapi.py | 11 ------
> scripts/qapi-types.py | 8 +---
> scripts/qapi-visit.py | 22 ++---------
> backends/baum.c | 2 +-
> backends/msmouse.c | 2 +-
> block/nbd.c | 6 +--
> block/qcow2.c | 6 +--
> block/vmdk.c | 8 ++--
> blockdev.c | 24 ++++++------
> hmp.c | 8 ++--
> hw/char/escc.c | 2 +-
> hw/input/hid.c | 8 ++--
> hw/input/ps2.c | 6 +--
> hw/input/virtio-input-hid.c | 8 ++--
> hw/mem/pc-dimm.c | 2 +-
> net/dump.c | 2 +-
> net/hub.c | 2 +-
> net/l2tpv3.c | 2 +-
> net/net.c | 4 +-
> net/netmap.c | 2 +-
> net/slirp.c | 2 +-
> net/socket.c | 2 +-
> net/tap-win32.c | 2 +-
> net/tap.c | 4 +-
> net/vde.c | 2 +-
> net/vhost-user.c | 2 +-
> numa.c | 4 +-
> qemu-char.c | 82
> +++++++++++++++++++++--------------------
> qemu-nbd.c | 6 +--
> replay/replay-input.c | 44 +++++++++++-----------
> spice-qemu-char.c | 14 ++++---
> tests/test-io-channel-socket.c | 40 ++++++++++----------
> tests/test-qmp-commands.c | 2 +-
> tests/test-qmp-input-visitor.c | 25 +++++++------
> tests/test-qmp-output-visitor.c | 24 ++++++------
> tpm.c | 2 +-
> ui/console.c | 4 +-
> ui/input-keymap.c | 10 ++---
> ui/input-legacy.c | 8 ++--
> ui/input.c | 34 ++++++++---------
> ui/vnc-auth-sasl.c | 3 +-
> ui/vnc.c | 29 ++++++++-------
> util/qemu-sockets.c | 35 +++++++++---------
> 43 files changed, 246 insertions(+), 269 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 797ac7a..0574b8b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1006,7 +1006,6 @@ class QAPISchemaObjectType(QAPISchemaType):
> return c_name(self.name) + pointer_suffix
>
> def c_unboxed_type(self):
> - assert not self.is_implicit()
Doesn't this belong into PATCH 04?
> return c_name(self.name)
>
> def json_type(self):
> @@ -1126,16 +1125,6 @@ class
> QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
> def __init__(self, name, typ):
> QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
>
> - # This function exists to support ugly simple union special cases
> - # TODO get rid of them, and drop the function
> - def simple_union_type(self):
> - if (self.type.is_implicit() and
> - isinstance(self.type, QAPISchemaObjectType)):
> - assert len(self.type.members) == 1
> - assert not self.type.variants
> - return self.type.members[0].type
> - return None
> -
>
> class QAPISchemaAlternateType(QAPISchemaType):
> def __init__(self, name, info, variants):
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index fa2d6af..b8499ac 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -121,16 +121,10 @@ def gen_variants(variants):
> c_name=c_name(variants.tag_member.name))
>
> for var in variants.variants:
> - # Ugly special case for simple union TODO get rid of it
> - simple_union_type = var.simple_union_type()
> - if simple_union_type:
> - typ = simple_union_type.c_type()
> - else:
> - typ = var.type.c_unboxed_type()
> ret += mcgen('''
> %(c_type)s %(c_name)s;
> ''',
> - c_type=typ,
> + c_type=var.type.c_unboxed_type(),
> c_name=c_name(var.name))
>
> ret += mcgen('''
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index b4303a7..afbaeeb 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -60,29 +60,15 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s
> *obj, Error **errp)
> c_name=c_name(variants.tag_member.name))
>
> for var in variants.variants:
> - # TODO ugly special case for simple union
> - simple_union_type = var.simple_union_type()
> ret += mcgen('''
> case %(case)s:
> + visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, &err);
> + break;
> ''',
> case=c_enum_const(variants.tag_member.type.name,
> var.name,
> - variants.tag_member.type.prefix))
> - if simple_union_type:
> - ret += mcgen('''
> - visit_type_%(c_type)s(v, "data", &obj->u.%(c_name)s, &err);
> -''',
> - c_type=simple_union_type.c_name(),
> - c_name=c_name(var.name))
> - else:
> - ret += mcgen('''
> - visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, &err);
> -''',
> - c_type=var.type.c_name(),
> - c_name=c_name(var.name))
> - ret += mcgen('''
> - break;
> -''')
> + variants.tag_member.type.prefix),
> + c_type=var.type.c_name(), c_name=c_name(var.name))
>
> ret += mcgen('''
> default:
This stupid special case has given us enough trouble, good to see it
gone!
[Tedious part snipped, it looks good to me...]
[Qemu-devel] [PATCH v4 09/10] qapi: Use anonymous bases in QMP flat unions, Eric Blake, 2016/03/05
[Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers, Eric Blake, 2016/03/05
- Re: [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers,
Markus Armbruster <=