Max Reitz <mre...@redhat.com> writes: > On 21.10.2016 11:58, Markus Armbruster wrote: >> "Daniel P. Berrange" <berra...@redhat.com> writes: >> >>> On Tue, Oct 18, 2016 at 04:32:13PM +0200, Markus Armbruster wrote: >>>> "Daniel P. Berrange" <berra...@redhat.com> 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. >>>>> >>>>> Reviewed-by: Eric Blake <ebl...@redhat.com> >>>>> Reviewed-by: Kevin Wolf <kw...@redhat.com> >>>>> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> >>>>> Signed-off-by: Daniel P. Berrange <berra...@redhat.com> >>>>> --- >>>>> include/qapi/qmp/qdict.h | 1 + >>>>> qobject/qdict.c | 289 >>>>> +++++++++++++++++++++++++++++++++++++++++++++++ >>>>> tests/check-qdict.c | 261 ++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 551 insertions(+) >>>>> >>>>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h >>>>> index 71b8eb0..e0d24e1 100644 >>>>> --- a/include/qapi/qmp/qdict.h >>>>> +++ b/include/qapi/qmp/qdict.h >>>>> @@ -73,6 +73,7 @@ void qdict_flatten(QDict *qdict); >>>>> void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start); >>>>> void qdict_array_split(QDict *src, QList **dst); >>>>> int qdict_array_entries(QDict *src, const char *subqdict); >>>>> +QObject *qdict_crumple(const QDict *src, bool recursive, Error **errp); >>>>> >>>>> void qdict_join(QDict *dest, QDict *src, bool overwrite); >>>>> >>>>> diff --git a/qobject/qdict.c b/qobject/qdict.c >>>>> index 60f158c..c38e90e 100644 >>>>> --- a/qobject/qdict.c >>>>> +++ b/qobject/qdict.c >>>> [...] >>>>> +/** >>>>> + * qdict_crumple: >>>>> + * @src: the original flat dictionary (only scalar values) to crumple >>>>> + * @recursive: true to recursively crumple nested dictionaries >>>> >>>> Is recursive=false used outside tests in this series? >>> >>> No, its not used. >>> >>> It was suggested in a way earlier version by Max, but not sure if his >>> code uses it or not. >> >> In general, I prefer features to be added right before they're used, and >> I really dislike adding features "just in case". YAGNI. >> >> Max, do you actually need this one? If yes, please explain your use >> case. > > As far as I can tell from a quick glance, I made the point for v1 that > qdict_crumple() could be simplified by using qdict_array_split() and > qdict_array_entries(). > > Dan then (correctly) said that using these functions would worsen > runtime performance of qdict_crumple() and that instead we can replace > qdict_array_split() by qdict_crumple(). However, for that to work, we > need to be able to make qdict_crumple() non-recursive (because > qdict_array_split() is non-recursive). > > Dan also said that in the long run we want to keep the tree structure in > the block layer instead of flattening everything down which would rid us > of several other QDict functions (and would make non-recursive behavior > obsolete again). I believe that this is an idea for the (far?) future, > as can be seen from the discussion you and others had on this very topic > in this version here.
In the long run, the block layer should use proper C types instead of mucking around with QDict. That's what QAPI is for. > However, clearly there are no patches out yet that replace > qdict_array_split() by qdict_crumple(recursive=false), and I certainly > won't write any until qdict_crumple() is merged. All right, I'll go hunting for prior versions of qdict_crumple() to see whether I find a suitable one without @recursive.