Re: [PATCH v10 10/10] qcow2_format.py: support dumping metadata in JSON format

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

14.07.2020 00:36, Andrey Shinkevich wrote:
> Implementation of dumping QCOW2 image metadata.
> The sample output:
> {
>      "Header_extensions": [
>          {
>              "name": "Feature table",
>              "magic": 1745090647,
>              "length": 192,
>              "data_str": "<binary>"
>          },
>          {
>              "name": "Bitmaps",
>              "magic": 595929205,
>              "length": 24,
>              "data": {
>                  "nb_bitmaps": 2,
>                  "reserved32": 0,
>                  "bitmap_directory_size": 64,
>                  "bitmap_directory_offset": 1048576,
>                  "bitmap_directory": [
>                      {
>                          "name": "bitmap-1",
>                          "bitmap_table_offset": 589824,
>                          "bitmap_table_size": 1,
>                          "flags": 2,
>                          "type": 1,
>                          "granularity_bits": 15,
>                          "name_size": 8,
>                          "extra_data_size": 0,
>                          "bitmap_table": {
>                              "entries": [
>                                  {
>                                      "type": "serialized",
>                                      "offset": 655360
>                                  },
>                                  ...
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Aha, so you actually do what I imagine in a previous patch, add a method returning dict (I'd call it just "to_dict", but that doesn't really matter). (Stil, I'm against keeping self.fields_dict attribute. Just make correct .to_dict() method for base class (which will utilize .fields), and call super().to_dict() and add some another fields in children classes (like it is done for dump() method)


> ---
>   tests/qemu-iotests/qcow2_format.py | 59 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 59 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index 83c3482..a263858 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -19,6 +19,15 @@
>   
>   import struct
>   import string
> +import json
> +
> +
> +class ComplexEncoder(json.JSONEncoder):
> +    def default(self, obj):
> +        if hasattr(obj, 'get_fields_dict'):
> +            return obj.get_fields_dict()
> +        else:
> +            return json.JSONEncoder.default(self, obj)
>   
>   
>   class Qcow2Field:
> @@ -112,6 +121,11 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
>           self.fields_dict = self.__dict__.copy()
>   
>       def dump(self, dump_json=None):
> +        if dump_json:
> +            print(json.dumps(self.get_fields_dict(), indent=4,
> +                             cls=ComplexEncoder))
> +            return

Now it is obvious, that this is enough: we can just make it a seprate method dump_json in Qcow2Struct and we'll never need to implement dump_json in other classes (only .to_dict). But really, with dump_json parameter other classes are wrong now: they call super().dump(dump_json) which dumps json, and they they print additional things in a non-json format.

> +
>           for f in self.fields:
>               value = self.__dict__[f[2]]
>               if isinstance(f[1], str):
> @@ -154,6 +168,9 @@ class Qcow2BitmapExt(Qcow2Struct):
>               print()
>               entry.dump()
>   
> +    def get_fields_dict(self):
> +        return self.fields_dict
> +
>   
>   class Qcow2BitmapDirEntry(Qcow2Struct):
>   
> @@ -199,6 +216,11 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>           super(Qcow2BitmapDirEntry, self).dump()
>           self.bitmap_table.dump()
>   
> +    def get_fields_dict(self):
> +        bmp_name = dict(name=self.name)
> +        bme_dict = {**bmp_name, **self.fields_dict}
> +        return bme_dict
> +
>   
>   class Qcow2BitmapTableEntry:
>   
> @@ -214,6 +236,9 @@ class Qcow2BitmapTableEntry:
>           else:
>               self.type = 'all-zeroes'
>   
> +    def get_fields_dict(self):
> +        return dict(type=self.type, offset=self.offset)
> +
>   
>   class Qcow2BitmapTable:
>   
> @@ -230,6 +255,18 @@ class Qcow2BitmapTable:
>           for i, entry in bitmap_table:
>               print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
>   
> +    def get_fields_dict(self):
> +        return dict(entries=self.entries)
> +
> +
> +class Qcow2HeaderExtensionsDoc:
> +
> +    def __init__(self, extensions):
> +        self.extensions = extensions
> +
> +    def get_fields_dict(self):
> +        return dict(Header_extensions=self.extensions)
> +
>   
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>   
> @@ -245,6 +282,9 @@ class QcowHeaderExtension(Qcow2Struct):
>               0x44415441: 'Data file'
>           }
>   
> +        def get_fields_dict(self):
> +            return self.mapping.get(self.value, "<unknown>")
> +
>       fields = (
>           ('u32', Magic, 'magic'),
>           ('u32', '{}', 'length')
> @@ -303,6 +343,16 @@ class QcowHeaderExtension(Qcow2Struct):
>           else:
>               self.obj.dump(dump_json)
>   
> +    def get_fields_dict(self):
> +        ext_name = dict(name=self.Magic(self.magic))
> +        he_dict = {**ext_name, **self.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)
> @@ -401,7 +451,16 @@ class QcowHeader(Qcow2Struct):
>           fd.write(buf)
>   
>       def dump_extensions(self, dump_json=None):
> +        if dump_json:
> +            ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
> +            print(json.dumps(ext_doc.get_fields_dict(), indent=4,
> +                             cls=ComplexEncoder))
> +            return
> +
>           for ex in self.extensions:
>               print('Header extension:')
>               ex.dump(dump_json)
>               print()
> +
> +    def get_fields_dict(self):
> +        return self.fields_dict
> 


-- 
Best regards,
Vladimir