qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values
Date: Mon, 17 Oct 2016 11:43:49 -0400 (EDT)

> For me, a replacement should do structured values by providing suitable
> *value* syntax instead of hacking it into the keys:
> 
>     { "driver": "quorum",
>       "child": [ { "driver": "file", "filename": "disk1.img" },
>                  { "driver": "host_device", "filename=/dev/sdb" },
>                  { "driver": "nbd", "host": "localhost" } ] }
> 
> Note how the structure is obvious.  It isn't with dotted keys, not even
> if you order them sensibly (which users inevitably won't).
> 
> Also not that the value needs to be parsed by QemuOpts!  You can't leave
> it to the consumer of the QemuOpts, because if you did, you'd have to
> escape the commas.
> 
> If you'd rather invent syntax closer to QemuOpts than reuse JSON, you
> could try
> 
>     driver=quorum,
>     child=[{ driver=file, filename=disk1.img },
>            { driver=host_device, filename=/dev/sdb },
>            { driver=nbd, host=localhost } ]
> 
> I'd go with some existing syntax, though.  The one we already use is
> JSON.

In fact there is already "filename=json:{...}" support in the block layer.
By the way, abuse of QemuOpts dates back to http://wiki.qemu.org/Features/QCFG.

> Your dotted key convention requires two rules: 1. names must not look
> like integers, and 2. names must not contain '.'.
> 
> We can avoid rule 2 by requiring '.' to be escaped.  Dan's
> qdict_crumple() currently does that, to your surprise.  Adding the
> escaping to existing options is a compatibility break, however.  So, if
> names with '.' already exist, we can either break compatibility by
> renaming them, or break it by requiring the '.' to be escaped.

> * "device", qemu_device_opts in qdev-monitor.c
> 
>   This one pulls in qdev properties.  Properties violating rule 2 exist.

Which are they?  Only bus names?

> * "object", qemu_object_opts in vl.c
> 
>   This one pulls in properties of user-creatable objects.
> 
> * "machine", qemu_machine_opts in vl.c
> 
>   This one pulls in machine properties.

> > > I know we have keys containing '.' elsewhere, e.g. device "macio-ide"
> > > property "ide.0".  Our chronic inability to consistently restrict names
> > > in ABI to something sane is beyond foolish.
> >
> > I wanted to have a look at this example, but I can only find the string
> > "ide.0" used as a bus name in the sources, that is, a value rather than
> > a key.
> >
> > Do you have a pointer to the property definition that you mean?
> 
> We've gotten better at hiding property definitions...
> 
> "qemu-system-ppc -device macio-ide,help" shows the property:
> 
>     macio-ide.ide.0=child<IDE>

It is a bug that this property is shown in the help, because it's not
assignable (same for all other child<> properties).  I'd rather declare
other occurrences of "." in user-accessible property names to be bugs,
and break the ABI if there are any.

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]