Re: [Qemu-devel] [PATCH v2 2/2] vl: Partial support for non-scalar properties with -object

[Markus Armbruster]

Daniel P. Berrangé <address@hidden> writes:

> On Mon, Aug 14, 2017 at 01:24:17PM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <address@hidden> writes:
>> 
>> > On Fri, Aug 11, 2017 at 12:47:44PM -0500, Eric Blake wrote:
>> >> On 08/11/2017 11:05 AM, Markus Armbruster wrote:
>> >> > We've wanted -object to support non-scalar properties for a while.
>> >> > Dan Berrange tried in "[PATCH v4 00/10]Provide a QOM-based
>> >> > authorization API".  Review led to the conclusion that we need to
>> >> > replace rather than add to QemuOpts.  Initial work towards that goal
>> >> > has been merged to provide -blockdev (commit 8746709), but there's
>> >> > substantial work left, mostly due to an bewildering array of
>> >> > compatibility problems.
>> >> > 
>> >> > Even if a full solution is still out of reach, we can have a partial
>> >> > solution now: accept -object argument in JSON syntax.  This should
>> >> > unblock development work that needs non-scalar properties with
>> >> > -object.
>> >> > 
>> >> > The implementation is similar to -blockdev, except we use the new
>> >> > infrastructure only for the new JSON case, and stick to QemuOpts for
>> >> > the existing KEY=VALUE,... case, to sidestep compatibility problems.
>> >> > 
>> >> > If we did this for more options, we'd have to factor out common code.
>> >> > But for one option, this will do.
>> >> > 
>> >> > Signed-off-by: Markus Armbruster <address@hidden>
>> >> > ---
>> >> >  qapi-schema.json | 14 +++++++++++---
>> >> >  vl.c             | 51 
>> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> >  2 files changed, 62 insertions(+), 3 deletions(-)
>> >> > 
>> >> >  static void object_create(bool (*type_predicate)(const char *))
>> >> >  {
>> >> > +    ObjectOptionsQueueEntry *e, *next;
>> >> > +
>> >> > +    QSIMPLEQ_FOREACH_SAFE(e, &oo_queue, entry, next) {
>> >> > +        if (!type_predicate(e->oo->qom_type)) {
>> >> > +            continue;
>> >> > +        }
>> >> > +
>> >> > +        loc_push_restore(&e->loc);
>> >> > +        qmp_object_add(e->oo->qom_type, e->oo->id,
>> >> > +                       e->oo->has_props, e->oo->props, &error_fatal);
>> >> > +        loc_pop(&e->loc);
>> >> > +
>> >> > +        QSIMPLEQ_REMOVE(&oo_queue, e, ObjectOptionsQueueEntry, entry);
>> >> > +        qapi_free_ObjectOptions(e->oo);
>> >> > +    }
>> >> > +
>> >> >      if (qemu_opts_foreach(qemu_find_opts("object"),
>> >> 
>> >> This handles all JSON forms prior to any QemuOpt forms (within the two
>> >> priority levels), such that a command line using:
>> >> 
>> >>  -object type,id=1,oldstyle... -object '{'id':2, 'type':..., newstyle...}'
>> >> 
>> >> processes the arguments in a different order than
>> >> 
>> >>  -object type,id=1,oldstyle... -object type,id=2,oldstyle
>> >> 
>> >> But I don't see that as too bad (ideally, someone using the {} JSON
>> >> style will use it consistently).
>> >
>> > I don't really like such a constraint - the ordering of object
>> > creation is already complex with some objets created at a different
>> > point in startup to other objects. Adding yet another constraint
>> > feels like it is painting ourselves into a corner wrt future changes.
>> 
>> The full solution will evaluate -object left to right.
>> 
>> This partial solution doesn't, but it's not meant for use in anger, just
>> for unblocking development work.  Can add scary warnings to deter
>> premature use.
>> 
>> > In particular I think it is quite possible to use the dotted
>> > form primarily, and only use JSON for the immediate scenario
>> > where non-JSON form won't work - I expect that's how we would
>> > use it in libvirt - I don't see libvirt changing 100% to JSON
>> > based objects
>> 
>> You need the JSON form anyway for QMP, and for the cases where dotted
>> keys break down.  Doing both just for the command line requires code to
>> do dotted keys (which may already exist), and code to decide whether it
>> can be used (which probably doesn't exist, yet).
>> 
>> Wouldn't this add complexity?  For what benefit exactly?
>
> Surprisingly, it appears we do actually have code that generates the
> JSON syntax for (probably) all uses of -object today. In fact we are
> actually generating JSON and then converting it to dotted syntax in
> most cases, which I didn't realize when writing the above.
>
> We'll have to keep support for dotted syntax around a while for old
> QEMU versions, but it looks like we could reasonably easily switch
> to JSON syntax for all -object usage at the same time.

Excellent.