From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41090) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5CC8-0005i0-Ut for qemu-devel@nongnu.org; Wed, 17 Jun 2015 08:11:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5CC4-0004Wf-M8 for qemu-devel@nongnu.org; Wed, 17 Jun 2015 08:11:52 -0400 Received: from mail-wi0-x22d.google.com ([2a00:1450:400c:c05::22d]:36251) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5CC4-0004WS-CE for qemu-devel@nongnu.org; Wed, 17 Jun 2015 08:11:48 -0400 Received: by wicnd19 with SMTP id nd19so26861961wic.1 for ; Wed, 17 Jun 2015 05:11:47 -0700 (PDT) From: "=?UTF-8?B?S8WRdsOhZ8OzIFpvbHTDoW4=?=" Message-ID: <55816405.7090502@gmail.com> Date: Wed, 17 Jun 2015 14:11:49 +0200 MIME-Version: 1.0 References: <88a1cc0775d3d4f5262b31b9452f8acccc6bbb41.1434458391.git.DirtY.iCE.hu@gmail.com> <87oakeg4eo.fsf@blackfin.pond.sub.org> <1434530501.5549.23.camel@redhat.com> <87bngea8hi.fsf@blackfin.pond.sub.org> In-Reply-To: <87bngea8hi.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Gerd Hoffmann Cc: Kevin Wolf , =?UTF-8?B?TMOhc3psw7MgRXJzZQ==?= =?UTF-8?B?aw==?= , Michael Roth , qemu-devel@nongnu.org 2015-06-17 13:18 keltezéssel, Markus Armbruster írta: > Copying Kevin because similar issues exist in the block layer. > > Gerd Hoffmann 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" 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. >