* [Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option
@ 2015-09-23 14:27 Kővágó, Zoltán
2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union Kővágó, Zoltán
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Kővágó, Zoltán @ 2015-09-23 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Eduardo Habkost
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union 2015-09-23 14:27 [Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option Kővágó, Zoltán @ 2015-09-23 14:27 ` Kővágó, Zoltán 2015-09-23 14:40 ` Eric Blake 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 2/3] qapi: change NetLegacy " Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 3/3] qapi: support nested structs in OptsVisitor Kővágó, Zoltán 2 siblings, 1 reply; 6+ messages in thread From: Kővágó, Zoltán @ 2015-09-23 14:27 UTC (permalink / raw) To: qemu-devel; +Cc: Eduardo Habkost, Markus Armbruster 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union Kővágó, Zoltán @ 2015-09-23 14:40 ` Eric Blake 2015-09-23 17:05 ` Kővágó Zoltán 0 siblings, 1 reply; 6+ messages in thread From: Eric Blake @ 2015-09-23 14:40 UTC (permalink / raw) To: Kővágó, Zoltán, qemu-devel Cc: Eduardo Habkost, Markus Armbruster [-- 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 --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union 2015-09-23 14:40 ` Eric Blake @ 2015-09-23 17:05 ` Kővágó Zoltán 0 siblings, 0 replies; 6+ messages in thread From: Kővágó Zoltán @ 2015-09-23 17:05 UTC (permalink / raw) To: Eric Blake, qemu-devel; +Cc: Eduardo Habkost, Markus Armbruster 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 >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] qapi: change NetLegacy into a flat union 2015-09-23 14:27 [Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union Kővágó, Zoltán @ 2015-09-23 14:27 ` Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 3/3] qapi: support nested structs in OptsVisitor Kővágó, Zoltán 2 siblings, 0 replies; 6+ messages in thread From: Kővágó, Zoltán @ 2015-09-23 14:27 UTC (permalink / raw) To: qemu-devel; +Cc: Jason Wang, Markus Armbruster 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] qapi: support nested structs in OptsVisitor 2015-09-23 14:27 [Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 2/3] qapi: change NetLegacy " Kővágó, Zoltán @ 2015-09-23 14:27 ` Kővágó, Zoltán 2 siblings, 0 replies; 6+ messages in thread From: Kővágó, Zoltán @ 2015-09-23 14:27 UTC (permalink / raw) To: qemu-devel; +Cc: Markus Armbruster, Michael Roth 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-23 17:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-23 14:27 [Qemu-devel] [PATCH v2 0/3] qapi-flattening and preparation of -audiodev option Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 1/3] qapi: convert NumaOptions into a flat union Kővágó, Zoltán 2015-09-23 14:40 ` Eric Blake 2015-09-23 17:05 ` Kővágó Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 2/3] qapi: change NetLegacy " Kővágó, Zoltán 2015-09-23 14:27 ` [Qemu-devel] [PATCH v2 3/3] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).