On Thu, Oct 13, 2016 at 02:35:38PM +0200, Markus Armbruster wrote:
> Cc: Kevin for discussion of QemuOpts dotted key convention
>
> "Daniel P. Berrange" <berra...@redhat.com> writes:
>
> > Currently qdict_crumple requires a totally flat QDict as its
> > input. i.e. all values in the QDict must be scalar types.
> >
> > In order to have backwards compatibility with the OptsVisitor,
> > qemu_opt_to_qdict() has a new mode where it may return a QList
> > for values in the QDict, if there was a repeated key. We thus
> > need to allow compound types to appear as values in the input
> > dict given to qdict_crumple().
> >
> > To avoid confusion, we sanity check that the user has not mixed
> > the old and new syntax at the same time. e.g. these are allowed
> >
> > foo=hello,foo=world,foo=wibble
> > foo.0=hello,foo.1=world,foo.2=wibble
> >
> > but this is forbidden
> >
> > foo=hello,foo=world,foo.2=wibble
>
> I understand the need for foo.bar=val. It makes it possible to specify
> nested dictionaries with QemuOpts.
>
> The case for foo.0=val is less clear. QemuOpts already supports lists,
> by repeating keys. Why do we need a second, wordier way to specify
> them?
Two reasons I did this. First blockdev already uses this foo.0=val
syntax, and I wanted to be compatible with blockdev, so it could be
converted to use this new code.
Second, using foo.0 syntax means that you can unambigously determine
whether a key is going to be a scalar or a list. This lets the
qdict_crumple() method convert the QemuOpts to a QDict without
needing to know anything about the QAPI schema.
Of course I later had to add hacks to the visitor to cope with
the bare repeated key syntax, so I lost some of that benefit.
Personally I still prefer the unambiguous syntax as it lets us
give clear error messages when users do unexpected things, instead
of say, silently ignoring all but the last key.
> Note that this second way creates entirely new failure modes and
> restrictions. Let me show using an example derived from one in
> qdict_crumple()'s contract:
>
> foo.0.bar=bla,foo.eek.bar=blubb
>
> Without the dotted key convention, this is perfectly fine: key
> "foo.0.bar" has the single value "bla", and key "foo.eek.bar" has
> the single value "blubb". Equivalent JSON would be
>
> { "foo.0.bar": "bla", "foo.eek.bar": "blubb" }
>
> With just the struct convention, it's still fine: it obviously means
> the same as JSON
>
> { "foo": { "0": { "bar": "bla" }, "eek": { "bar": "blubb" } } }
>
> Adding the list convention makes it invalid. It also outlaws a
> bunch of keys that would be just fine in JSON, namely any that get
> recognized as list index. Raise your hand if you're willing to bet
> real money on your predictions of what will be recognized as list
> index, without looking at the code. I'm not.
>
> I'm afraid I have growing doubts regarding the QemuOpts dotted key
> convention in general.
>
> The convention makes '.' a special character in keys, but only
> sometimes. If the key gets consumed by something that uses dotted key
> convention, '.' is special, and to get a non-special '.', you need to
> escape it by doubling. Else, it's not.
>
> Since the same key can be used differently by different code, the same
> '.' could in theory be both special and non-special. In practice, this
> would be madness.
>
> Adopting the dotted key convention for an existing QemuOpts option, say
> -object [PATCH 15], *breaks* existing command line usage of keys
> containing '.', because you now have to escape the '.'. Dan, I'm afraid
> you need to show that no such keys exist, or if they exist they don't
> matter.
I checked the things that I converted (eg -net, -object, -numa, etc),
but I didn't check -device since that's processed using different code.
>
> 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.
>
> It's probably too late to back out the dotted key convention completely.
> Kevin?
>
> Can we still back out the list part of the convention, and use repeated
> keys instead?
>
> If we're stuck with some form of the dotted key convention, can we at
> least make it a more integral part of QemuOpts rather than something
> bolted on as an afterthought? Here's my thinking on how that might be
> done:
The only issue with dropping the dotted list convention is compat
with the block layer code - we couldn't easily use this new visitor
logic to turn -drive into a QAPI BlockOptions object. Kevin's new
-blockdev arg would potentially be ok with it since its a new arg,
but IIUC, we would have to do some cleanup inside various block
driver impls, since block layer doesn't use the QAPI objects
internally - they all get converted back into QemuOpts :-(
>
> * Have a QemuOptsList flag @flat.
>
> * If @flat, QemuOpts behaves as it always has: the special characters
> are ',' and '=', and parsing a key=value,... string produces a list
> where each element represents one key=value from the string, in the
> same order.
>
> * If not @flat, '.' becomes an additional special character, and parsing
> a key=value,... string produces a dictionary, similar to the one you
> get now by converting with qemu_opts_to_qdict() and filtering through
> qdict_crumple().
>
> The difference to now is that you either always crumple, or not at all,
> and the meaning of '.' is unambiguous.
>
> I wish we had refrained from saddling QemuOpts with even more magic.
> Compared to this swamp, use of JSON on the command line looks rather
> appealing to me.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
