From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45708) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwA4y-0004LZ-2t for qemu-devel@nongnu.org; Mon, 17 Oct 2016 11:43:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwA4t-00019k-7N for qemu-devel@nongnu.org; Mon, 17 Oct 2016 11:43:56 -0400 Date: Mon, 17 Oct 2016 11:43:49 -0400 (EDT) From: Paolo Bonzini Message-ID: <1842252195.4099505.1476719029214.JavaMail.zimbra@redhat.com> In-Reply-To: <87a8e3av2r.fsf@dusky.pond.sub.org> References: <1475246744-29302-1-git-send-email-berrange@redhat.com> <1475246744-29302-14-git-send-email-berrange@redhat.com> <87fuo0a0kl.fsf@dusky.pond.sub.org> <20161013144640.GN5803@noname.redhat.com> <87a8e3av2r.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Max Reitz , Andreas =?utf-8?Q?F=C3=A4rber?= > 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" } ] } > > Note how the structure is obvious. It isn't with dotted keys, not even > if you order them sensibly (which users inevitably won't). > > Also not that the value needs to be parsed by QemuOpts! You can't leave > it to the consumer of the QemuOpts, because if you did, you'd have to > escape the commas. > > If you'd rather invent syntax closer to QemuOpts than reuse JSON, you > could try > > driver=quorum, > child=[{ driver=file, filename=disk1.img }, > { driver=host_device, filename=/dev/sdb }, > { driver=nbd, host=localhost } ] > > I'd go with some existing syntax, though. The one we already use is > JSON. In fact there is already "filename=json:{...}" support in the block layer. By the way, abuse of QemuOpts dates back to http://wiki.qemu.org/Features/QCFG. > Your dotted key convention requires two rules: 1. names must not look > like integers, and 2. names must not contain '.'. > > We can avoid rule 2 by requiring '.' to be escaped. Dan's > qdict_crumple() currently does that, to your surprise. Adding the > escaping to existing options is a compatibility break, however. So, if > names with '.' already exist, we can either break compatibility by > renaming them, or break it by requiring the '.' to be escaped. > * "device", qemu_device_opts in qdev-monitor.c > > This one pulls in qdev properties. Properties violating rule 2 exist. Which are they? Only bus names? > * "object", qemu_object_opts in vl.c > > This one pulls in properties of user-creatable objects. > > * "machine", qemu_machine_opts in vl.c > > This one pulls in machine properties. > > > I know we have keys containing '.' elsewhere, e.g. device "macio-ide" > > > property "ide.0". Our chronic inability to consistently restrict names > > > in ABI to something sane is beyond foolish. > > > > I wanted to have a look at this example, but I can only find the string > > "ide.0" used as a bus name in the sources, that is, a value rather than > > a key. > > > > Do you have a pointer to the property definition that you mean? > > We've gotten better at hiding property definitions... > > "qemu-system-ppc -device macio-ide,help" shows the property: > > macio-ide.ide.0=child It is a bug that this property is shown in the help, because it's not assignable (same for all other child<> properties). I'd rather declare other occurrences of "." in user-accessible property names to be bugs, and break the ABI if there are any. Paolo