Re: [PATCH v11 09/11] qcow2_format.py: collect fields to dump in JSON format

[Andrey Shinkevich]

On 28.07.2020 14:09, Vladimir Sementsov-Ogievskiy wrote:
> 17.07.2020 11:14, Andrey Shinkevich wrote:
> > As __dict__ is being extended with class members we do not want to
> > print, add the to_dict() method to classes that returns a dictionary
> > with desired fields and their values. Extend it in subclass when
> > necessary to print the final dictionary in the JSON output which
> > follows.
> >
> > Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > ---
> >   tests/qemu-iotests/qcow2_format.py | 38 
> > ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/qcow2_format.py 
> > b/tests/qemu-iotests/qcow2_format.py
> > index 2921a27..19d29b8 100644
> > --- a/tests/qemu-iotests/qcow2_format.py
> > +++ b/tests/qemu-iotests/qcow2_format.py
...
> >     class Qcow2BitmapDirEntry(Qcow2Struct):
> >   @@ -190,6 +198,13 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
> >           super(Qcow2BitmapDirEntry, self).dump()
> >           self.bitmap_table.dump()
> >   +    def to_dict(self):
> > +        fields_dict = super().to_dict()
> > +        fields_dict.update(bitmap_table=self.bitmap_table)
> > +        bmp_name = dict(name=self.name)
> > +        bme_dict = {**bmp_name, **fields_dict}
>
> hmmm... I don't follow, why not simply
>
>            fields_dict = super().to_dict()
>            fields_dict['name'] = self.name
>            fields_dict['bitmap_table'] = self.bitmap_table
>            ?


The idea is to put the name ahead of the dict. It is the same to 
QcowHeaderExtension::to_dict(). The relevant comment will be supplied in 
the code.

The .update() will be replaced with the assignment operator.

Andrey


> > +        return bme_dict
> > +
...
> > @@ -303,6 +327,17 @@ class QcowHeaderExtension(Qcow2Struct):
> >           else:
> >               self.obj.dump()
> >   +    def to_dict(self):
> > +        fields_dict = super().to_dict()
> > +        ext_name = dict(name=self.Magic(self.magic))
> > +        he_dict = {**ext_name, **fields_dict}
>
> again, why not just add a field to fields_dict ?
>
> > +        if self.obj is not None:
> > +            he_dict.update(data=self.obj)
> > +        else:
> > +            he_dict.update(data_str=self.data_str)
> > +
> > +        return he_dict
> > +
...