On 09/05/2016 10:16 AM, Daniel P. Berrange wrote: > 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. >
> The previously added qdict_crumple() method is able to
> take a qdict containing a flat set of properties and
> turn that into a arbitrarily nested set of dicts and
> lists. By combining qemu_opts_to_qdict and qdict_crumple()
> together, we can turn the opt string into a data structure
> that is practically identical to that passed over QMP
> when defining an object. The only difference is that all
> the scalar values are represented as strings, rather than
> strings, ints and bools. This is sufficient to let us
> replace the OptsVisitor with the QMPInputVisitor for
QObjectInputVisitor
> use with -object.
>
> 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.
>
> This is also wired up to work for the 'object_add' command
> in the HMP monitor with the same syntax.
>
> (hmp) object_add demo,id=demo0,\
> foo.0.bar=one,foo.0.wizz=1,\
> foo.1.bar=two,foo.1.wizz=2
>
> NB indentation should not be used with HMP commands, this
> is just for convenient formatting in this commit message.
>
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
I still haven't looked closely at the intermediate patches in this
series, but I do like the end result, so I'll start with a review of
this one.
> @@ -158,13 +167,21 @@ 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);
>
> - obj = user_creatable_add(pdict, v, errp);
> + pobj = qdict_crumple(pdict, true, errp);
> + if (!pobj) {
> + goto cleanup;
> + }
> + v = qobject_string_input_visitor_new(pobj);
> +
> + obj = user_creatable_add((QDict *)pobj, v, errp);
This cast looks fishy; I think you want qobject_to_qdict(pobj) for
absolute safety (if some future change causes QDict to not contain
QObject at offset 0). Besides, qdict_crumple() can return a QList
instead of a QDict in some cases, so asserting that qobject_to_qdict()
did not return NULL may be worthwhile to prove that this particular
crumple never gives us something unexpected.
> +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=65,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";
> + QemuOpts *opts;
Actually, what happens if I do an optstr of "qemu-dummy,1=foo,2=bar"?
Is that rejected (because '1' and '2' are not valid key names) or does
it try to create a 2-element list instead of the usual dict? That is,
if a list is something the user can trigger at the CLI, then we need
better error handling than just an assertion that we actually get a
QDict above.
But overall I like this patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
