[Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option

[Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option

[Kővágó, Zoltán]

Here is the second version of my qapi flattening attempts.  This is now
based on Eric Blake's "post-introspection cleanups, and qapi-ify
netdev_add" patch series[1], which contains some of my previous commits.

What remains here: NumaOptions flattening (with proposed documentation
changes from Eduardo) and OptsVisitor.  The Netdev part was mostly taken
care of by Eric, I only had to convert NetLegacy into a flat union.

Please review.

[1]: http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg05410.html

Kővágó, Zoltán (3):
  qapi: convert NumaOptions into a flat union
  qapi: change NetLegacy into a flat union
  qapi: support nested structs in OptsVisitor

 net/net.c                               |  47 +++++++------
 numa.c                                  |   2 +-
 qapi-schema.json                        |  78 +++++++++++++++------
 qapi/opts-visitor.c                     | 116 ++++++++++++++++++++++++++------
 tests/qapi-schema/qapi-schema-test.json |   9 ++-
 tests/qapi-schema/qapi-schema-test.out  |   4 ++
 tests/test-opts-visitor.c               |  34 ++++++++++
 7 files changed, 224 insertions(+), 66 deletions(-)

-- 
2.5.2

[Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union

[Kővágó, Zoltán]

Changes the NumaOptions to flat union from a simple one.  This is
required by my later OptsVisitor patch to preserve backward
compatibility.

Strictly speaking this would break QMP compatibility (as specified in
docs/qapi-code-gen.txt), but since no QMP command use this structure,
it's not an issue.  The -numa option syntax doesn't change.  There are
some changes in the C api, but this patch fixes them.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

---

Changes from v1:
* fixed documentation

 numa.c           |  2 +-
 qapi-schema.json | 49 ++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/numa.c b/numa.c
index 16a8c41..9cd0c84 100644
--- a/numa.c
+++ b/numa.c
@@ -227,7 +227,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
     }
 
     switch (object->type) {
-    case NUMA_OPTIONS_KIND_NODE:
+    case NUMA_OPTION_TYPE_NODE:
         numa_node_parse(object->node, opts, &err);
         if (err) {
             goto error;
diff --git a/qapi-schema.json b/qapi-schema.json
index 263053d..72827f8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3573,17 +3573,6 @@
   'data': { '*console':'int', 'events': [ 'InputEvent' ] } }
 
 ##
-# @NumaOptions
-#
-# A discriminated record of NUMA options. (for OptsVisitor)
-#
-# Since 2.1
-##
-{ 'union': 'NumaOptions',
-  'data': {
-    'node': 'NumaNodeOptions' }}
-
-##
 # @NumaNodeOptions
 #
 # Create a guest NUMA node. (for OptsVisitor)
@@ -3610,6 +3599,44 @@
    '*memdev': 'str' }}
 
 ##
+# @NumaOptionType
+#
+# NUMA command-line option type.
+#
+# @node: Create a guest NUMA node. See @NumaNodeOptions.
+#
+# Since: 2.5
+##
+{ 'enum': 'NumaOptionType',
+  'data': [ 'node' ] }
+
+##
+# @NumaCommonOptions
+#
+# Common set of numa options.
+#
+# @type: NUMA command-line option type.
+#
+# Since: 2.5
+##
+{ 'struct': 'NumaCommonOptions',
+  'data': {
+    'type': 'NumaOptionType' } }
+
+##
+# @NumaOptions
+#
+# A discriminated record of NUMA options. (for OptsVisitor)
+#
+# Since 2.1
+##
+{ 'union': 'NumaOptions',
+  'base': 'NumaCommonOptions',
+  'discriminator': 'type',
+  'data': {
+    'node': 'NumaNodeOptions' }}
+
+##
 # @HostMemPolicy
 #
 # Host memory policy types
-- 
2.5.2

[Qemu-devel] [PATCH v2 2/3] qapi: change NetLegacy into a flat union

[Kővágó, Zoltán]

This is required to keep backward compatibility while applying the
proposed changes to OptsVisitor in the next commit.

Strictly speaking this change breaks QMP compatibility, but since this
struct is only used by the (legacy) -net option, it's not a problem.
(The command line syntax doesn't change.)

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 net/net.c        | 47 +++++++++++++++++++++++------------------------
 qapi-schema.json | 29 +++++++++++++++++++++--------
 2 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/net/net.c b/net/net.c
index 69456b2..8ae1cd8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -931,55 +931,54 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
         }
     } else {
         const NetLegacy *net = object;
-        const NetClientOptions *opts = net->opts;
         legacy.id = net->id;
         netdev = &legacy;
         /* missing optional values have been initialized to "all bits zero" */
         name = net->has_id ? net->id : net->name;
 
         /* Map the old options to the new flat type */
-        switch (opts->type) {
-        case NET_CLIENT_OPTIONS_KIND_NONE:
+        switch (net->type) {
+        case NET_CLIENT_DRIVER_LEGACY_NONE:
             return 0; /* nothing to do */
-        case NET_CLIENT_OPTIONS_KIND_NIC:
+        case NET_CLIENT_DRIVER_LEGACY_NIC:
             legacy.type = NET_CLIENT_DRIVER_NIC;
-            legacy.nic = opts->nic;
+            legacy.nic = net->nic;
             break;
-        case NET_CLIENT_OPTIONS_KIND_USER:
+        case NET_CLIENT_DRIVER_LEGACY_USER:
             legacy.type = NET_CLIENT_DRIVER_USER;
-            legacy.user = opts->user;
+            legacy.user = net->user;
             break;
-        case NET_CLIENT_OPTIONS_KIND_TAP:
+        case NET_CLIENT_DRIVER_LEGACY_TAP:
             legacy.type = NET_CLIENT_DRIVER_TAP;
-            legacy.tap = opts->tap;
+            legacy.tap = net->tap;
             break;
-        case NET_CLIENT_OPTIONS_KIND_L2TPV3:
+        case NET_CLIENT_DRIVER_LEGACY_L2TPV3:
             legacy.type = NET_CLIENT_DRIVER_L2TPV3;
-            legacy.l2tpv3 = opts->l2tpv3;
+            legacy.l2tpv3 = net->l2tpv3;
             break;
-        case NET_CLIENT_OPTIONS_KIND_SOCKET:
+        case NET_CLIENT_DRIVER_LEGACY_SOCKET:
             legacy.type = NET_CLIENT_DRIVER_SOCKET;
-            legacy.socket = opts->socket;
+            legacy.socket = net->socket;
             break;
-        case NET_CLIENT_OPTIONS_KIND_VDE:
+        case NET_CLIENT_DRIVER_LEGACY_VDE:
             legacy.type = NET_CLIENT_DRIVER_VDE;
-            legacy.vde = opts->vde;
+            legacy.vde = net->vde;
             break;
-        case NET_CLIENT_OPTIONS_KIND_DUMP:
+        case NET_CLIENT_DRIVER_LEGACY_DUMP:
             legacy.type = NET_CLIENT_DRIVER_DUMP;
-            legacy.dump = opts->dump;
+            legacy.dump = net->dump;
             break;
-        case NET_CLIENT_OPTIONS_KIND_BRIDGE:
+        case NET_CLIENT_DRIVER_LEGACY_BRIDGE:
             legacy.type = NET_CLIENT_DRIVER_BRIDGE;
-            legacy.bridge = opts->bridge;
+            legacy.bridge = net->bridge;
             break;
-        case NET_CLIENT_OPTIONS_KIND_NETMAP:
+        case NET_CLIENT_DRIVER_LEGACY_NETMAP:
             legacy.type = NET_CLIENT_DRIVER_NETMAP;
-            legacy.netmap = opts->netmap;
+            legacy.netmap = net->netmap;
             break;
-        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
+        case NET_CLIENT_DRIVER_LEGACY_VHOST_USER:
             legacy.type = NET_CLIENT_DRIVER_VHOST_USER;
-            legacy.vhost_user = opts->vhost_user;
+            legacy.vhost_user = net->vhost_user;
             break;
         default:
             abort();
@@ -994,7 +993,7 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
 
         /* Do not add to a vlan if it's a nic with a netdev= parameter. */
         if (netdev->type != NET_CLIENT_DRIVER_NIC ||
-            !opts->nic->has_netdev) {
+            !net->nic->has_netdev) {
             peer = net_hub_add_port(net->has_vlan ? net->vlan : 0, NULL);
         }
     }
diff --git a/qapi-schema.json b/qapi-schema.json
index 72827f8..4f9946e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2531,9 +2531,9 @@
     'vhost-user': 'NetdevVhostUserOptions' } }
 
 ##
-# @NetLegacy
+# @NetLegacyBase
 #
-# Captures the configuration of a network device; legacy.
+# Captures the common configuration of a network device, legacy.
 #
 # @vlan: #optional vlan number
 #
@@ -2541,25 +2541,38 @@
 #
 # @name: #optional identifier for monitor commands, ignored if @id is present
 #
-# @opts: device type specific properties (legacy)
+# @type: specify the driver used for interpreting remaining arguments.
 #
 # Since 1.2
 ##
-{ 'struct': 'NetLegacy',
+{ 'struct': 'NetLegacyBase',
   'data': {
     '*vlan': 'int32',
     '*id':   'str',
     '*name': 'str',
-    'opts':  'NetClientOptions' } }
+    'type':  'NetClientDriverLegacy' } }
 
 ##
-# @NetClientOptions
+# @NetClientDriverLegacy
 #
-# Like Netdev, but for use only by the legacy command line options
+# Available netdev drivers, legacy.
+#
+# Since 2.5
+##
+{'enum': 'NetClientDriverLegacy',
+  'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
+            'bridge', 'netmap', 'vhost-user' ] }
+
+##
+# @NetLegacy
+#
+# Captures the configuration of a network device; legacy.
 #
 # Since 1.2
 ##
-{ 'union': 'NetClientOptions',
+{ 'union': 'NetLegacy',
+  'base': 'NetLegacyBase',
+  'discriminator': 'type',
   'data': {
     'none':     'NetdevNoneOptions',
     'nic':      'NetLegacyNicOptions',
-- 
2.5.2

[Qemu-devel] [PATCH v2 3/3] qapi: support nested structs in OptsVisitor

[Kővágó, Zoltán]

The current OptsVisitor flattens the whole structure, if there are same
named fields under different paths (like `in' and `out' in `Audiodev'),
the current visitor can't cope with them (for example setting
`frequency=44100' will set the in's frequency to 44100 and leave out's
frequency unspecified).

This patch fixes it, by always requiring a complete path in case of
nested structs.  Fields in the path are separated by dots, similar to C
structs (without pointers), like `in.frequency' or `out.frequency'.

You must provide a full path even in non-ambiguous cases.  The qapi
flattening commits for Numa and Netdev and NetLegacy ensures that this
change doesn't create backward compatibility problems.  (Previously
OptsVisitor didn't bother with structs and only consulted field names,
so something like `data.helper' was invalid previously, and stays so,
since unions are flattened now.)

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 qapi/opts-visitor.c                     | 116 ++++++++++++++++++++++++++------
 tests/qapi-schema/qapi-schema-test.json |   9 ++-
 tests/qapi-schema/qapi-schema-test.out  |   4 ++
 tests/test-opts-visitor.c               |  34 ++++++++++
 4 files changed, 141 insertions(+), 22 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 900b2fa..70dbaf0 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -71,6 +71,7 @@ struct OptsVisitor
      * schema, with a single mandatory scalar member. */
     ListMode list_mode;
     GQueue *repeated_opts;
+    char *repeated_name;
 
     /* When parsing a list of repeating options as integers, values of the form
      * "a-b", representing a closed interval, are allowed. Elements in the
@@ -86,6 +87,9 @@ struct OptsVisitor
      * not survive or escape the OptsVisitor object.
      */
     QemuOpt *fake_id_opt;
+
+    /* List of field names leading to the current structure. */
+    GQueue *nested_names;
 };
 
 
@@ -100,6 +104,7 @@ static void
 opts_visitor_insert(GHashTable *unprocessed_opts, const QemuOpt *opt)
 {
     GQueue *list;
+    assert(opt);
 
     list = g_hash_table_lookup(unprocessed_opts, opt->name);
     if (list == NULL) {
@@ -127,6 +132,9 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
     if (obj) {
         *obj = g_malloc0(size > 0 ? size : 1);
     }
+
+    g_queue_push_tail(ov->nested_names, (gpointer) name);
+
     if (ov->depth++ > 0) {
         return;
     }
@@ -169,6 +177,8 @@ opts_end_struct(Visitor *v, Error **errp)
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     GQueue *any;
 
+    g_queue_pop_tail(ov->nested_names);
+
     if (--ov->depth > 0) {
         return;
     }
@@ -198,15 +208,55 @@ opts_end_implicit_struct(Visitor *v, Error **errp)
 }
 
 
+static void
+sum_strlen(gpointer data, gpointer user_data)
+{
+    const char *str = data;
+    size_t *sum_len = user_data;
+
+    if (str) { /* skip NULLs */
+        *sum_len += strlen(str) + 1;
+    }
+}
+
+static void
+append_str(gpointer data, gpointer user_data)
+{
+    const char *str = data;
+    char *concat_str = user_data;
+
+    if (str) {
+        strcat(concat_str, str);
+        strcat(concat_str, ".");
+    }
+}
+
+/* lookup a name, using a fully qualified version */
 static GQueue *
-lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp)
+lookup_distinct(const OptsVisitor *ov, const char *name, char **out_key,
+                Error **errp)
 {
-    GQueue *list;
+    GQueue *list = NULL;
+    char *key;
+    size_t sum_len = strlen(name);
+
+    g_queue_foreach(ov->nested_names, sum_strlen, &sum_len);
+    key = g_malloc(sum_len+1);
+    key[0] = 0;
+    g_queue_foreach(ov->nested_names, append_str, key);
+    strcat(key, name);
+
+    list = g_hash_table_lookup(ov->unprocessed_opts, key);
+    if (list && out_key) {
+        *out_key = key;
+        key = NULL;
+    }
 
-    list = g_hash_table_lookup(ov->unprocessed_opts, name);
     if (!list) {
         error_setg(errp, QERR_MISSING_PARAMETER, name);
     }
+
+    g_free(key);
     return list;
 }
 
@@ -218,7 +268,8 @@ opts_start_list(Visitor *v, const char *name, Error **errp)
 
     /* we can't traverse a list in a list */
     assert(ov->list_mode == LM_NONE);
-    ov->repeated_opts = lookup_distinct(ov, name, errp);
+    assert(ov->repeated_opts == NULL && ov->repeated_name == NULL);
+    ov->repeated_opts = lookup_distinct(ov, name, &ov->repeated_name, errp);
     if (ov->repeated_opts != NULL) {
         ov->list_mode = LM_STARTED;
     }
@@ -254,11 +305,9 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
         /* range has been completed, fall through in order to pop option */
 
     case LM_IN_PROGRESS: {
-        const QemuOpt *opt;
-
-        opt = g_queue_pop_head(ov->repeated_opts);
+        g_queue_pop_head(ov->repeated_opts);
         if (g_queue_is_empty(ov->repeated_opts)) {
-            g_hash_table_remove(ov->unprocessed_opts, opt->name);
+            g_hash_table_remove(ov->unprocessed_opts, ov->repeated_name);
             return NULL;
         }
         link = &(*list)->next;
@@ -284,22 +333,28 @@ opts_end_list(Visitor *v, Error **errp)
            ov->list_mode == LM_SIGNED_INTERVAL ||
            ov->list_mode == LM_UNSIGNED_INTERVAL);
     ov->repeated_opts = NULL;
+
+    g_free(ov->repeated_name);
+    ov->repeated_name = NULL;
+
     ov->list_mode = LM_NONE;
 }
 
 
 static const QemuOpt *
-lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
+lookup_scalar(const OptsVisitor *ov, const char *name, char** out_key,
+              Error **errp)
 {
     if (ov->list_mode == LM_NONE) {
         GQueue *list;
 
         /* the last occurrence of any QemuOpt takes effect when queried by name
          */
-        list = lookup_distinct(ov, name, errp);
+        list = lookup_distinct(ov, name, out_key, errp);
         return list ? g_queue_peek_tail(list) : NULL;
     }
     assert(ov->list_mode == LM_IN_PROGRESS);
+    assert(out_key == NULL || *out_key == NULL);
     return g_queue_peek_head(ov->repeated_opts);
 }
 
@@ -321,13 +376,15 @@ opts_type_str(Visitor *v, char **obj, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     const QemuOpt *opt;
+    char *key = NULL;
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
     *obj = g_strdup(opt->str ? opt->str : "");
-    processed(ov, name);
+    processed(ov, key);
+    g_free(key);
 }
 
 
@@ -337,8 +394,9 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     const QemuOpt *opt;
+    char *key = NULL;
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -355,13 +413,15 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
         } else {
             error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                        "on|yes|y|off|no|n");
+            g_free(key);
             return;
         }
     } else {
         *obj = true;
     }
 
-    processed(ov, name);
+    processed(ov, key);
+    g_free(key);
 }
 
 
@@ -373,13 +433,14 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     const char *str;
     long long val;
     char *endptr;
+    char *key = NULL;
 
     if (ov->list_mode == LM_SIGNED_INTERVAL) {
         *obj = ov->range_next.s;
         return;
     }
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -393,11 +454,13 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) {
         if (*endptr == '\0') {
             *obj = val;
-            processed(ov, name);
+            processed(ov, key);
+            g_free(key);
             return;
         }
         if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
             long long val2;
+            assert(key == NULL);
 
             str = endptr + 1;
             val2 = strtoll(str, &endptr, 0);
@@ -418,6 +481,7 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                (ov->list_mode == LM_NONE) ? "an int64 value" :
                                             "an int64 value or range");
+    g_free(key);
 }
 
 
@@ -429,13 +493,14 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     const char *str;
     unsigned long long val;
     char *endptr;
+    char *key = NULL;
 
     if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
         *obj = ov->range_next.u;
         return;
     }
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -447,11 +512,13 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     if (parse_uint(str, &val, &endptr, 0) == 0 && val <= UINT64_MAX) {
         if (*endptr == '\0') {
             *obj = val;
-            processed(ov, name);
+            processed(ov, key);
+            g_free(key);
             return;
         }
         if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
             unsigned long long val2;
+            assert(key == NULL);
 
             str = endptr + 1;
             if (parse_uint_full(str, &val2, 0) == 0 &&
@@ -470,6 +537,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                (ov->list_mode == LM_NONE) ? "a uint64 value" :
                                             "a uint64 value or range");
+    g_free(key);
 }
 
 
@@ -480,8 +548,9 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     const QemuOpt *opt;
     int64_t val;
     char *endptr;
+    char *key = NULL;
 
-    opt = lookup_scalar(ov, name, errp);
+    opt = lookup_scalar(ov, name, &key, errp);
     if (!opt) {
         return;
     }
@@ -491,11 +560,13 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     if (val < 0 || *endptr) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
                    "a size value representible as a non-negative int64");
+        g_free(key);
         return;
     }
 
     *obj = val;
-    processed(ov, name);
+    processed(ov, key);
+    g_free(key);
 }
 
 
@@ -506,7 +577,7 @@ opts_optional(Visitor *v, bool *present, const char *name)
 
     /* we only support a single mandatory scalar field in a list node */
     assert(ov->list_mode == LM_NONE);
-    *present = (lookup_distinct(ov, name, NULL) != NULL);
+    *present = (lookup_distinct(ov, name, NULL, NULL) != NULL);
 }
 
 
@@ -517,6 +588,8 @@ opts_visitor_new(const QemuOpts *opts)
 
     ov = g_malloc0(sizeof *ov);
 
+    ov->nested_names = g_queue_new();
+
     ov->visitor.start_struct = &opts_start_struct;
     ov->visitor.end_struct   = &opts_end_struct;
 
@@ -560,6 +633,7 @@ opts_visitor_cleanup(OptsVisitor *ov)
     if (ov->unprocessed_opts != NULL) {
         g_hash_table_destroy(ov->unprocessed_opts);
     }
+    g_queue_free(ov->nested_names);
     g_free(ov->fake_id_opt);
     g_free(ov);
 }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ad37013..3a650c7 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -106,6 +106,11 @@
 { 'command': 'boxed', 'box': true, 'data': 'UserDefZero' }
 { 'command': 'boxed2', 'data': 'UserDefNativeListUnion', 'box': true }
 
+# For testing hierarchy support in opts-visitor
+{ 'struct': 'UserDefOptionsSub',
+  'data': {
+    '*nint': 'int' } }
+
 # For testing integer range flattening in opts-visitor. The following schema
 # corresponds to the option format:
 #
@@ -119,7 +124,9 @@
     '*u64' : [ 'uint64' ],
     '*u16' : [ 'uint16' ],
     '*i64x':   'int'     ,
-    '*u64x':   'uint64'  } }
+    '*u64x':   'uint64'  ,
+    'sub0':    'UserDefOptionsSub',
+    'sub1':    'UserDefOptionsSub' } }
 
 # testing event
 { 'struct': 'EventStructOne',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index b3b4cb8..83f0d69 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -148,6 +148,10 @@ object UserDefOptions
     member u16: uint16List optional=True
     member i64x: int optional=True
     member u64x: uint64 optional=True
+    member sub0: UserDefOptionsSub optional=False
+    member sub1: UserDefOptionsSub optional=False
+object UserDefOptionsSub
+    member nint: int optional=True
 object UserDefThree
     base UserDefOne
     member base: str optional=False
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 1c753d9..4393266 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -178,6 +178,34 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data)
     g_assert(f->userdef->u64->value == UINT64_MAX);
 }
 
+static void
+expect_both(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->sub0->has_nint);
+    g_assert(f->userdef->sub0->nint == 13);
+    g_assert(f->userdef->sub1->has_nint);
+    g_assert(f->userdef->sub1->nint == 17);
+}
+
+static void
+expect_sub0(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->sub0->has_nint);
+    g_assert(f->userdef->sub0->nint == 13);
+    g_assert(!f->userdef->sub1->has_nint);
+}
+
+static void
+expect_sub1(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(!f->userdef->sub0->has_nint);
+    g_assert(f->userdef->sub1->has_nint);
+    g_assert(f->userdef->sub1->nint == 13);
+}
+
 /* test cases */
 
 int
@@ -271,6 +299,12 @@ main(int argc, char **argv)
     add_test("/visitor/opts/i64/range/2big/full", &expect_fail,
              "i64=-0x8000000000000000-0x7fffffffffffffff");
 
+    /* Test nested structs support */
+    add_test("/visitor/opts/nested/unqualified", &expect_fail, "nint=13");
+    add_test("/visitor/opts/nested/both",        &expect_both,
+             "sub0.nint=13,sub1.nint=17");
+    add_test("/visitor/opts/nested/sub0",        &expect_sub0, "sub0.nint=13");
+    add_test("/visitor/opts/nested/sub1",        &expect_sub1, "sub1.nint=13");
     g_test_run();
     return 0;
 }
-- 
2.5.2

Re: [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]

On 09/23/2015 08:27 AM, Kővágó, Zoltán wrote:
> Changes the NumaOptions to flat union from a simple one.  This is
> required by my later OptsVisitor patch to preserve backward
> compatibility.
> 
> Strictly speaking this would break QMP compatibility (as specified in
> docs/qapi-code-gen.txt), but since no QMP command use this structure,
> it's not an issue.  The -numa option syntax doesn't change.  There are
> some changes in the C api, but this patch fixes them.
> 
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> ---
> 
> Changes from v1:
> * fixed documentation

Since you're basing this on top of my pending series, why not take
advantage of it...


> +##
> +# @NumaCommonOptions
> +#
> +# Common set of numa options.
> +#
> +# @type: NUMA command-line option type.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'NumaCommonOptions',
> +  'data': {
> +    'type': 'NumaOptionType' } }

...by dropping this type, and instead...

> +
> +##
> +# @NumaOptions
> +#

...document @type here, and...

> +# A discriminated record of NUMA options. (for OptsVisitor)
> +#
> +# Since 2.1
> +##
> +{ 'union': 'NumaOptions',
> +  'base': 'NumaCommonOptions',

...write this as 'base': { 'type': 'NumaOptionType' },

> +  'discriminator': 'type',
> +  'data': {
> +    'node': 'NumaNodeOptions' }}
> +
> +##
>  # @HostMemPolicy
>  #
>  # Host memory policy types
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union

[Kővágó Zoltán]

2015-09-23 16:40 keltezéssel, Eric Blake írta:
> On 09/23/2015 08:27 AM, Kővágó, Zoltán wrote:
>> Changes the NumaOptions to flat union from a simple one.  This is
>> required by my later OptsVisitor patch to preserve backward
>> compatibility.
>>
>> Strictly speaking this would break QMP compatibility (as specified in
>> docs/qapi-code-gen.txt), but since no QMP command use this structure,
>> it's not an issue.  The -numa option syntax doesn't change.  There are
>> some changes in the C api, but this patch fixes them.
>>
>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>> ---
>>
>> Changes from v1:
>> * fixed documentation
>
> Since you're basing this on top of my pending series, why not take
> advantage of it...

Oh, so now that's possible.  I was too lazy to read the changes you have 
made to qapi...

Then I suppose I should do the same thing with NetLegacy.

>
>
>> +##
>> +# @NumaCommonOptions
>> +#
>> +# Common set of numa options.
>> +#
>> +# @type: NUMA command-line option type.
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'NumaCommonOptions',
>> +  'data': {
>> +    'type': 'NumaOptionType' } }
>
> ...by dropping this type, and instead...
>
>> +
>> +##
>> +# @NumaOptions
>> +#
>
> ...document @type here, and...
>
>> +# A discriminated record of NUMA options. (for OptsVisitor)
>> +#
>> +# Since 2.1
>> +##
>> +{ 'union': 'NumaOptions',
>> +  'base': 'NumaCommonOptions',
>
> ...write this as 'base': { 'type': 'NumaOptionType' },
>
>> +  'discriminator': 'type',
>> +  'data': {
>> +    'node': 'NumaNodeOptions' }}
>> +
>> +##
>>   # @HostMemPolicy
>>   #
>>   # Host memory policy types
>>
>