Re: [Qemu-block] [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 10/17/2016 09:50 AM, Markus Armbruster wrote:
>>> But even if I realised that QemuOpts support this syntax, I think we
>>> would still have to use the dotted syntax because it's explicit about
>>> the index and we need that because the list can contains dicts.
>>>
>>> Compare this:
>>>
>>>     driver=quorum,
>>>     child.0.driver=file,child.0.filename=disk1.img,
>>>     child.1.driver=host_device,child.1.filename=/dev/sdb,
>>>     child.2.driver=nbd,child.2.host=localhost
>>>
>>> And this:
>>>
>>>     driver=quorum,
>>>     child.driver=file,child.filename=disk1.img,
>>>     child.driver=host_device,child.filename=/dev/sdb,
>>>     child.driver=nbd,child.host=localhost
>> 
>> Aside: both are about equally illegible, to be honest.
>
>> Permit me to digress.
>> 
>> QemuOpts wasn't designed for list-values keys.  Doing lists by
>> repetition was clever.
>> 
>> QemuOpts wasn't designed for structured values.  Doing structured values
>> by a dotted key convention plus repetition was clever.
>> 
>> And there's the problem: too much cleverness, not enough "this is being
>> pushed way beyond its design limits, time to replace it".
>> 
>> 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" } ] }
>
> Possible hack solution:
>
> QemuOpts already special-cases id=.  What if we ALSO make it
> special-case a leading json=?  Shown here with shell quoting, the above
> example of creating a Quorum -drive argument could then be:
>
> -drive json='
>     { "driver": "quorum",
>       "child": [ { "driver": "file", "filename": "disk1.img" },
>                  { "driver": "host_device", "filename=/dev/sdb" },
>                  { "driver": "nbd", "host": "localhost" } ] }
> '
>
> As far as I know, we don't have 'json' as any existing QemuOpts key (do
> we? A full audit may be better than my quick git grep '"json"').  Thus,
> if QemuOpts sees a leading json=, it hands off the rest of the string to
> the same parser as we use for QMP, where we no longer have to escape
> commas (even nicer than the drive hack where we support
> filename=json:{...} but have to double up all commas to make it through
> the QemuOpts layer).  Encountering json= as anything other than the
> first option would be an error, and you would be unable to combine a
> json= option with any other old-style option.  In other words, the use
> of leading json= would be the switch for whether to do old-style parsing
> or to use a saner syntax for everything else that needs structure, on a
> per-argument basis.

Slight variation: omit the 'json=' and recognize the '{':

    -drive '{ "driver": "quorum",
              "child": [ { "driver": "file", "filename": "disk1.img" },
                         { "driver": "host_device", "filename=/dev/sdb" },
                         { "driver": "nbd", "host": "localhost" } ] }'

As always when extending haphazardly defined syntax, the question to ask
is whether this makes the syntax (more) ambiguous.

So what is the option argument syntax now?  The abstract syntax is
simple enough: "list of (key, value) pairs".  For the concrete syntax,
we need to study opts_do_parse().

Each iteration of its loop accepts an abstract (key, value).  It first
looks for the leftmost '=' and ','.  Cases:

1. There is no '=', or it is to the right of the leftmost ','.  In other
words, this (key, value) can only be key with an implied value or a
value with an implied key.

1a. If this is the first iteration, and we accept an implied key, this
is a value for the implied key.  We consume everything up to the first
non-escaped ','.  This may be more than the leftmost ',' we found above.
The consumed string with escapes processed is the value.

1b. Else, this is a key with an implied value.  We consume everything up
to the leftmost ',' (no escaping here).

1b1. If the consumed string starts with "no", the key is everything after
the "no" and the value is "off".

1b2. Else the key is the string and the value is "on".

2. This is a key and a value.  We first consume everything up to the
leftmost '=' (no escaping here).  The consumed string is the key.  We
then consume '='.  Finally, we consume everything up to the first
non-escaped ','The consumed string with escapes processed is the value.

Thus, the option argument starts either with a key (case 1b1, 2), "no"
(case 1b2) or a value (case 1a).

Adding JSON object syntax (which always starts with '{') is ambiguous
when a key can start with '{' (case 1b1, 2) or when a value can (case
1a).

Keys starting with '{' are basically foolish.  Let's outlaw them by
adopting QAPI's name rules.

Values starting with '{' are possible.  The implied keys I can remember
don't have such values, though.  If we interpret '{' as JSON, then users
can't omit the key when the value starts with '{'.  Not pretty, but I'd
say it's tolerable.