All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Kővágó Zoltán" <dirty.ice.hu@gmail.com>
To: Markus Armbruster <armbru@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"László Ersek" <lersek@redhat.com>,
	"Michael Roth" <mdroth@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor
Date: Wed, 17 Jun 2015 14:11:49 +0200	[thread overview]
Message-ID: <55816405.7090502@gmail.com> (raw)
In-Reply-To: <87bngea8hi.fsf@blackfin.pond.sub.org>

2015-06-17 13:18 keltezéssel, Markus Armbruster írta:
> Copying Kevin because similar issues exist in the block layer.
>
> Gerd Hoffmann <kraxel@redhat.com> 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" <dirty.ice.hu@gmail.com> 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	other threads:[~2015-06-17 12:11 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-16 12:49 [Qemu-devel] [PATCH v2 0/6] -audiodev option Kővágó, Zoltán
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 1/6] qapi: qapi for audio backends Kővágó, Zoltán
2015-06-17  7:46   ` Markus Armbruster
2015-06-17 10:54     ` Kővágó Zoltán
2015-06-17 11:48       ` Markus Armbruster
2015-06-17 12:07         ` Kővágó Zoltán
2015-06-17 13:37           ` Markus Armbruster
2015-06-17 13:53             ` Kővágó Zoltán
2015-06-17 16:06               ` Markus Armbruster
2015-06-18  0:21                 ` Kővágó Zoltán
2015-06-18  8:51                   ` Markus Armbruster
2015-06-17 15:50         ` Eric Blake
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
2015-06-17  7:50   ` Markus Armbruster
2015-06-17  8:41     ` Gerd Hoffmann
2015-06-17 11:01       ` Kővágó Zoltán
2015-06-17 11:50         ` Markus Armbruster
2015-06-17 15:47         ` Eric Blake
2015-06-17 11:18       ` Markus Armbruster
2015-06-17 12:11         ` Kővágó Zoltán [this message]
2015-06-17 13:41           ` Markus Armbruster
2015-06-17 14:02             ` Kővágó Zoltán
2015-06-17 16:10               ` Markus Armbruster
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 3/6] opts: do not print separator before first item in qemu_opts_print Kővágó, Zoltán
2015-06-17  7:53   ` Markus Armbruster
2015-06-17  9:02   ` Kevin Wolf
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 4/6] qapi: AllocVisitor Kővágó, Zoltán
2015-06-17  7:56   ` Markus Armbruster
2015-06-17 12:01     ` Kővágó Zoltán
2015-06-17 13:42       ` Markus Armbruster
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 5/6] audio: use qapi AudioFormat instead of audfmt_e Kővágó, Zoltán
2015-06-17  8:01   ` Markus Armbruster
2015-06-17 11:05     ` Kővágó Zoltán
2015-06-17 11:51       ` Markus Armbruster
2015-06-17 16:01         ` Eric Blake
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 6/6] audio: -audiodev command line option Kővágó, Zoltán
2015-06-17  8:13   ` Markus Armbruster
2015-06-17 11:18     ` Kővágó Zoltán
2015-06-17 12:27       ` Markus Armbruster
2015-06-17 13:25         ` Kővágó Zoltán
2015-06-17 16:13           ` Markus Armbruster
2015-06-18  6:54             ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55816405.7090502@gmail.com \
    --to=dirty.ice.hu@gmail.com \
    --cc=armbru@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.