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.

Make the conversion to the new layout for block-related code.

Signed-off-by: Eric Blake <ebl...@redhat.com>

---
v10: no change
v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
---
 block/nbd.c   | 18 +++++++++---------
 block/qcow2.c | 10 ++++------
 block/vmdk.c  |  6 +++---
 blockdev.c    | 47 ++++++++++++++++++++++++-----------------------
 4 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index c2a87e9..cd6a587 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -206,24 +206,24 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, char **export,
     saddr = g_new0(SocketAddress, 1);

     if (qdict_haskey(options, "path")) {
-        saddr->kind = SOCKET_ADDRESS_KIND_UNIX;
-        saddr->q_unix = g_new0(UnixSocketAddress, 1);
-        saddr->q_unix->path = g_strdup(qdict_get_str(options, "path"));
+        saddr->type = SOCKET_ADDRESS_KIND_UNIX;
+        saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
+        saddr->u.q_unix->path = g_strdup(qdict_get_str(options, "path"));
         qdict_del(options, "path");
     } else {
-        saddr->kind = SOCKET_ADDRESS_KIND_INET;
-        saddr->inet = g_new0(InetSocketAddress, 1);
-        saddr->inet->host = g_strdup(qdict_get_str(options, "host"));
+        saddr->type = SOCKET_ADDRESS_KIND_INET;
+        saddr->u.inet = g_new0(InetSocketAddress, 1);
+        saddr->u.inet->host = g_strdup(qdict_get_str(options, "host"));
         if (!qdict_get_try_str(options, "port")) {
-            saddr->inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
+            saddr->u.inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
         } else {
-            saddr->inet->port = g_strdup(qdict_get_str(options, "port"));
+            saddr->u.inet->port = g_strdup(qdict_get_str(options, "port"));
         }
         qdict_del(options, "host");
         qdict_del(options, "port");
     }

-    s->client.is_unix = saddr->kind == SOCKET_ADDRESS_KIND_UNIX;
+    s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;

     *export = g_strdup(qdict_get_try_str(options, "export"));
     if (*export) {
diff --git a/block/qcow2.c b/block/qcow2.c
index bacc4f2..88f56c8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2738,18 +2738,16 @@ static ImageInfoSpecific 
*qcow2_get_specific_info(BlockDriverState *bs)
     ImageInfoSpecific *spec_info = g_new(ImageInfoSpecific, 1);

     *spec_info = (ImageInfoSpecific){
-        .kind  = IMAGE_INFO_SPECIFIC_KIND_QCOW2,
-        {
-            .qcow2 = g_new(ImageInfoSpecificQCow2, 1),
-        },
+        .type  = IMAGE_INFO_SPECIFIC_KIND_QCOW2,
+        .u.qcow2 = g_new(ImageInfoSpecificQCow2, 1),
     };
     if (s->qcow_version == 2) {
-        *spec_info->qcow2 = (ImageInfoSpecificQCow2){
+        *spec_info->u.qcow2 = (ImageInfoSpecificQCow2){
             .compat             = g_strdup("0.10"),
             .refcount_bits      = s->refcount_bits,
         };
     } else if (s->qcow_version == 3) {
-        *spec_info->qcow2 = (ImageInfoSpecificQCow2){
+        *spec_info->u.qcow2 = (ImageInfoSpecificQCow2){
             .compat             = g_strdup("1.1"),
             .lazy_refcounts     = s->compatible_features &
                                   QCOW2_COMPAT_LAZY_REFCOUNTS,
diff --git a/block/vmdk.c b/block/vmdk.c
index 0effb7d..6f819e4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2161,19 +2161,19 @@ static ImageInfoSpecific 
*vmdk_get_specific_info(BlockDriverState *bs)
     ImageInfoList **next;

     *spec_info = (ImageInfoSpecific){
-        .kind = IMAGE_INFO_SPECIFIC_KIND_VMDK,
+        .type = IMAGE_INFO_SPECIFIC_KIND_VMDK,
         {
             .vmdk = g_new0(ImageInfoSpecificVmdk, 1),
         },
     };

-    *spec_info->vmdk = (ImageInfoSpecificVmdk) {
+    *spec_info->u.vmdk = (ImageInfoSpecificVmdk) {
         .create_type = g_strdup(s->create_type),
         .cid = s->cid,
         .parent_cid = s->parent_cid,
     };

-    next = &spec_info->vmdk->extents;
+    next = &spec_info->u.vmdk->extents;
     for (i = 0; i < s->num_extents; i++) {
         *next = g_new0(ImageInfoList, 1);
         (*next)->value = vmdk_get_extent_info(&s->extents[i]);
diff --git a/blockdev.c b/blockdev.c
index 8141b6b..0bcd3f1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1050,13 +1050,14 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
     }
 }

-static void blockdev_do_action(int kind, void *data, Error **errp)
+static void blockdev_do_action(TransactionActionKind type, void *data,
+                               Error **errp)
 {
     TransactionAction action;
     TransactionActionList list;

-    action.kind = kind;
-    action.data = data;
+    action.type = type;
+    action.u.data = data;
     list.value = &action;
     list.next = NULL;
     qmp_transaction(&list, errp);
@@ -1295,9 +1296,9 @@ static void internal_snapshot_prepare(BlkTransactionState 
*common,
     InternalSnapshotState *state;
     int ret1;

-    g_assert(common->action->kind ==
+    g_assert(common->action->type ==
              TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
-    internal = common->action->blockdev_snapshot_internal_sync;
+    internal = common->action->u.blockdev_snapshot_internal_sync;
     state = DO_UPCAST(InternalSnapshotState, common, common);

     /* 1. parse input */
@@ -1437,22 +1438,22 @@ static void 
external_snapshot_prepare(BlkTransactionState *common,
     TransactionAction *action = common->action;

     /* get parameters */
-    g_assert(action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC);
+    g_assert(action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC);

-    has_device = action->blockdev_snapshot_sync->has_device;
-    device = action->blockdev_snapshot_sync->device;
-    has_node_name = action->blockdev_snapshot_sync->has_node_name;
-    node_name = action->blockdev_snapshot_sync->node_name;
+    has_device = action->u.blockdev_snapshot_sync->has_device;
+    device = action->u.blockdev_snapshot_sync->device;
+    has_node_name = action->u.blockdev_snapshot_sync->has_node_name;
+    node_name = action->u.blockdev_snapshot_sync->node_name;
     has_snapshot_node_name =
-        action->blockdev_snapshot_sync->has_snapshot_node_name;
-    snapshot_node_name = action->blockdev_snapshot_sync->snapshot_node_name;
+        action->u.blockdev_snapshot_sync->has_snapshot_node_name;
+    snapshot_node_name = action->u.blockdev_snapshot_sync->snapshot_node_name;

-    new_image_file = action->blockdev_snapshot_sync->snapshot_file;
-    if (action->blockdev_snapshot_sync->has_format) {
-        format = action->blockdev_snapshot_sync->format;
+    new_image_file = action->u.blockdev_snapshot_sync->snapshot_file;
+    if (action->u.blockdev_snapshot_sync->has_format) {
+        format = action->u.blockdev_snapshot_sync->format;
     }
-    if (action->blockdev_snapshot_sync->has_mode) {
-        mode = action->blockdev_snapshot_sync->mode;
+    if (action->u.blockdev_snapshot_sync->has_mode) {
+        mode = action->u.blockdev_snapshot_sync->mode;
     }

     /* start processing */
@@ -1577,8 +1578,8 @@ static void drive_backup_prepare(BlkTransactionState 
*common, Error **errp)
     DriveBackup *backup;
     Error *local_err = NULL;

-    assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
-    backup = common->action->drive_backup;
+    assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
+    backup = common->action->u.drive_backup;

     blk = blk_by_name(backup->device);
     if (!blk) {
@@ -1645,8 +1646,8 @@ static void blockdev_backup_prepare(BlkTransactionState 
*common, Error **errp)
     BlockBackend *blk;
     Error *local_err = NULL;

-    assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
-    backup = common->action->blockdev_backup;
+    assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
+    backup = common->action->u.blockdev_backup;

     blk = blk_by_name(backup->device);
     if (!blk) {
@@ -1772,9 +1773,9 @@ void qmp_transaction(TransactionActionList *dev_list, 
Error **errp)
         dev_info = dev_entry->value;
         dev_entry = dev_entry->next;

-        assert(dev_info->kind < ARRAY_SIZE(actions));
+        assert(dev_info->type < ARRAY_SIZE(actions));

-        ops = &actions[dev_info->kind];
+        ops = &actions[dev_info->type];
         assert(ops->instance_size > 0);

         state = g_malloc0(ops->instance_size);
-- 
2.4.3


Reply via email to