qemu-devel
[Top][All Lists]
Advanced

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

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


From: Kővágó Zoltán
Subject: Re: [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor
Date: Wed, 17 Jun 2015 14:11:49 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

2015-06-17 13:18 keltezéssel, Markus Armbruster írta:
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?

Currently it will error out, requiring the user to be more explicit. Probably not the best solution, but I can't really think of a better solution. (If we would ignore invalid values that would be very confusing imho.)

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?

Yes. (But I didn't really checked that as I didn't know about the "last one win", and just thought it's an artifact of the current implementation.)

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.





reply via email to

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