qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v14 02/21] qdict: implement a qdict


From: Markus Armbruster
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v14 02/21] qdict: implement a qdict_crumple method for un-flattening a dict
Date: Mon, 24 Oct 2016 11:18:29 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Max Reitz <address@hidden> writes:

> On 21.10.2016 11:58, Markus Armbruster wrote:
>> "Daniel P. Berrange" <address@hidden> writes:
>> 
>>> On Tue, Oct 18, 2016 at 04:32:13PM +0200, Markus Armbruster wrote:
>>>> "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.
>>>>>
>>>>> Reviewed-by: Eric Blake <address@hidden>
>>>>> Reviewed-by: Kevin Wolf <address@hidden>
>>>>> Reviewed-by: Marc-André Lureau <address@hidden>
>>>>> Signed-off-by: Daniel P. Berrange <address@hidden>
>>>>> ---
>>>>>  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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]