Re: [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 03/10/2016 11:59 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.
>> 
>> 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.
>> 
>> It is not practical to extend the OptsVisitor to support
>> more complex data structures while also maintaining
>> the existing list handling behaviour that is relied upon
>> by other areas of QEMU.
>
> Zoltán Kővágó tried earlier with his GSoC patches for the audio
> subsystem last year, but those got stalled waiting for qapi enhancements
> to go in.

Yet another series stalled on the big QAPI rework.  Hitting a GSoC
student that way is really unfortunate.

>            But I think your approach is indeed a bit nicer (rather than
> making the warty OptsVisitor even wartier, just avoid it).

QemuOpts defines an important part of the command line language, namely
(most of) the syntax of many option arguments.  It parses them into a
set of (name, value) pairs.

"Most of", because additional syntax may hide in the parameter value.

Parameter values are typed, except when they aren't.  Types are limited
to string, bool, uint64_t number (accepts negative numbers and casts
them) and uint64_t size (rejects negative numbers, accepts suffixes).

OptsVisitor adds special list syntax.  It's used with untyped values.

Bypassing OptsVisitor risks adding different special syntax.  Doesn't
mean it's a bad idea, only that we need to keep close watch on what it
does to the language.  See below.

I merely scratched the surprising amount of complexity that has accrued
over time.  If you doubt me, study how the merge_lists flag works, and
how it interacts with the several ways we do defaults.

The gap between QemuOpts' and QAPI's type system is obvious.  But it's
not just a gap, it's also contraditions: QAPI does only *signed*
integers.

Nevertheless, the path from command line to QAPI is through QemuOpts for
now.

Aside: for historical reasons, we have a few QMP commands that detour
through QemuOpts.  Mistakes, do not add more.

Ceterum censeo QemuOpts esse delendam.  And I say that as author of 35
out of 117 commits touching qemu-option.c.

>> Fortunately there is no existing object that implements
>> the UserCreatable interface that relies on the list
>> handling behaviour, so it is possible to swap out the
>> OptsVisitor for a different visitor implementation, so
>> -object supports non-scalar properties, thus leaving
>> other users of OptsVisitor unaffected.
>> 
>> 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
>> use with -object.
>
> Indeed, nice replacement.
>
>> 
>> 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

Okay, this adds a bare minimum of syntax, and it's not even new: we use
similar syntax for block stuff already.

>> 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
>
> Maybe mention that the indentation is not actually present in the real
> command lines typed.
>
>> 
>> Signed-off-by: Daniel P. Berrange <address@hidden>
>> ---
>>  hmp.c                      |  18 +--
>>  qom/object_interfaces.c    |  20 ++-
>>  tests/check-qom-proplist.c | 295 
>> ++++++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 313 insertions(+), 20 deletions(-)
>> 
>
>> @@ -120,6 +120,7 @@ Object *user_creatable_add_type(const char *type, const 
>> char *id,
>>      obj = object_new(type);
>>      if (qdict) {
>>          for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
>> +
>>              object_property_set(obj, v, e->key, &local_err);
>>              if (local_err) {
>>                  goto out;
>
> Spurious hunk?