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? 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 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: * 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.