[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 02/51] qapi: support nested structs in OptsVisitor
|
From: |
Kővágó, Zoltán |
|
Subject: |
[Qemu-devel] [PATCH 02/51] qapi: support nested structs in OptsVisitor |
|
Date: |
Thu, 14 Jan 2016 14:45:15 +0100 |
From: Kővágó, Zoltán <address@hidden>
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.
To keep backward compatibility, this new feature can be disabled when
creating a new OptsVisitor, in that case it will work identical to
previous versions.
Signed-off-by: Kővágó, Zoltán <address@hidden>
---
hmp.c | 2 +-
hw/acpi/core.c | 2 +-
include/qapi/opts-visitor.h | 2 +-
net/net.c | 2 +-
numa.c | 2 +-
qapi/opts-visitor.c | 132 ++++++++++++++++++++++++++------
tests/qapi-schema/qapi-schema-test.json | 9 ++-
tests/qapi-schema/qapi-schema-test.out | 4 +
tests/test-opts-visitor.c | 36 ++++++++-
vl.c | 2 +-
10 files changed, 163 insertions(+), 30 deletions(-)
diff --git a/hmp.c b/hmp.c
index c2b2c16..3d65529 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1676,7 +1676,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
goto out;
}
- ov = opts_visitor_new(opts);
+ ov = opts_visitor_new(opts, false);
pdict = qdict_clone_shallow(qdict);
visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 21e113d..3220ace 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -241,7 +241,7 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
{
OptsVisitor *ov;
- ov = opts_visitor_new(opts);
+ ov = opts_visitor_new(opts, false);
visit_type_AcpiTableOptions(opts_get_visitor(ov), &hdrs, NULL, &err);
opts_visitor_cleanup(ov);
}
diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
index fd48c14..8c0f3dc 100644
--- a/include/qapi/opts-visitor.h
+++ b/include/qapi/opts-visitor.h
@@ -30,7 +30,7 @@ typedef struct OptsVisitor OptsVisitor;
* - values below INT64_MIN or LLONG_MIN are rejected,
* - values above INT64_MAX or LLONG_MAX are rejected.
*/
-OptsVisitor *opts_visitor_new(const QemuOpts *opts);
+OptsVisitor *opts_visitor_new(const QemuOpts *opts, bool nested);
void opts_visitor_cleanup(OptsVisitor *nv);
Visitor *opts_get_visitor(OptsVisitor *nv);
diff --git a/net/net.c b/net/net.c
index 87dd356..04dd9fe 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1049,7 +1049,7 @@ int net_client_init(QemuOpts *opts, int is_netdev, Error
**errp)
int ret = -1;
{
- OptsVisitor *ov = opts_visitor_new(opts);
+ OptsVisitor *ov = opts_visitor_new(opts, false);
net_visit(opts_get_visitor(ov), is_netdev, &object, &err);
opts_visitor_cleanup(ov);
diff --git a/numa.c b/numa.c
index 1710946..271fbec 100644
--- a/numa.c
+++ b/numa.c
@@ -217,7 +217,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error
**errp)
Error *err = NULL;
{
- OptsVisitor *ov = opts_visitor_new(opts);
+ OptsVisitor *ov = opts_visitor_new(opts, false);
visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err);
opts_visitor_cleanup(ov);
}
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 1820d54..a21881f 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,11 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
if (obj) {
*obj = g_malloc0(size > 0 ? size : 1);
}
+
+ if (ov->nested_names != NULL) {
+ g_queue_push_tail(ov->nested_names, (gpointer) name);
+ }
+
if (ov->depth++ > 0) {
return;
}
@@ -169,6 +179,10 @@ opts_end_struct(Visitor *v, Error **errp)
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
GQueue *any;
+ if (ov->nested_names != NULL) {
+ g_queue_pop_tail(ov->nested_names);
+ }
+
if (--ov->depth > 0) {
return;
}
@@ -198,15 +212,59 @@ 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;
+
+ if (ov->nested_names != NULL) {
+ 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);
+ } else {
+ key = g_strdup(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 +276,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 +313,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 +341,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 +384,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 +402,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 +421,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 +441,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 +462,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 +489,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 +501,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 +520,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 +545,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 +556,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 +568,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,17 +585,23 @@ 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);
}
OptsVisitor *
-opts_visitor_new(const QemuOpts *opts)
+opts_visitor_new(const QemuOpts *opts, bool nested)
{
OptsVisitor *ov;
ov = g_malloc0(sizeof *ov);
+ if (nested) {
+ ov->nested_names = g_queue_new();
+ } else {
+ ov->nested_names = NULL;
+ }
+
ov->visitor.start_struct = &opts_start_struct;
ov->visitor.end_struct = &opts_end_struct;
@@ -560,6 +645,9 @@ opts_visitor_cleanup(OptsVisitor *ov)
if (ov->unprocessed_opts != NULL) {
g_hash_table_destroy(ov->unprocessed_opts);
}
+ if (ov->nested_names != NULL) {
+ 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 4b89527..bd2bf0f 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -125,6 +125,11 @@
'returns': 'int' }
{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
+# 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:
#
@@ -138,7 +143,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 2c546b7..9821063 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -153,6 +153,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 UserDefTwo
member string0: str optional=False
member dict1: UserDefTwoDict optional=False
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 1c753d9..58ca41a 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -43,7 +43,7 @@ setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
NULL);
g_assert(opts != NULL);
- ov = opts_visitor_new(opts);
+ ov = opts_visitor_new(opts, true);
visit_type_UserDefOptions(opts_get_visitor(ov), &f->userdef, NULL,
&f->err);
opts_visitor_cleanup(ov);
@@ -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;
}
diff --git a/vl.c b/vl.c
index 5aaea77..eb7edb0 100644
--- a/vl.c
+++ b/vl.c
@@ -2835,7 +2835,7 @@ static int object_create(void *opaque, QemuOpts *opts,
Error **errp)
QDict *pdict;
bool (*type_predicate)(const char *) = opaque;
- ov = opts_visitor_new(opts);
+ ov = opts_visitor_new(opts, false);
pdict = qemu_opts_to_qdict(opts, NULL);
visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
--
2.7.0
- [Qemu-devel] [PATCH 00/51] audio 5.1 patches, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 03/51] qapi: qapi for audio backends, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 01/51] qapi: support implicit structs in OptsVisitor, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 02/51] qapi: support nested structs in OptsVisitor,
Kővágó, Zoltán <=
- [Qemu-devel] [PATCH 05/51] audio: -audiodev command line option: documentation, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 04/51] audio: use qapi AudioFormat instead of audfmt_e, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 08/51] coreaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 13/51] sdlaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 16/51] audio: -audiodev command line option: cleanup, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 15/51] wavaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 10/51] noaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 11/51] ossaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 07/51] alsaaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14
- [Qemu-devel] [PATCH 14/51] spiceaudio: port to -audiodev config, Kővágó, Zoltán, 2016/01/14