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

[Vladimir Sementsov-Ogievskiy]

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
> @@ -119,6 +119,9 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
>   
>               print('{:<25} {}'.format(f[2], value_str))
>   
> +    def to_dict(self):
> +        return dict((f[2], self.__dict__[f[2]]) for f in self.fields)

good!

> +
>   
>   class Qcow2BitmapExt(Qcow2Struct):
>   
> @@ -152,6 +155,11 @@ class Qcow2BitmapExt(Qcow2Struct):
>               print()
>               entry.dump()
>   
> +    def to_dict(self):
> +        fields_dict = super().to_dict()
> +        fields_dict.update(bitmap_directory=self.bitmap_directory)

No reason to use .update. Let's use usual notation:
           fields_dict['bitmap_directory'] = self.bitmap_directory

> +        return fields_dict
> +
>   
>   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
           
?

> +        return bme_dict
> +
>   
>   class Qcow2BitmapTableEntry:
>   
> @@ -205,6 +220,9 @@ class Qcow2BitmapTableEntry:
>           else:
>               self.type = 'all-zeroes'
>   
> +    def to_dict(self):
> +        return dict(type=self.type, offset=self.offset)
> +
>   
>   class Qcow2BitmapTable:
>   
> @@ -226,6 +244,9 @@ class Qcow2BitmapTable:
>           for i, entry in bitmap_table:
>               print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
>   
> +    def to_dict(self):
> +        return dict(entries=self.entries)

Probably, this could be just list of entries, not creating interleaving dict.. 
and then, the method should be to_json (returning something json-serializable), 
and return just list here.. But it may be refactored later.

> +
>   
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>   
> @@ -241,6 +262,9 @@ class QcowHeaderExtension(Qcow2Struct):
>               0x44415441: 'Data file'
>           }
>   
> +        def to_dict(self):
> +            return self.mapping.get(self.value, "<unknown>")
> +
>       fields = (
>           ('u32', Magic, 'magic'),
>           ('u32', '{}', 'length')
> @@ -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
> +
>       @classmethod
>       def create(cls, magic, data):
>           return QcowHeaderExtension(magic, len(data), data)
> @@ -405,3 +440,6 @@ class QcowHeader(Qcow2Struct):
>               print('Header extension:')
>               ex.dump()
>               print()
> +
> +    def to_dict(self):
> +        return super().to_dict()

this is useless. You have to_dict of super class automatically.
 


--
Best regards,
Vladimir