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

[Daniel P. Berrange]

On Mon, Mar 21, 2016 at 05:27:24PM -0600, Eric Blake wrote:
> 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.  But I think your approach is indeed a bit nicer (rather than
> making the warty OptsVisitor even wartier, just avoid it).

My first attempt did indeed modify OptsVisitor, but I quickly
abandoned it since it ended up being quite complex code to
make it fit in with the pre-existing hack to supports lists
of scalars in OptsVisitor.  The QmpInputVisitor approach is
cleaner and simpler overall


> > 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
> > 
> > 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.

Heh, yeah

> > @@ -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?

Indeed

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|