Re: [Qemu-devel] [PATCH 12/17] qapi: Don't special-case simple union wrappers

[Daniel P. Berrange]

On Fri, Feb 19, 2016 at 05:19:42PM -0700, Eric Blake wrote:
> 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 using the
> work we started by unboxing flat union and alternate branches, we
> expose the simple union's implicit type in qapi-types.h as an
> anonymous type, and drop our last use of the hack.
> 
> | struct ImageInfoSpecific {
> |     ImageInfoSpecificKind type;
> |     union { /* union tag is @type */
> |         void *data;
> |-        ImageInfoSpecificQCow2 *qcow2;
> |-        ImageInfoSpecificVmdk *vmdk;
> |+        struct {
> |+            ImageInfoSpecificQCow2 *data;
> |+        } qcow2;
> |+        struct {
> |+            ImageInfoSpecificVmdk *data;
> |+        } vmdk;
> |     } u;
> | };
> 
> All clients of simple unions have to adjust from using su->u.member
> to now 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 field access.  The generated qapi-visit.c code
> is included in the files affected by the layout change, with a
> diff that looks like:
> 
> |@@ -4510,10 +4567,16 @@ static void visit_type_ImageInfoSpecific
> |     }
> |     switch (obj->type) {
> |     case IMAGE_INFO_SPECIFIC_KIND_QCOW2:
> |-        visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err);
> |+        visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2.data, 
> &err);
> |+        if (err) {
> |+            goto out;
> |+        }
> |         break;
> |     case IMAGE_INFO_SPECIFIC_KIND_VMDK:
> |-        visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err);
> |+        visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk.data, 
> &err);
> |+        if (err) {
> |+            goto out;
> |+        }
> |         break;
> |     default:
> |         abort();
> |     }
> |
> | out:
> |     error_propagate(errp, err);
> 
> The added error checks there are a side effect of visiting all
> fields of each implicit struct (there is only one such field for
> simple unions); but do not change semantics, and will be important
> when later patches allow for flat unions with anonymous branches
> with more than one field.  That future work will look like:
> { 'union': 'Foo', 'base': 'Base', 'discriminator': 'type',
>   'data': { 'branch1': { 'anonymous': 'str', 'number': 'int' },
>             'branch2': 'Named' } }
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>  scripts/qapi.py                 | 10 -----
>  scripts/qapi-types.py           | 22 ++++++++---
>  scripts/qapi-visit.py           | 15 +++-----
>  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                |  4 +-
>  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.c                       |  4 +-
>  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 +++++++++---------
>  41 files changed, 262 insertions(+), 257 deletions(-)

Reviewed-by: Daniel P. Berrange <address@hidden>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|