From patchwork Tue Sep 27 13:13:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 675585 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sk1rM3461z9s8x for ; Tue, 27 Sep 2016 23:28:51 +1000 (AEST) Received: from localhost ([::1]:51001 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bosRF-0002cw-5l for incoming@patchwork.ozlabs.org; Tue, 27 Sep 2016 09:28:49 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bosD3-0004xF-N2 for qemu-devel@nongnu.org; Tue, 27 Sep 2016 09:14:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bosD0-0003pA-HR for qemu-devel@nongnu.org; Tue, 27 Sep 2016 09:14:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52914) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bosCu-0003n4-BZ; Tue, 27 Sep 2016 09:14:00 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DEBA83F1EC; Tue, 27 Sep 2016 13:13:59 +0000 (UTC) Received: from t530wlan.home.berrange.com.com (vpn1-5-127.ams2.redhat.com [10.36.5.127]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8RDDUK1008449; Tue, 27 Sep 2016 09:13:57 -0400 From: "Daniel P. Berrange" To: qemu-devel@nongnu.org Date: Tue, 27 Sep 2016 14:13:12 +0100 Message-Id: <1474982001-20878-11-git-send-email-berrange@redhat.com> In-Reply-To: <1474982001-20878-1-git-send-email-berrange@redhat.com> References: <1474982001-20878-1-git-send-email-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 27 Sep 2016 13:13:59 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v14 10/19] qapi: permit auto-creating nested structs X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-block@nongnu.org, Markus Armbruster , Max Reitz , Paolo Bonzini , =?UTF-8?q?Andreas=20F=C3=A4rber?= Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Some of the historical command line opts that had their keys in in a completely flat namespace are now represented by QAPI schemas that use a nested structs. When converting the QemuOpts to QObject, there is no information about compound types available, so the QObject will be completely flat, even after the qdict_crumple() call. So when starting a struct, we may not have a QDict available in the input data, so we auto-create a QDict containing all the currently unvisited input data keys. Not all historical command line opts require this, so the behaviour is opt-in, by specifying how many levels of structs are permitted to be auto-created. Note that this only works if the child struct is the last field to the visited in the parent struct. This is always the case for currently existing legacy command line options. The example is the NetLegacy type which has 3 levels of structs. The modern way to represent this in QemuOpts would be the dot-separated component approach -net vlan=1,id=foo,name=bar,opts.type=tap,\ opts.data.fd=3,opts.data.script=ifup The legacy syntax will just be presenting -net vlan=1,id=foo,name=bar,type=tap,fd=3,script=ifup So we need to auto-create 3 levels of struct when visiting. The implementation here will enable visiting in both the modern and legacy syntax, compared to OptsVisitor which only allows the legacy syntax. Signed-off-by: Daniel P. Berrange --- include/qapi/qobject-input-visitor.h | 21 +++++- qapi/qobject-input-visitor.c | 55 +++++++++++++-- tests/test-qobject-input-visitor.c | 130 ++++++++++++++++++++++++++++++++--- 3 files changed, 190 insertions(+), 16 deletions(-) diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h index fb52457..90989f4 100644 --- a/include/qapi/qobject-input-visitor.h +++ b/include/qapi/qobject-input-visitor.h @@ -45,7 +45,7 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict); * If @autocreate_list is true, then as an alternative to a normal QList, * list values can be stored as a QString or QDict instead, which will * be interpreted as representing single element lists. This should only - * by used if compatibility is required with the OptsVisitor which allowed + * be used if compatibility is required with the OptsVisitor which allowed * repeated keys, without list indexes, to represent lists. e.g. set this * to true if you have compatibility requirements for * @@ -55,6 +55,22 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict); * * -arg foo.0=hello,foo.1=world * + * If @autocreate_struct_levels is non-zero, then when visiting structs, + * if the corresponding QDict is not found, it will automatically create + * a QDict containing all remaining unvisited options. This should only + * be used if compatibility is required with the OptsVisitor which flatten + * structs so that all keys were at the same level. e.g. set this to a + * non-zero number if you compatibility requirements for + * + * -arg type=person,surname=blogs,forename=fred + * + * to be treated as equivalent to the perferred syntax + * + * -arg type=person,data.surname=blogs,data.forename=fred + * + * The value given determines how many levels of structs are allowed to + * be flattened in this way. + * * The visitor always operates in strict mode, requiring all dict keys * to be consumed during visitation. An error will be reported if this * does not happen. @@ -63,6 +79,7 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict); * visit_free() when no longer required. */ Visitor *qobject_input_visitor_new_autocast(QObject *obj, - bool autocreate_list); + bool autocreate_list, + size_t autocreate_struct_levels); #endif diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index e8afd1e..6cacd4b 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -52,6 +52,14 @@ struct QObjectInputVisitor /* Whether we can auto-create single element lists when * encountering a non-QList type */ bool autocreate_list; + + /* Current depth of recursion into structs */ + size_t struct_level; + + /* Numbers of levels at which we will + * consider auto-creating a struct containing + * remaining unvisited items */ + size_t autocreate_struct_levels; }; static QObjectInputVisitor *to_qiv(Visitor *v) @@ -79,7 +87,12 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, if (qobject_type(qobj) == QTYPE_QDICT) { assert(name); - ret = qdict_get(qobject_to_qdict(qobj), name); + if (qiv->autocreate_struct_levels && + !g_hash_table_contains(tos->h, name)) { + ret = NULL; + } else { + ret = qdict_get(qobject_to_qdict(qobj), name); + } if (tos->h && consume && ret) { bool removed = g_hash_table_remove(tos->h, name); assert(removed); @@ -114,7 +127,8 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, tos->qapi = qapi; qobject_incref(obj); - if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) { + if ((qiv->autocreate_struct_levels || qiv->strict) && + qobject_type(obj) == QTYPE_QDICT) { h = g_hash_table_new(g_str_hash, g_str_equal); qdict_iter(qobject_to_qdict(obj), qdict_add_key, h); tos->h = h; @@ -163,6 +177,9 @@ static void qobject_input_pop(Visitor *v, void **obj) StackObject *tos = QSLIST_FIRST(&qiv->stack); assert(tos && tos->qapi == obj); + if (tos->h != NULL) { + qiv->struct_level--; + } QSLIST_REMOVE_HEAD(&qiv->stack, node); qobject_input_stack_object_free(tos); } @@ -171,12 +188,38 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj, size_t size, Error **errp) { QObjectInputVisitor *qiv = to_qiv(v); - QObject *qobj = qobject_input_get_object(qiv, name, true); + QObject *qobj; + QDict *subopts = NULL; Error *err = NULL; + StackObject *tos = QSLIST_FIRST(&qiv->stack); + qobj = qobject_input_get_object(qiv, name, true); if (obj) { *obj = NULL; } + + if (!qobj && (qiv->struct_level < qiv->autocreate_struct_levels)) { + /* Create a new dict that contains all the currently + * unvisited items */ + if (tos) { + GHashTableIter iter; + const char *key; + + subopts = qdict_new(); + + g_hash_table_iter_init(&iter, tos->h); + while (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { + QObject *val = qdict_get(qobject_to_qdict(tos->obj), key); + qobject_incref(val); + qdict_put_obj(subopts, key, val); + } + g_hash_table_remove_all(tos->h); + qobj = QOBJECT(subopts); + } else { + qobj = qiv->root; + } + } + if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict"); @@ -184,6 +227,7 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj, } qobject_input_push(qiv, qobj, obj, &err); + qiv->struct_level++; if (err) { error_propagate(errp, err); return; @@ -192,6 +236,7 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj, if (obj) { *obj = g_malloc0(size); } + QDECREF(subopts); } @@ -530,7 +575,8 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict) } Visitor *qobject_input_visitor_new_autocast(QObject *obj, - bool autocreate_list) + bool autocreate_list, + size_t autocreate_struct_levels) { QObjectInputVisitor *v; @@ -556,6 +602,7 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj, v->visitor.free = qobject_input_free; v->strict = true; v->autocreate_list = autocreate_list; + v->autocreate_struct_levels = autocreate_struct_levels; v->root = obj; qobject_incref(obj); diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index c227565..ab55f99 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -40,11 +40,13 @@ static void visitor_input_teardown(TestInputVisitorData *data, /* The various test_init functions are provided instead of a test setup function so that the JSON string used by the tests are kept in the test functions (and not in main()). */ -static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, - bool strict, bool autocast, - bool autocreate_list, - const char *json_string, - va_list *ap) +static Visitor * +visitor_input_test_init_internal(TestInputVisitorData *data, + bool strict, bool autocast, + bool autocreate_list, + size_t autocreate_struct_levels, + const char *json_string, + va_list *ap) { visitor_input_teardown(data, NULL); @@ -53,10 +55,11 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, if (autocast) { assert(strict); - data->qiv = qobject_input_visitor_new_autocast(data->obj, - autocreate_list); + data->qiv = qobject_input_visitor_new_autocast( + data->obj, autocreate_list, autocreate_struct_levels); } else { assert(!autocreate_list); + assert(!autocreate_struct_levels); data->qiv = qobject_input_visitor_new(data->obj, strict); } g_assert(data->qiv); @@ -74,7 +77,7 @@ Visitor *visitor_input_test_init_full(TestInputVisitorData *data, va_start(ap, json_string); v = visitor_input_test_init_internal(data, strict, autocast, - autocreate_list, + autocreate_list, 0, json_string, &ap); va_end(ap); return v; @@ -88,7 +91,7 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, va_list ap; va_start(ap, json_string); - v = visitor_input_test_init_internal(data, true, false, false, + v = visitor_input_test_init_internal(data, true, false, false, 0, json_string, &ap); va_end(ap); return v; @@ -104,7 +107,7 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data, const char *json_string) { - return visitor_input_test_init_internal(data, true, false, false, + return visitor_input_test_init_internal(data, true, false, false, 0, json_string, NULL); } @@ -372,6 +375,109 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data, qapi_free_UserDefTwo(udp); } +static void test_visitor_in_struct_autocreate(TestInputVisitorData *data, + const void *unused) +{ + Visitor *v; + int64_t vlan; + char *id = NULL; + char *type; + int64_t fd; + char *script = NULL; + + v = visitor_input_test_init_internal( + data, true, true, false, 3, + "{ 'vlan': '1', 'id': 'foo', 'type': 'tap', 'fd': '3', " + "'script': 'ifup' }", NULL); + + visit_start_struct(v, NULL, NULL, 0, &error_abort); + + visit_type_int64(v, "vlan", &vlan, &error_abort); + visit_type_str(v, "id", &id, &error_abort); + + visit_start_struct(v, "opts", NULL, 0, &error_abort); + visit_type_str(v, "type", &type, &error_abort); + + visit_start_struct(v, "data", NULL, 0, &error_abort); + + visit_type_int64(v, "fd", &fd, &error_abort); + visit_type_str(v, "script", &script, &error_abort); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + g_assert_cmpstr(id, ==, "foo"); + g_assert_cmpint(vlan, ==, 1); + g_assert_cmpstr(type, ==, "tap"); + g_assert_cmpstr(script, ==, "ifup"); + g_assert_cmpint(fd, ==, 3); + + g_free(id); + g_free(type); + g_free(script); +} + +static void test_visitor_in_struct_autocreate_extra(TestInputVisitorData *data, + const void *unused) +{ + Visitor *v; + int64_t vlan; + char *id = NULL; + char *type; + int64_t fd; + char *script = NULL; + Error *err = NULL; + + v = visitor_input_test_init_internal( + data, true, true, false, 3, + "{ 'vlan': '1', 'id': 'foo', 'type': 'tap', 'fd': '3', " + "'script': 'ifup' }", NULL); + + visit_start_struct(v, NULL, NULL, 0, &error_abort); + + visit_type_int64(v, "vlan", &vlan, &error_abort); + + visit_start_struct(v, "opts", NULL, 0, &error_abort); + visit_type_str(v, "type", &type, &error_abort); + + visit_start_struct(v, "data", NULL, 0, &error_abort); + + visit_type_int64(v, "fd", &fd, &error_abort); + visit_type_str(v, "script", &script, &error_abort); + + /* We've not visited 'id' so should see a complaint */ + visit_check_struct(v, &err); + error_free_or_abort(&err); + visit_end_struct(v, NULL); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + /* We can't visit stuff in the base struct after + * we auto-created child structs */ + visit_type_str(v, "id", &id, &err); + error_free_or_abort(&err); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + g_assert_cmpstr(id, ==, NULL); + g_assert_cmpint(vlan, ==, 1); + g_assert_cmpstr(type, ==, "tap"); + g_assert_cmpstr(script, ==, "ifup"); + g_assert_cmpint(fd, ==, 3); + + g_free(id); + g_free(type); + g_free(script); +} + static void test_visitor_in_list(TestInputVisitorData *data, const void *unused) { @@ -1096,6 +1202,10 @@ int main(int argc, char **argv) NULL, test_visitor_in_struct); input_visitor_test_add("/visitor/input/struct-nested", NULL, test_visitor_in_struct_nested); + input_visitor_test_add("/visitor/input/struct-autocreate", + NULL, test_visitor_in_struct_autocreate); + input_visitor_test_add("/visitor/input/struct-autocreate-extra", + NULL, test_visitor_in_struct_autocreate_extra); input_visitor_test_add("/visitor/input/list", NULL, test_visitor_in_list); input_visitor_test_add("/visitor/input/list-autocreate-noautocast",