"Daniel P. Berrange" <berra...@redhat.com> writes: > 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 <berra...@redhat.com> > --- > 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) {
Uh, can you explain this hunk? The < comes from PATCH 10. > 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); Can you explain why you swap the order of visit_check_struct() and user_creatable_add_type()? It smells like a bug fix to me... Odd: opts_check_struct() does nothing unless depth == 0. But depth is at least 1 between opts_start_struct() and opts_end_struct(). Bug? > @@ -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; > } Why this hunk? > @@ -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. I can't find a "numa" object in the object-add sense, only a NumaOptions QAPI object created by -numa. Is that what you mean? > + * > + * 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. We screwed up QMP object-add. device_add, netdev_add and object_add all treat additional arguments as properties *except* for QMP object-add, which has them wrapped in a JSON object instead. Aside: if someone foolishly adds a property named "qom-type" or "id", it'll work in QMP but not in HMP or -object. The inconsistency complicates the code even before this patch: * Core: user_creatable_add_type() takes properties in *two* forms: as a visitor and as a qdict. It visits a struct with the members given by the qdict's keys. That's its only use of the qdict. * QMP: qmp_object_add() gets properties as a separate QDict. It creates a QObject visitor for it, and passes it to user_creatable_add_type() along with the QDict. * -object: user_creatable_add_opts() gets the option argument as a QemuOpts. It creates an options visitor for it *and* converts it to a QDict, then passes both to user_creatable_add(). * user_creatable_add() visits a struct. It first visits the non-property arguments "qom-type" and "id". It passes the visitor and a shallow copy of the QDict with the non-property entries deleted to user_creatable_add_type() to visit the remaining struct members. * HMP: hmp_object_add() gets its arguments as a QDict. It converts it to QemuOpts and creates an options visitor for it *boggle*. It passes the visitor along with original QDict to user_creatable_add(). I still can't quite see what's requiring autocreate_structs. But I hope we can get rid of it by cleaning up this mess a bit. Here's how I'd try: * Move visit_start_struct() and visit_end_struct() from user_creatable_add_type() to its two callers qmp_object_add() and user_creatable_add(). * Make user_creatable_add_type() ignore QDict keys "qom-type" and "id". * QMP: any attempt to specify property "qom-type" or "id" now fails, because visit_check_struct() flags them as unexpected. * user_creatable_add(): ditch the silly QDict copying. * hmp_object_add(): ditch the silly conversion to QemuOpts, and create a QObject visitor instead. > + */ > + 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; > } [Skipping test updates for now...]