Re: [Qemu-block] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
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 :|
Re: [Qemu-block] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
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).
>
> 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
>
> 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
> ---
> 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?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-block] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
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.
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.
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
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
Signed-off-by: Daniel P. Berrange
---
hmp.c | 18 +--
qom/object_interfaces.c| 20 ++-
tests/check-qom-proplist.c | 295 -
3 files changed, 313 insertions(+), 20 deletions(-)
diff --git a/hmp.c b/hmp.c
index 5b6084a..7a98726 100644
--- a/hmp.c
+++ b/hmp.c
@@ -25,7 +25,7 @@
#include "qemu/sockets.h"
#include "monitor/monitor.h"
#include "monitor/qdev.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qmp-input-visitor.h"
#include "qapi/qmp/qerror.h"
#include "qapi/string-output-visitor.h"
#include "qapi/util.h"
@@ -1673,20 +1673,12 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
-QemuOpts *opts;
-OptsVisitor *ov;
+QmpInputVisitor *qiv;
Object *obj = NULL;
-opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
-if (err) {
-hmp_handle_error(mon, &err);
-return;
-}
-
-ov = opts_visitor_new(opts);
-obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
-opts_visitor_cleanup(ov);
-qemu_opts_del(opts);
+qiv = qmp_input_visitor_new_full((QObject *)qdict, true, true);
+obj = user_creatable_add(qdict, qmp_input_get_visitor(qiv), &err);
+qmp_input_visitor_cleanup(qiv);
if (err) {
hmp_handle_error(mon, &err);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index c2f6e29..9c41730 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -1,9 +1,9 @@
#include "qemu/osdep.h"
#include "qom/object_interfaces.h"
#include "qemu/module.h"
+#include "qemu/option.h"
#include "qapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qmp-input-visitor.h"
void user_creatable_complete(Object *obj, Error **errp)
{
@@ -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;
@@ -151,15 +152,22 @@ out:
Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
{
-OptsVisitor *ov;
+QmpInputVisitor *qiv;
QDict *pdict;
+QObject *pobj;
Object *obj = NULL;
-ov = opts_visitor_new(opts);
pdict = qemu_opts_to_qdict(opts, NULL);
+pobj = qdict_crumple(pdict, true, errp);
+if (!pobj) {
+goto cleanup;
+}
+qiv = qmp_input_visitor_new_full(pobj, true, true);
-obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
-opts_v
