Re: [Qemu-devel] [PATCH v5 01/11] qdict: implement a qdict_crumple method for un-flattening a dict

[Daniel P. Berrange]

On Thu, Jun 09, 2016 at 03:20:47PM +0200, Markus Armbruster wrote:
> I apologize for the lateness of this review.
> 
> "Daniel P. Berrange" <address@hidden> 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 <address@hidden>
> 
> 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 :|