From: "Kővágó, Zoltán" <dirty.ice.hu@gmail.com>
To: qemu-devel@nongnu.org
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>,
Gerd Hoffmann <kraxel@redhat.com>,
Markus Armbruster <armbru@redhat.com>
Subject: [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor
Date: Tue, 16 Jun 2015 14:49:05 +0200 [thread overview]
Message-ID: <88a1cc0775d3d4f5262b31b9452f8acccc6bbb41.1434458391.git.DirtY.iCE.hu@gmail.com> (raw)
In-Reply-To: <cover.1434458391.git.DirtY.iCE.hu@gmail.com>
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 the following changes:
1) Specifying just the field name will apply to all fields that has the
specified name (this means it would set both in's and out's frequency to
44100 in the above example).
2) Optionally user can specify the path in the hierarchy. Names are separated by
a dot (e.g. `in.frequency', `foo.bar.something', etc). The user need not
specify the whole path, only the last few components (i.e. `bar.something' is
equivalent to `foo.bar.something' if only `foo' has a `bar' field). This way
1) is just a special case of this when only the last component is specified.
3) In case of an ambiguity (e.g `frequency=44100,in.frequency=8000') the longest
matching (the most specific) path wins (so in this example, in's frequency
would become 8000, because `in.frequency' is more specific that `frequency',
and out's frequency would become 44100, because only `frequency' matches it).
Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
qapi/opts-visitor.c | 144 +++++++++++++++++++++++++-------
tests/qapi-schema/qapi-schema-test.json | 9 +-
tests/test-opts-visitor.c | 34 ++++++++
3 files changed, 157 insertions(+), 30 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index f2ad6d7..409d8b7 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -64,13 +64,14 @@ struct OptsVisitor
/* Non-null iff depth is positive. Each key is a QemuOpt name. Each value
* is a non-empty GQueue, enumerating all QemuOpt occurrences with that
* name. */
- GHashTable *unprocessed_opts;
+ GHashTable *unprocessed_opts, *opts;
/* The list currently being traversed with opts_start_list() /
* opts_next_list(). The list must have a struct element type in the
* 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;
};
@@ -97,11 +101,12 @@ destroy_list(gpointer list)
static void
-opts_visitor_insert(GHashTable *unprocessed_opts, const QemuOpt *opt)
+opts_visitor_insert(OptsVisitor *ov, const QemuOpt *opt)
{
GQueue *list;
+ assert(opt);
- list = g_hash_table_lookup(unprocessed_opts, opt->name);
+ list = g_hash_table_lookup(ov->opts, opt->name);
if (list == NULL) {
list = g_queue_new();
@@ -109,7 +114,8 @@ opts_visitor_insert(GHashTable *unprocessed_opts, const QemuOpt *opt)
* "key_destroy_func" in opts_start_struct(). Thus cast away key
* const-ness in order to suppress gcc's warning.
*/
- g_hash_table_insert(unprocessed_opts, (gpointer)opt->name, list);
+ g_hash_table_insert(ov->opts, (gpointer)opt->name, list);
+ g_hash_table_insert(ov->unprocessed_opts, (gpointer)opt->name, list);
}
/* Similarly, destroy_list() doesn't call g_queue_free_full(). */
@@ -127,17 +133,27 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
if (obj) {
*obj = g_malloc0(size > 0 ? size : 1);
}
+
+ /* assuming name is a statically allocated string (or at least it's lifetime
+ * is longer than the visitor's) */
+ if (!name) {
+ name = "";
+ }
+ g_queue_push_tail(ov->nested_names, (gpointer) name);
+
if (ov->depth++ > 0) {
return;
}
- ov->unprocessed_opts = g_hash_table_new_full(&g_str_hash, &g_str_equal,
- NULL, &destroy_list);
+ ov->opts = g_hash_table_new_full(&g_str_hash, &g_str_equal,
+ NULL, &destroy_list);
+ ov->unprocessed_opts = g_hash_table_new(&g_str_hash, &g_str_equal);
+
QTAILQ_FOREACH(opt, &ov->opts_root->head, next) {
/* ensured by qemu-option.c::opts_do_parse() */
assert(strcmp(opt->name, "id") != 0);
- opts_visitor_insert(ov->unprocessed_opts, opt);
+ opts_visitor_insert(ov, opt);
}
if (ov->opts_root->id != NULL) {
@@ -145,7 +161,7 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
ov->fake_id_opt->name = g_strdup("id");
ov->fake_id_opt->str = g_strdup(ov->opts_root->id);
- opts_visitor_insert(ov->unprocessed_opts, ov->fake_id_opt);
+ opts_visitor_insert(ov, ov->fake_id_opt);
}
}
@@ -163,6 +179,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;
}
@@ -177,6 +195,8 @@ opts_end_struct(Visitor *v, Error **errp)
}
g_hash_table_destroy(ov->unprocessed_opts);
ov->unprocessed_opts = NULL;
+ g_hash_table_destroy(ov->opts);
+ ov->opts = NULL;
if (ov->fake_id_opt) {
g_free(ov->fake_id_opt->name);
g_free(ov->fake_id_opt->str);
@@ -185,16 +205,56 @@ opts_end_struct(Visitor *v, Error **errp)
ov->fake_id_opt = NULL;
}
+static void
+sum_strlen(gpointer data, gpointer user_data)
+{
+ const char *str = data;
+ size_t *sum_len = user_data;
+ *sum_len += strlen(str) + 1;
+}
+
+static void
+append_str(gpointer data, gpointer user_data)
+{
+ strcat(user_data, data);
+ strcat(user_data, ".");
+}
+
+/* lookup a name, trying from the most qualified version (e.g. foo.bar.asd) to
+ * least qualified ones (i.e. foo.bar.asd overrides bar.asd or asd) */
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, *key2;
+ size_t sum_len = strlen(name);
- list = g_hash_table_lookup(ov->unprocessed_opts, 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);
+
+ key2 = key;
+ while (*key2) {
+ list = g_hash_table_lookup(ov->opts, key2);
+ if (list) {
+ if (out_key) {
+ *out_key = g_strdup(key2);
+ }
+ break;
+ }
+
+ while (*key2 && *key2++ != '.') {
+ }
+ }
if (!list) {
error_set(errp, QERR_MISSING_PARAMETER, name);
}
+
+ g_free(key);
return list;
}
@@ -206,7 +266,7 @@ 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);
+ ov->repeated_opts = lookup_distinct(ov, name, &ov->repeated_name, errp);
if (ov->repeated_opts != NULL) {
ov->list_mode = LM_STARTED;
}
@@ -242,11 +302,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;
@@ -272,22 +330,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);
}
@@ -309,13 +373,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);
}
@@ -325,8 +391,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;
}
@@ -343,13 +410,15 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
} else {
error_set(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);
}
@@ -361,13 +430,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;
}
@@ -381,11 +451,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);
@@ -406,6 +478,7 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
(ov->list_mode == LM_NONE) ? "an int64 value" :
"an int64 value or range");
+ g_free(key);
}
@@ -417,13 +490,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;
}
@@ -435,11 +509,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 &&
@@ -458,6 +534,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
(ov->list_mode == LM_NONE) ? "a uint64 value" :
"a uint64 value or range");
+ g_free(key);
}
@@ -468,8 +545,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;
}
@@ -479,11 +557,13 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
if (val < 0 || *endptr) {
error_set(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);
}
@@ -494,7 +574,7 @@ opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
/* 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);
}
@@ -505,6 +585,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;
@@ -545,6 +627,10 @@ opts_visitor_cleanup(OptsVisitor *ov)
if (ov->unprocessed_opts != NULL) {
g_hash_table_destroy(ov->unprocessed_opts);
}
+ if (ov->opts != NULL) {
+ g_hash_table_destroy(ov->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 c7eaa86..a818eff 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -81,6 +81,11 @@
{ 'command': 'user_def_cmd3', 'data': {'a': 'int', '*b': 'int' },
'returns': 'int' }
+# 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:
#
@@ -94,7 +99,9 @@
'*u64' : [ 'uint64' ],
'*u16' : [ 'uint16' ],
'*i64x': 'int' ,
- '*u64x': 'uint64' } }
+ '*u64x': 'uint64' ,
+ 'sub0': 'UserDefOptionsSub',
+ 'sub1': 'UserDefOptionsSub' } }
# testing event
{ 'struct': 'EventStructOne',
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index ebeee5d..5862c7c 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -177,6 +177,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 == 13);
+}
+
+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
@@ -270,6 +298,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_both, "nint=13");
+ add_test("/visitor/opts/nested/both", &expect_both,
+ "sub0.nint=13,sub1.nint=13");
+ 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.4.3
next prev parent reply other threads:[~2015-06-16 12:49 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 12:49 [Qemu-devel] [PATCH v2 0/6] -audiodev option Kővágó, Zoltán
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 1/6] qapi: qapi for audio backends Kővágó, Zoltán
2015-06-17 7:46 ` Markus Armbruster
2015-06-17 10:54 ` Kővágó Zoltán
2015-06-17 11:48 ` Markus Armbruster
2015-06-17 12:07 ` Kővágó Zoltán
2015-06-17 13:37 ` Markus Armbruster
2015-06-17 13:53 ` Kővágó Zoltán
2015-06-17 16:06 ` Markus Armbruster
2015-06-18 0:21 ` Kővágó Zoltán
2015-06-18 8:51 ` Markus Armbruster
2015-06-17 15:50 ` Eric Blake
2015-06-16 12:49 ` Kővágó, Zoltán [this message]
2015-06-17 7:50 ` [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor Markus Armbruster
2015-06-17 8:41 ` Gerd Hoffmann
2015-06-17 11:01 ` Kővágó Zoltán
2015-06-17 11:50 ` Markus Armbruster
2015-06-17 15:47 ` Eric Blake
2015-06-17 11:18 ` Markus Armbruster
2015-06-17 12:11 ` Kővágó Zoltán
2015-06-17 13:41 ` Markus Armbruster
2015-06-17 14:02 ` Kővágó Zoltán
2015-06-17 16:10 ` Markus Armbruster
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 3/6] opts: do not print separator before first item in qemu_opts_print Kővágó, Zoltán
2015-06-17 7:53 ` Markus Armbruster
2015-06-17 9:02 ` Kevin Wolf
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 4/6] qapi: AllocVisitor Kővágó, Zoltán
2015-06-17 7:56 ` Markus Armbruster
2015-06-17 12:01 ` Kővágó Zoltán
2015-06-17 13:42 ` Markus Armbruster
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 5/6] audio: use qapi AudioFormat instead of audfmt_e Kővágó, Zoltán
2015-06-17 8:01 ` Markus Armbruster
2015-06-17 11:05 ` Kővágó Zoltán
2015-06-17 11:51 ` Markus Armbruster
2015-06-17 16:01 ` Eric Blake
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 6/6] audio: -audiodev command line option Kővágó, Zoltán
2015-06-17 8:13 ` Markus Armbruster
2015-06-17 11:18 ` Kővágó Zoltán
2015-06-17 12:27 ` Markus Armbruster
2015-06-17 13:25 ` Kővágó Zoltán
2015-06-17 16:13 ` Markus Armbruster
2015-06-18 6:54 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=88a1cc0775d3d4f5262b31b9452f8acccc6bbb41.1434458391.git.DirtY.iCE.hu@gmail.com \
--to=dirty.ice.hu@gmail.com \
--cc=armbru@redhat.com \
--cc=kraxel@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.