From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48768) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bB015-0004Fj-QF for qemu-devel@nongnu.org; Thu, 09 Jun 2016 09:29:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bB013-0006R2-7E for qemu-devel@nongnu.org; Thu, 09 Jun 2016 09:28:58 -0400 Date: Thu, 9 Jun 2016 14:28:48 +0100 From: "Daniel P. Berrange" Message-ID: <20160609132847.GE7218@redhat.com> Reply-To: "Daniel P. Berrange" References: <1464885987-4039-1-git-send-email-berrange@redhat.com> <1464885987-4039-2-git-send-email-berrange@redhat.com> <87shwma47k.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87shwma47k.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v5 01/11] qdict: implement a qdict_crumple method for un-flattening a dict List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz , Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= On Thu, Jun 09, 2016 at 03:20:47PM +0200, Markus Armbruster wrote: > I apologize for the lateness of this review. > > "Daniel P. Berrange" writes: > > > The qdict_flatten() method will take a dict whose elements are > > further nested dicts/lists and flatten them by concatenating > > keys. > > > > The qdict_crumple() method aims to do the reverse, taking a flat > > qdict, and turning it into a set of nested dicts/lists. It will > > apply nesting based on the key name, with a '.' indicating a > > new level in the hierarchy. If the keys in the nested structure > > are all numeric, it will create a list, otherwise it will create > > a dict. > > > > If the keys are a mixture of numeric and non-numeric, or the > > numeric keys are not in strictly ascending order, an error will > > be reported. > > > > As an example, a flat dict containing > > > > { > > 'foo.0.bar': 'one', > > 'foo.0.wizz': '1', > > 'foo.1.bar': 'two', > > 'foo.1.wizz': '2' > > } > > > > will get turned into a dict with one element 'foo' whose > > value is a list. The list elements will each in turn be > > dicts. > > > > { > > 'foo': [ > > { 'bar': 'one', 'wizz': '1' }, > > { 'bar': 'two', 'wizz': '2' } > > ], > > } > > > > If the key is intended to contain a literal '.', then it must > > be escaped as '..'. ie a flat dict > > > > { > > 'foo..bar': 'wizz', > > 'bar.foo..bar': 'eek', > > 'bar.hello': 'world' > > } > > > > Will end up as > > > > { > > 'foo.bar': 'wizz', > > 'bar': { > > 'foo.bar': 'eek', > > 'hello': 'world' > > } > > } > > > > The intent of this function is that it allows a set of QemuOpts > > to be turned into a nested data structure that mirrors the nesting > > used when the same object is defined over QMP. > > > > Signed-off-by: Daniel P. Berrange > > Not an objection to your patch, just an observation: we're digging > ourselves ever deeper into the QemuOpts / QDict hole. We've pushed > QemuOpts far beyond its original purpose "parse key=value,... option > arguments". In my opinion, we'd better parse straight to QAPI-generated > structs. NB we're not actually digging the hole any deeper here. The BlockDev code already fully dug the hole. This code is just generalizing the code for dealing with the hole (intentionally designed to 100% mirror the syntax that BlockDev already defined), so that we get consistent handling across the codebase, rather than having many separate subtley different holes :-) I have patches that I've not posted yet, which start to convert some of the blockdev code over to use this method. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|