diff mbox series

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

Message ID 1594973699-781898-10-git-send-email-andrey.shinkevich@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series iotests: Dump QCOW2 dirty bitmaps metadata | expand

Commit Message

Andrey Shinkevich July 17, 2020, 8:14 a.m. UTC 
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(+)

Comments

Vladimir Sementsov-Ogievskiy July 28, 2020, 11:09 a.m. UTC | #1 
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.
Andrey Shinkevich July 29, 2020, 5:56 a.m. UTC | #2 
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
>> +
...
Vladimir Sementsov-Ogievskiy Aug. 5, 2020, 7:49 a.m. UTC | #3 
29.07.2020 08:56, Andrey Shinkevich wrote:
> 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.

Not worth doing. Json is not human output, it's mostly for parsing, so using so hard magic in the code to sort fields as you want is not worth doing. And I'm not sure how much is it guaranteed to keep some ordering of dict fields, why can't it change from version to version?

> 
> 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
>>> +
> ...
diff mbox series

Patch

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)
+
 
 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)
+        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}
+        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)
+
 
 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}
+        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()