[Qemu-devel] [PATCH v14 15/21] qom: support non-scalar properties with -object

[Daniel P. Berrange]

The current -object command line syntax only allows for
creation of objects with scalar properties, or a list
with a fixed scalar element type. Objects which have
properties that are represented as structs in the QAPI
schema cannot be created using -object.

This is a design limitation of the way the OptsVisitor
is written. It simply iterates over the QemuOpts values
as a flat list. The support for lists is enabled by
allowing the same key to be repeated in the opts string.

The QObjectInputVisitor now has functionality that allows
it to replace OptsVisitor, maintaining full backwards
compatibility for previous CLI syntax, while also allowing
use of new syntax for structs.

Thus -object can now support non-scalar properties,
for example the QMP object:

  {
    "execute": "object-add",
    "arguments": {
      "qom-type": "demo",
      "id": "demo0",
      "parameters": {
        "foo": [
          { "bar": "one", "wizz": "1" },
          { "bar": "two", "wizz": "2" }
        ]
      }
    }
  }

Would be creatable via the CLI now using

    $QEMU \
      -object demo,id=demo0,\
              foo.0.bar=one,foo.0.wizz=1,\
              foo.1.bar=two,foo.1.wizz=2

Notice that this syntax is intentionally compatible
with that currently used by block drivers. NB the
indentation seen here after line continuations should
not be used in reality, it is just for clarity in this
commit message.

Signed-off-by: Daniel P. Berrange <address@hidden>
---
 qapi/qobject-input-visitor.c |   2 +-
 qom/object_interfaces.c      |  37 ++++-
 tests/check-qom-proplist.c   | 367 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 397 insertions(+), 9 deletions(-)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 5a3872c..f1030d5 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -204,7 +204,7 @@ static void qobject_input_start_struct(Visitor *v, const 
char *name, void **obj,
         *obj = NULL;
     }
 
-    if (!qobj && (qiv->struct_level < qiv->autocreate_struct_levels)) {
+    if (!qobj && (qiv->struct_level <= qiv->autocreate_struct_levels)) {
         /* Create a new dict that contains all the currently
          * unvisited items */
         if (tos) {
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index fdc406b..88a1e88 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -4,7 +4,8 @@
 #include "qemu/module.h"
 #include "qapi-visit.h"
 #include "qapi/qobject-output-visitor.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qemu/option.h"
 
 void user_creatable_complete(Object *obj, Error **errp)
 {
@@ -63,12 +64,16 @@ Object *user_creatable_add(const QDict *qdict,
     if (local_err) {
         goto out_visit;
     }
-    visit_check_struct(v, &local_err);
+
+    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
     if (local_err) {
         goto out_visit;
     }
 
-    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+    visit_check_struct(v, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
 
 out_visit:
     visit_end_struct(v, NULL);
@@ -114,7 +119,7 @@ Object *user_creatable_add_type(const char *type, const 
char *id,
 
     assert(qdict);
     obj = object_new(type);
-    visit_start_struct(v, NULL, NULL, 0, &local_err);
+    visit_start_struct(v, "props", NULL, 0, &local_err);
     if (local_err) {
         goto out;
     }
@@ -158,14 +163,32 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 {
     Visitor *v;
     QDict *pdict;
+    QObject *pobj;
     Object *obj = NULL;
 
-    v = opts_visitor_new(opts);
-    pdict = qemu_opts_to_qdict(opts, NULL, QEMU_OPTS_REPEAT_POLICY_LAST,
+    pdict = qemu_opts_to_qdict(opts, NULL,
+                               QEMU_OPTS_REPEAT_POLICY_ALL,
                                &error_abort);
 
-    obj = user_creatable_add(pdict, v, errp);
+    pobj = qdict_crumple(pdict, true, errp);
+    if (!pobj) {
+        goto cleanup;
+    }
+    /*
+     * We need autocreate_list=true + permit_int_ranges=true
+     * in order to maintain compat with OptsVisitor creation
+     * of the 'numa' object which had an int16List property.
+     *
+     * We need autocreate_structs=1, because at the CLI level
+     * we have the object type + properties in the same flat
+     * struct, even though at the QMP level they are nested.
+     */
+    v = qobject_input_visitor_new_autocast(pobj, true, 1, true);
+
+    obj = user_creatable_add(qobject_to_qdict(pobj), v, errp);
     visit_free(v);
+    qobject_decref(pobj);
+ cleanup:
     QDECREF(pdict);
     return obj;
 }
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index a16cefc..20eb1d1 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -22,7 +22,16 @@
 
 #include "qapi/error.h"
 #include "qom/object.h"
+#include "qom/object_interfaces.h"
 #include "qemu/module.h"
+#include "qapi/visitor.h"
+#include "qom/object_interfaces.h"
+#include "qemu/option.h"
+#include "qemu/config-file.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi-visit.h"
+#include "qapi/dealloc-visitor.h"
 
 
 #define TYPE_DUMMY "qemu-dummy"
@@ -30,6 +39,11 @@
 typedef struct DummyObject DummyObject;
 typedef struct DummyObjectClass DummyObjectClass;
 
+typedef struct DummyPerson DummyPerson;
+typedef struct DummyAddr DummyAddr;
+typedef struct DummyAddrList DummyAddrList;
+typedef struct DummySizeList DummySizeList;
+
 #define DUMMY_OBJECT(obj)                               \
     OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY)
 
@@ -50,12 +64,35 @@ static const char *const dummy_animal_map[DUMMY_LAST + 1] = 
{
     [DUMMY_LAST] = NULL,
 };
 
+
+struct DummyAddr {
+    char *ip;
+    int64_t prefix;
+    bool ipv6only;
+};
+
+struct DummyAddrList {
+    DummyAddrList *next;
+    struct DummyAddr *value;
+};
+
+struct DummyPerson {
+    char *name;
+    int64_t age;
+};
+
 struct DummyObject {
     Object parent_obj;
 
     bool bv;
     DummyAnimal av;
     char *sv;
+
+    intList *sizes;
+
+    DummyPerson *person;
+
+    DummyAddrList *addrs;
 };
 
 struct DummyObjectClass {
@@ -117,6 +154,157 @@ static char *dummy_get_sv(Object *obj,
     return g_strdup(dobj->sv);
 }
 
+static void visit_type_DummyPerson_fields(Visitor *v, DummyPerson **obj,
+                                          Error **errp)
+{
+    Error *err = NULL;
+
+    visit_type_str(v, "name", &(*obj)->name, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int(v, "age", &(*obj)->age, &err);
+    if (err) {
+        goto out;
+    }
+
+out:
+    error_propagate(errp, err);
+}
+
+static void visit_type_DummyPerson(Visitor *v, const char *name,
+                                   DummyPerson **obj, Error **errp)
+{
+    Error *err = NULL;
+
+    visit_start_struct(v, name, (void **)obj, sizeof(DummyPerson), &err);
+    if (err) {
+        goto out;
+    }
+    if (!*obj) {
+        goto out_obj;
+    }
+    visit_type_DummyPerson_fields(v, obj, &err);
+    error_propagate(errp, err);
+    err = NULL;
+out_obj:
+    visit_end_struct(v, (void **)obj);
+out:
+    error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddr_members(Visitor *v, DummyAddr **obj,
+                                         Error **errp)
+{
+    Error *err = NULL;
+
+    visit_type_str(v, "ip", &(*obj)->ip, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int(v, "prefix", &(*obj)->prefix, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_bool(v, "ipv6only", &(*obj)->ipv6only, &err);
+    if (err) {
+        goto out;
+    }
+
+out:
+    error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddr(Visitor *v, const char *name,
+                                 DummyAddr **obj, Error **errp)
+{
+    Error *err = NULL;
+
+    visit_start_struct(v, name, (void **)obj, sizeof(DummyAddr), &err);
+    if (err) {
+        goto out;
+    }
+    if (!*obj) {
+        goto out_obj;
+    }
+    visit_type_DummyAddr_members(v, obj, &err);
+    error_propagate(errp, err);
+    err = NULL;
+out_obj:
+    visit_end_struct(v, (void **)obj);
+out:
+    error_propagate(errp, err);
+}
+
+static void qapi_free_DummyAddrList(DummyAddrList *obj);
+
+static void visit_type_DummyAddrList(Visitor *v, const char *name,
+                                     DummyAddrList **obj, Error **errp)
+{
+    Error *err = NULL;
+    DummyAddrList *tail;
+    size_t size = sizeof(**obj);
+
+    visit_start_list(v, name, (GenericList **)obj, size, &err);
+    if (err) {
+        goto out;
+    }
+
+    for (tail = *obj; tail;
+         tail = (DummyAddrList *)visit_next_list(v,
+                                                 (GenericList *)tail,
+                                                 size)) {
+        visit_type_DummyAddr(v, NULL, &tail->value, &err);
+        if (err) {
+            break;
+        }
+    }
+
+    visit_end_list(v, (void **)obj);
+    if (err && visit_is_input(v)) {
+        qapi_free_DummyAddrList(*obj);
+        *obj = NULL;
+    }
+out:
+    error_propagate(errp, err);
+}
+
+static void qapi_free_DummyAddrList(DummyAddrList *obj)
+{
+    Visitor *v;
+
+    if (!obj) {
+        return;
+    }
+
+    v = qapi_dealloc_visitor_new();
+    visit_type_DummyAddrList(v, NULL, &obj, NULL);
+    visit_free(v);
+}
+
+static void dummy_set_sizes(Object *obj, Visitor *v, const char *name,
+                            void *opaque, Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    visit_type_intList(v, name, &dobj->sizes, errp);
+}
+
+static void dummy_set_person(Object *obj, Visitor *v, const char *name,
+                            void *opaque, Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    visit_type_DummyPerson(v, name, &dobj->person, errp);
+}
+
+static void dummy_set_addrs(Object *obj, Visitor *v, const char *name,
+                            void *opaque, Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    visit_type_DummyAddrList(v, name, &dobj->addrs, errp);
+}
 
 static void dummy_init(Object *obj)
 {
@@ -126,9 +314,16 @@ static void dummy_init(Object *obj)
                              NULL);
 }
 
+static void
+dummy_complete(UserCreatable *uc, Error **errp)
+{
+}
 
 static void dummy_class_init(ObjectClass *cls, void *data)
 {
+    UserCreatableClass *ucc = USER_CREATABLE_CLASS(cls);
+    ucc->complete = dummy_complete;
+
     object_class_property_add_bool(cls, "bv",
                                    dummy_get_bv,
                                    dummy_set_bv,
@@ -143,12 +338,34 @@ static void dummy_class_init(ObjectClass *cls, void *data)
                                    dummy_get_av,
                                    dummy_set_av,
                                    NULL);
+    object_class_property_add(cls, "sizes",
+                              "int[]",
+                              NULL,
+                              dummy_set_sizes,
+                              NULL, NULL, NULL);
+    object_class_property_add(cls, "person",
+                              "DummyPerson",
+                              NULL,
+                              dummy_set_person,
+                              NULL, NULL, NULL);
+    object_class_property_add(cls, "addrs",
+                              "DummyAddrList",
+                              NULL,
+                              dummy_set_addrs,
+                              NULL, NULL, NULL);
 }
 
 
 static void dummy_finalize(Object *obj)
 {
     DummyObject *dobj = DUMMY_OBJECT(obj);
+    Visitor *v;
+
+    v = qapi_dealloc_visitor_new();
+    visit_type_intList(v, NULL, &dobj->sizes, NULL);
+    visit_type_DummyAddrList(v, NULL, &dobj->addrs, NULL);
+    visit_type_DummyPerson(v, NULL, &dobj->person, NULL);
+    visit_free(v);
 
     g_free(dobj->sv);
 }
@@ -162,6 +379,10 @@ static const TypeInfo dummy_info = {
     .instance_finalize = dummy_finalize,
     .class_size = sizeof(DummyObjectClass),
     .class_init = dummy_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
 };
 
 
@@ -388,6 +609,128 @@ static void test_dummy_createlist(void)
     object_unparent(OBJECT(dobj));
 }
 
+
+static QemuOptsList dummy_opts = {
+    .name = "object",
+    .implied_opt_name = "qom-type",
+    .head = QTAILQ_HEAD_INITIALIZER(dummy_opts.head),
+    .desc = {
+        { }
+    },
+};
+
+static void test_dummy_create_complex(DummyObject *dummy)
+{
+    g_assert(dummy->person != NULL);
+    g_assert_cmpstr(dummy->person->name, ==, "fred");
+    g_assert_cmpint(dummy->person->age, ==, 52);
+
+    g_assert(dummy->sizes != NULL);
+    g_assert_cmpint(dummy->sizes->value, ==, 12);
+    g_assert_cmpint(dummy->sizes->next->value, ==, 13);
+    g_assert_cmpint(dummy->sizes->next->next->value, ==, 8139);
+
+    g_assert(dummy->addrs != NULL);
+    g_assert_cmpstr(dummy->addrs->value->ip, ==, "127.0.0.1");
+    g_assert_cmpint(dummy->addrs->value->prefix, ==, 24);
+    g_assert(dummy->addrs->value->ipv6only);
+    g_assert_cmpstr(dummy->addrs->next->value->ip, ==, "0.0.0.0");
+    g_assert_cmpint(dummy->addrs->next->value->prefix, ==, 16);
+    g_assert(!dummy->addrs->next->value->ipv6only);
+}
+
+
+static void _test_dummy_createopts(const char *optstr)
+{
+    QemuOpts *opts;
+    DummyObject *dummy;
+
+    opts = qemu_opts_parse_noisily(&dummy_opts,
+                                   optstr, true);
+    g_assert(opts != NULL);
+
+    dummy = DUMMY_OBJECT(user_creatable_add_opts(opts, &error_abort));
+
+    test_dummy_create_complex(dummy);
+
+    object_unparent(OBJECT(dummy));
+    object_unref(OBJECT(dummy));
+    qemu_opts_reset(&dummy_opts);
+}
+
+
+static void test_dummy_createopts(void)
+{
+    const char *optstr = "qemu-dummy,id=dummy0,bv=yes,av=alligator,sv=hiss,"
+        "person.name=fred,person.age=52,sizes.0=12,sizes.1=13,sizes.2=8139,"
+        "addrs.0.ip=127.0.0.1,addrs.0.prefix=24,addrs.0.ipv6only=yes,"
+        "addrs.1.ip=0.0.0.0,addrs.1.prefix=16,addrs.1.ipv6only=no";
+    _test_dummy_createopts(optstr);
+}
+
+static void test_dummy_createopts_repeat(void)
+{
+    const char *optstr = "qemu-dummy,id=dummy0,bv=yes,av=alligator,sv=hiss,"
+        "person.name=fred,person.age=52,sizes=12,sizes=13,sizes=8139,"
+        "addrs.0.ip=127.0.0.1,addrs.0.prefix=24,addrs.0.ipv6only=yes,"
+        "addrs.1.ip=0.0.0.0,addrs.1.prefix=16,addrs.1.ipv6only=no";
+    _test_dummy_createopts(optstr);
+}
+
+static void test_dummy_createopts_range(void)
+{
+    const char *optstr = "qemu-dummy,id=dummy0,bv=yes,av=alligator,sv=hiss,"
+        "person.name=fred,person.age=52,sizes=12-13,sizes=8139,"
+        "addrs.0.ip=127.0.0.1,addrs.0.prefix=24,addrs.0.ipv6only=yes,"
+        "addrs.1.ip=0.0.0.0,addrs.1.prefix=16,addrs.1.ipv6only=no";
+    _test_dummy_createopts(optstr);
+}
+
+
+static void test_dummy_createopts_bad(void)
+{
+    /* Something that tries to create a QList at the top level
+     * should be invalid. */
+    const char *optstr = "qemu-dummy,id=dummy0,1=foo,2=bar,3=wizz";
+    QemuOpts *opts;
+    DummyObject *dummy;
+    Error *err = NULL;
+
+    opts = qemu_opts_parse_noisily(&dummy_opts,
+                                   optstr, true);
+    g_assert(opts != NULL);
+
+    dummy = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
+    error_free_or_abort(&err);
+    g_assert(!dummy);
+    qemu_opts_reset(&dummy_opts);
+}
+
+
+static void test_dummy_createqmp(void)
+{
+    const char *jsonstr =
+        "{ 'bv': true, 'av': 'alligator', 'sv': 'hiss', "
+        "  'person': { 'name': 'fred', 'age': 52 }, "
+        "  'sizes': [12, 13, 8139], "
+        "  'addrs': [ { 'ip': '127.0.0.1', 'prefix': 24, 'ipv6only': true }, "
+        "             { 'ip': '0.0.0.0', 'prefix': 16, 'ipv6only': false } ] 
}";
+    QObject *obj = qobject_from_json(jsonstr);
+    Visitor *v = qobject_input_visitor_new(obj, true);
+    DummyObject *dummy;
+    g_assert(obj);
+    dummy = DUMMY_OBJECT(user_creatable_add_type("qemu-dummy", "dummy0",
+                                                 qobject_to_qdict(obj), v,
+                                                 &error_abort));
+
+    test_dummy_create_complex(dummy);
+    visit_free(v);
+    object_unparent(OBJECT(dummy));
+    object_unref(OBJECT(dummy));
+    qobject_decref(obj);
+}
+
+
 static void test_dummy_badenum(void)
 {
     Error *err = NULL;
@@ -473,7 +816,9 @@ static void test_dummy_iterator(void)
 
     ObjectProperty *prop;
     ObjectPropertyIterator iter;
-    bool seenbv = false, seensv = false, seenav = false, seentype;
+    bool seenbv = false, seensv = false, seenav = false,
+        seentype = false, seenaddrs = false, seenperson = false,
+        seensizes = false;
 
     object_property_iter_init(&iter, OBJECT(dobj));
     while ((prop = object_property_iter_next(&iter))) {
@@ -486,6 +831,12 @@ static void test_dummy_iterator(void)
         } else if (g_str_equal(prop->name, "type")) {
             /* This prop comes from the base Object class */
             seentype = true;
+        } else if (g_str_equal(prop->name, "addrs")) {
+            seenaddrs = true;
+        } else if (g_str_equal(prop->name, "person")) {
+            seenperson = true;
+        } else if (g_str_equal(prop->name, "sizes")) {
+            seensizes = true;
         } else {
             g_printerr("Found prop '%s'\n", prop->name);
             g_assert_not_reached();
@@ -495,6 +846,9 @@ static void test_dummy_iterator(void)
     g_assert(seenav);
     g_assert(seensv);
     g_assert(seentype);
+    g_assert(seenaddrs);
+    g_assert(seenperson);
+    g_assert(seensizes);
 
     object_unparent(OBJECT(dobj));
 }
@@ -513,11 +867,15 @@ static void test_dummy_delchild(void)
     object_unparent(OBJECT(dev));
 }
 
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
     module_call_init(MODULE_INIT_QOM);
+
+    qemu_add_opts(&dummy_opts);
+
     type_register_static(&dummy_info);
     type_register_static(&dummy_dev_info);
     type_register_static(&dummy_bus_info);
@@ -525,6 +883,13 @@ int main(int argc, char **argv)
 
     g_test_add_func("/qom/proplist/createlist", test_dummy_createlist);
     g_test_add_func("/qom/proplist/createv", test_dummy_createv);
+    g_test_add_func("/qom/proplist/createopts", test_dummy_createopts);
+    g_test_add_func("/qom/proplist/createoptsrepeat",
+                    test_dummy_createopts_repeat);
+    g_test_add_func("/qom/proplist/createoptsrange",
+                    test_dummy_createopts_range);
+    g_test_add_func("/qom/proplist/createoptsbad", test_dummy_createopts_bad);
+    g_test_add_func("/qom/proplist/createqmp", test_dummy_createqmp);
     g_test_add_func("/qom/proplist/badenum", test_dummy_badenum);
     g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);
     g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
-- 
2.7.4