Re: [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor

[Markus Armbruster]

Copying Kevin because similar issues exist in the block layer.

Gerd Hoffmann <address@hidden> writes:

> On Mi, 2015-06-17 at 09:50 +0200, Markus Armbruster wrote:
>> Copying László because his fingerprints are on OptsVisitor.
>> 
>> "Kővágó, Zoltán" <address@hidden> writes:
>> 
>> > The current OptsVisitor flattens the whole structure, if there are
>> > same named
>> > fields under different paths (like `in' and `out' in `Audiodev'),
>> > the current
>> > visitor can't cope with them (for example setting
>> > frequency=44100' will set the
>> > in's frequency to 44100 and leave out's frequency unspecified).
>> >
>> > This patch fixes it, by the following changes:
>> > 1) Specifying just the field name will apply to all fields that has the
>> >    specified name (this means it would set both in's and out's frequency to
>> >    44100 in the above example).

What if they have different types?

What if one of them can't take the value?

>> > 2) Optionally user can specify the path in the hierarchy. Names
>> > are separated by
>> >    a dot (e.g. `in.frequency', `foo.bar.something', etc). The user need not
>> >    specify the whole path, only the last few components
>> > (i.e. `bar.something' is
>> >    equivalent to `foo.bar.something' if only `foo' has a `bar'
>> > field). This way
>> >    1) is just a special case of this when only the last component
>> > is specified.
>> > 3) In case of an ambiguity (e.g
>> > frequency=44100,in.frequency=8000') the longest
>> >    matching (the most specific) path wins (so in this example,
>> > in's frequency
>> >    would become 8000, because `in.frequency' is more specific that
>> > frequency',
>> >    and out's frequency would become 44100, because only
>> > frequency' matches it).

The current rule for multiple assignments is "last one wins".  E.g. in

    -drive if=none,file=tmp.img,file=tmp.qcow2

file=tmp.qcow2 wins.

If I understand correctly, this patch amends the rule to "last most
specific one wins".  Correct?

>> Can you explain why the complexity is needed, i.e. why we can't just
>> require full paths always?
>
> Keeping the short names is required for -netdev backward compatibility.

I suspect mostly because NetLegacy and Netdev aren't flat unions.
Could be self-inflicted pain.

What about flattening them instead?  Assuming that's possible; I'd have
to try.

> Restricting to short or full (i.e. something= or foo.bar.something=, but
> disallow bar.something=) should not be a problem.  I'm not sure this
> simplifies things much though.  We have to build the full path anyway,
> and I think bar.something= is just a convenient thing we get almost for
> free ...

We've been bitten by convenience features before.  Adding them tends to
be cheap, but maintaining compatibility can become a terrible headache.