[PATCH v11 00/11] iotests: Dump QCOW2 dirty bitmaps metadata

[PATCH v11 00/11] iotests: Dump QCOW2 dirty bitmaps metadata

[Andrey Shinkevich]

Add dirty bitmap information to QCOW2 metadata dump in the qcow2_format.py.

v10:
  01: Fixing of issues in QCOW2 extension classes noted by Vladimir.
  02: Reading bitmap tables was moved into Qcow2BitmapTable class.
  03: Handling '-j' key was moved into "if __name__" section.
  04: Making copy of __dict__ was replaced with the method to_dict().
  05: Qcow2HeaderExtensionsDoc is introduced in the separate patch.

Andrey Shinkevich (11):
  qcow2: Fix capitalization of header extension constant.
  qcow2_format.py: make printable data an extension class member
  qcow2_format.py: change Qcow2BitmapExt initialization method
  qcow2_format.py: dump bitmap flags in human readable way.
  qcow2_format.py: Dump bitmap directory information
  qcow2_format.py: pass cluster size to substructures
  qcow2_format.py: Dump bitmap table serialized entries
  qcow2.py: Introduce '-j' key to dump in JSON format
  qcow2_format.py: collect fields to dump in JSON format
  qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class
  qcow2_format.py: support dumping metadata in JSON format

 block/qcow2.c                      |   2 +-
 docs/interop/qcow2.txt             |   2 +-
 tests/qemu-iotests/qcow2.py        |  18 ++-
 tests/qemu-iotests/qcow2_format.py | 221 ++++++++++++++++++++++++++++++++++---
 4 files changed, 220 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[PATCH v11 01/11] qcow2: Fix capitalization of header extension constant.

[Andrey Shinkevich]

Make the capitalization of the hexadecimal numbers consistent for the
QCOW2 header extension constants in docs/interop/qcow2.txt.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.c          | 2 +-
 docs/interop/qcow2.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index fadf342..6ad6bdc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -66,7 +66,7 @@ typedef struct {
 } QEMU_PACKED QCowExtension;
 
 #define  QCOW2_EXT_MAGIC_END 0
-#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
+#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xe2792aca
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
 #define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
 #define  QCOW2_EXT_MAGIC_BITMAPS 0x23852875
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index cb72346..f072e27 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -231,7 +231,7 @@ be stored. Each extension has a structure like the following:
 
     Byte  0 -  3:   Header extension type:
                         0x00000000 - End of the header extension area
-                        0xE2792ACA - Backing file format name string
+                        0xe2792aca - Backing file format name string
                         0x6803f857 - Feature name table
                         0x23852875 - Bitmaps extension
                         0x0537be77 - Full disk encryption header pointer
-- 
1.8.3.1

Re: [PATCH v11 01/11] qcow2: Fix capitalization of header extension constant.

[Eric Blake]

On 7/17/20 3:14 AM, Andrey Shinkevich wrote:
> Make the capitalization of the hexadecimal numbers consistent for the
> QCOW2 header extension constants in docs/interop/qcow2.txt.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/qcow2.c          | 2 +-
>   docs/interop/qcow2.txt | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

This one is trivial; if I have a reason to do a bitmaps pull request for 
the next 5.1 -rc build, I'll include this too; if not, it doesn't hurt 
to wait for 5.2.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH v11 02/11] qcow2_format.py: make printable data an extension class member

[Andrey Shinkevich]

Let us differ binary data type from string one for the extension data
variable and keep the string as the QcowHeaderExtension class member.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index cc432e7..2f3681b 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -165,6 +165,13 @@ class QcowHeaderExtension(Qcow2Struct):
             self.data = fd.read(padded)
             assert self.data is not None
 
+        data_str = self.data[:self.length]
+        if all(c in string.printable.encode('ascii') for c in data_str):
+            data_str = f"'{ data_str.decode('ascii') }'"
+        else:
+            data_str = '<binary>'
+        self.data_str = data_str
+
         if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
             self.obj = Qcow2BitmapExt(data=self.data)
         else:
@@ -174,12 +181,7 @@ class QcowHeaderExtension(Qcow2Struct):
         super().dump()
 
         if self.obj is None:
-            data = self.data[:self.length]
-            if all(c in string.printable.encode('ascii') for c in data):
-                data = f"'{ data.decode('ascii') }'"
-            else:
-                data = '<binary>'
-            print(f'{"data":<25} {data}')
+            print(f'{"data":<25} {self.data_str}')
         else:
             self.obj.dump()
 
-- 
1.8.3.1

[PATCH v11 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method

[Andrey Shinkevich]

There are two ways to initialize a class derived from Qcow2Struct:
1. Pass a block of binary data to the constructor.
2. Pass the file descriptor to allow reading the file from constructor.
Let's change the Qcow2BitmapExt initialization method from 1 to 2 to
support a scattered reading in the initialization chain.
The implementation comes with the patch that follows.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 2f3681b..d4a9974 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -113,6 +113,11 @@ class Qcow2BitmapExt(Qcow2Struct):
         ('u64', '{:#x}', 'bitmap_directory_offset')
     )
 
+    def __init__(self, fd):
+        super().__init__(fd=fd)
+        tail = struct.calcsize(self.fmt) % 8
+        if tail:
+            fd.seek(8 - tail, 1)
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
@@ -161,21 +166,24 @@ class QcowHeaderExtension(Qcow2Struct):
         else:
             assert all(v is None for v in (magic, length, data))
             super().__init__(fd=fd)
-            padded = (self.length + 7) & ~7
-            self.data = fd.read(padded)
-            assert self.data is not None
-
-        data_str = self.data[:self.length]
-        if all(c in string.printable.encode('ascii') for c in data_str):
-            data_str = f"'{ data_str.decode('ascii') }'"
-        else:
-            data_str = '<binary>'
-        self.data_str = data_str
+            if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
+                self.obj = Qcow2BitmapExt(fd=fd)
+                self.data = None
+            else:
+                padded = (self.length + 7) & ~7
+                self.data = fd.read(padded)
+                assert self.data is not None
+                self.obj = None
+
+        if self.data is not None:
+            data_str = self.data[:self.length]
+            if all(c in string.printable.encode(
+                'ascii') for c in data_str):
+                data_str = f"'{ data_str.decode('ascii') }'"
+            else:
+                data_str = '<binary>'
+            self.data_str = data_str
 
-        if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-            self.obj = Qcow2BitmapExt(data=self.data)
-        else:
-            self.obj = None
 
     def dump(self):
         super().dump()
-- 
1.8.3.1

Re: [PATCH v11 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method

[Vladimir Sementsov-Ogievskiy]

17.07.2020 11:14, Andrey Shinkevich wrote:
> There are two ways to initialize a class derived from Qcow2Struct:
> 1. Pass a block of binary data to the constructor.
> 2. Pass the file descriptor to allow reading the file from constructor.
> Let's change the Qcow2BitmapExt initialization method from 1 to 2 to
> support a scattered reading in the initialization chain.
> The implementation comes with the patch that follows.
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich<andrey.shinkevich@virtuozzo.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

[PATCH v11 04/11] qcow2_format.py: dump bitmap flags in human readable way.

[Andrey Shinkevich]

Introduce the class BitmapFlags that parses a bitmap flags mask.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index d4a9974..b447344 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -40,6 +40,22 @@ class Flags64(Qcow2Field):
         return str(bits)
 
 
+class BitmapFlags(Qcow2Field):
+
+    flags = {
+        0x1: 'in-use',
+        0x2: 'auto'
+    }
+
+    def __str__(self):
+        bits = []
+        for bit in range(64):
+            flag = self.value & (1 << bit)
+            if flag:
+                bits.append(self.flags.get(flag, f'bit-{bit}'))
+        return f'{self.value:#x} ({bits})'
+
+
 class Enum(Qcow2Field):
 
     def __str__(self):
-- 
1.8.3.1

[PATCH v11 05/11] qcow2_format.py: Dump bitmap directory information

[Andrey Shinkevich]

Read and dump entries from the bitmap directory of QCOW2 image.

Header extension:
magic                     0x23852875 (Bitmaps)
...
Bitmap name               bitmap-1
bitmap_table_offset       0xf0000
bitmap_table_size         1
flags                     0x2 (['auto'])
type                      1
granularity_bits          16
name_size                 8
extra_data_size           0

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 47 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index b447344..05a8aa9 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -134,6 +134,53 @@ class Qcow2BitmapExt(Qcow2Struct):
         tail = struct.calcsize(self.fmt) % 8
         if tail:
             fd.seek(8 - tail, 1)
+        position = fd.tell()
+        self.read_bitmap_directory(fd)
+        fd.seek(position)
+
+    def read_bitmap_directory(self, fd):
+        fd.seek(self.bitmap_directory_offset)
+        self.bitmap_directory = \
+            [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+
+    def dump(self):
+        super().dump()
+        for entry in self.bitmap_directory:
+            print()
+            entry.dump()
+
+
+class Qcow2BitmapDirEntry(Qcow2Struct):
+
+    fields = (
+        ('u64', '{:#x}', 'bitmap_table_offset'),
+        ('u32', '{}', 'bitmap_table_size'),
+        ('u32', BitmapFlags, 'flags'),
+        ('u8',  '{}', 'type'),
+        ('u8',  '{}', 'granularity_bits'),
+        ('u16', '{}', 'name_size'),
+        ('u32', '{}', 'extra_data_size')
+    )
+
+    def __init__(self, fd):
+        super().__init__(fd=fd)
+        # Seek relative to the current position in the file
+        fd.seek(self.extra_data_size, 1)
+        bitmap_name = fd.read(self.name_size)
+        self.name = bitmap_name.decode('ascii')
+        # Move position to the end of the entry in the directory
+        entry_raw_size = self.bitmap_dir_entry_raw_size()
+        padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
+        fd.seek(padding, 1)
+
+    def bitmap_dir_entry_raw_size(self):
+        return struct.calcsize(self.fmt) + self.name_size + \
+            self.extra_data_size
+
+    def dump(self):
+        print(f'{"Bitmap name":<25} {self.name}')
+        super(Qcow2BitmapDirEntry, self).dump()
+
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
-- 
1.8.3.1

[PATCH v11 06/11] qcow2_format.py: pass cluster size to substructures

[Andrey Shinkevich]

The cluster size of an image is the QcowHeader class member and may be
obtained by dependent extension structures such as Qcow2BitmapExt for
further bitmap table details print.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 05a8aa9..ca0d350 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -129,19 +129,21 @@ class Qcow2BitmapExt(Qcow2Struct):
         ('u64', '{:#x}', 'bitmap_directory_offset')
     )
 
-    def __init__(self, fd):
+    def __init__(self, fd, cluster_size):
         super().__init__(fd=fd)
         tail = struct.calcsize(self.fmt) % 8
         if tail:
             fd.seek(8 - tail, 1)
         position = fd.tell()
+        self.cluster_size = cluster_size
         self.read_bitmap_directory(fd)
         fd.seek(position)
 
     def read_bitmap_directory(self, fd):
         fd.seek(self.bitmap_directory_offset)
         self.bitmap_directory = \
-            [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+            [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
+             for _ in range(self.nb_bitmaps)]
 
     def dump(self):
         super().dump()
@@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         ('u32', '{}', 'extra_data_size')
     )
 
-    def __init__(self, fd):
+    def __init__(self, fd, cluster_size):
         super().__init__(fd=fd)
+        self.cluster_size = cluster_size
         # Seek relative to the current position in the file
         fd.seek(self.extra_data_size, 1)
         bitmap_name = fd.read(self.name_size)
@@ -203,11 +206,13 @@ class QcowHeaderExtension(Qcow2Struct):
         # then padding to next multiply of 8
     )
 
-    def __init__(self, magic=None, length=None, data=None, fd=None):
+    def __init__(self, magic=None, length=None, data=None, fd=None,
+                 cluster_size=None):
         """
         Support both loading from fd and creation from user data.
         For fd-based creation current position in a file will be used to read
         the data.
+        The cluster_size value may be obtained by dependent structures.
 
         This should be somehow refactored and functionality should be moved to
         superclass (to allow creation of any qcow2 struct), but then, fields
@@ -230,7 +235,7 @@ class QcowHeaderExtension(Qcow2Struct):
             assert all(v is None for v in (magic, length, data))
             super().__init__(fd=fd)
             if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-                self.obj = Qcow2BitmapExt(fd=fd)
+                self.obj = Qcow2BitmapExt(fd=fd, cluster_size=cluster_size)
                 self.data = None
             else:
                 padded = (self.length + 7) & ~7
@@ -319,7 +324,7 @@ class QcowHeader(Qcow2Struct):
             end = self.cluster_size
 
         while fd.tell() < end:
-            ext = QcowHeaderExtension(fd=fd)
+            ext = QcowHeaderExtension(fd=fd, cluster_size=self.cluster_size)
             if ext.magic == 0:
                 break
             else:
-- 
1.8.3.1

[PATCH v11 07/11] qcow2_format.py: Dump bitmap table serialized entries

[Andrey Shinkevich]

Add bitmap table information to the QCOW2 metadata dump.

Bitmap name               bitmap-1
...
Bitmap table   type            offset                   size
0              serialized      4718592                  65536
1              serialized      4294967296               65536
2              serialized      5348033147437056         65536
3              serialized      13792273858822144        65536
4              serialized      4718592                  65536
5              serialized      4294967296               65536
6              serialized      4503608217305088         65536
7              serialized      14073748835532800        65536

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 41 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index ca0d350..ad1918c 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -175,6 +175,10 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         entry_raw_size = self.bitmap_dir_entry_raw_size()
         padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
         fd.seek(padding, 1)
+        self.bitmap_table = Qcow2BitmapTable(fd=fd,
+                                             offset=self.bitmap_table_offset,
+                                             size=self.bitmap_table_size,
+                                             cluster_size=self.cluster_size)
 
     def bitmap_dir_entry_raw_size(self):
         return struct.calcsize(self.fmt) + self.name_size + \
@@ -183,6 +187,43 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
     def dump(self):
         print(f'{"Bitmap name":<25} {self.name}')
         super(Qcow2BitmapDirEntry, self).dump()
+        self.bitmap_table.dump()
+
+
+class Qcow2BitmapTableEntry:
+
+    BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
+    BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
+
+    def __init__(self, entry):
+        self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
+        if self.offset:
+            self.type = 'serialized'
+        elif entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
+            self.type = 'all-ones'
+        else:
+            self.type = 'all-zeroes'
+
+
+class Qcow2BitmapTable:
+
+    def __init__(self, fd, offset, size, cluster_size):
+        self.entries = []
+        self.cluster_size = cluster_size
+        table_size = size * 8
+        position = fd.tell()
+        fd.seek(offset)
+        table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
+        fd.seek(position)
+        for entry in table:
+            self.entries.append(Qcow2BitmapTableEntry(entry))
+
+    def dump(self):
+        size = self.cluster_size
+        bitmap_table = enumerate(self.entries)
+        print(f'{"Bitmap table":<14} {"type":<15} {"offset":<24} {"size"}')
+        for i, entry in bitmap_table:
+            print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
 
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
-- 
1.8.3.1

Re: [PATCH v11 07/11] qcow2_format.py: Dump bitmap table serialized entries

[Vladimir Sementsov-Ogievskiy]

17.07.2020 11:14, Andrey Shinkevich wrote:
> Add bitmap table information to the QCOW2 metadata dump.
> 
> Bitmap name               bitmap-1
> ...
> Bitmap table   type            offset                   size
> 0              serialized      4718592                  65536
> 1              serialized      4294967296               65536
> 2              serialized      5348033147437056         65536
> 3              serialized      13792273858822144        65536
> 4              serialized      4718592                  65536
> 5              serialized      4294967296               65536
> 6              serialized      4503608217305088         65536
> 7              serialized      14073748835532800        65536

big numbers seems unrelated. Did you updated commit-msg after fixing s/* 8 * 8/* 8/ bug of v10?

> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2_format.py | 41 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index ca0d350..ad1918c 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -175,6 +175,10 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>           entry_raw_size = self.bitmap_dir_entry_raw_size()
>           padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
>           fd.seek(padding, 1)
> +        self.bitmap_table = Qcow2BitmapTable(fd=fd,
> +                                             offset=self.bitmap_table_offset,
> +                                             size=self.bitmap_table_size,
> +                                             cluster_size=self.cluster_size)
>   
>       def bitmap_dir_entry_raw_size(self):
>           return struct.calcsize(self.fmt) + self.name_size + \
> @@ -183,6 +187,43 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>       def dump(self):
>           print(f'{"Bitmap name":<25} {self.name}')
>           super(Qcow2BitmapDirEntry, self).dump()
> +        self.bitmap_table.dump()
> +
> +
> +class Qcow2BitmapTableEntry:

Why not to derive it from Qcow2Struct? It will have only one field, but it will work, and we don't need to open-code loading in Qcow2BitmapTable

> +
> +    BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
> +    BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
> +
> +    def __init__(self, entry):
> +        self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
> +        if self.offset:
> +            self.type = 'serialized'
> +        elif entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
> +            self.type = 'all-ones'
> +        else:
> +            self.type = 'all-zeroes'

Would be good to note if reserved bits are set, as well as wrong combination of offset and all-ones bit.

> +
> +
> +class Qcow2BitmapTable:
> +
> +    def __init__(self, fd, offset, size, cluster_size):
> +        self.entries = []
> +        self.cluster_size = cluster_size
> +        table_size = size * 8

you may rename size to nb_entries, to make it obvious what "size" is (I know, that it's my idea to call it "size" in qcow2 spec. Probably bad one).

> +        position = fd.tell()
> +        fd.seek(offset)
> +        table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]

this should be self.entries = [Qcow2BitmapTableEntry(fd) for _ in range(nb_entries)]

> +        fd.seek(position)
> +        for entry in table:
> +            self.entries.append(Qcow2BitmapTableEntry(entry))
> +
> +    def dump(self):
> +        size = self.cluster_size
> +        bitmap_table = enumerate(self.entries)
> +        print(f'{"Bitmap table":<14} {"type":<15} {"offset":<24} {"size"}')
> +        for i, entry in bitmap_table:
> +            print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
>   
>   
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
> 


-- 
Best regards,
Vladimir

[PATCH v11 08/11] qcow2.py: Introduce '-j' key to dump in JSON format

[Andrey Shinkevich]

Add the command key to the qcow2.py arguments list to dump QCOW2
metadata in JSON format. Here is the suggested way to do that. The
implementation of the dump in JSON format is in the patch that follows.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/qcow2.py        | 18 ++++++++++++++----
 tests/qemu-iotests/qcow2_format.py |  5 +++--
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 0910e6a..77ca59c 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -26,16 +26,19 @@ from qcow2_format import (
 )
 
 
+is_json = False
+
+
 def cmd_dump_header(fd):
     h = QcowHeader(fd)
-    h.dump()
+    h.dump(is_json)
     print()
-    h.dump_extensions()
+    h.dump_extensions(is_json)
 
 
 def cmd_dump_header_exts(fd):
     h = QcowHeader(fd)
-    h.dump_extensions()
+    h.dump_extensions(is_json)
 
 
 def cmd_set_header(fd, name, value):
@@ -151,11 +154,14 @@ def main(filename, cmd, args):
 
 
 def usage():
-    print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0])
+    print("Usage: %s <file> <cmd> [<arg>, ...] [<key>, ...]" % sys.argv[0])
     print("")
     print("Supported commands:")
     for name, handler, num_args, desc in cmds:
         print("    %-20s - %s" % (name, desc))
+    print("")
+    print("Supported keys:")
+    print("    %-20s - %s" % ('-j', 'Dump in JSON format'))
 
 
 if __name__ == '__main__':
@@ -163,4 +169,8 @@ if __name__ == '__main__':
         usage()
         sys.exit(1)
 
+    is_json = '-j' in sys.argv
+    if is_json:
+        sys.argv.remove('-j')
+
     main(sys.argv[1], sys.argv[2], sys.argv[3:])
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index ad1918c..2921a27 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
         self.__dict__ = dict((field[2], values[i])
                              for i, field in enumerate(self.fields))
 
-    def dump(self):
+    def dump(self, is_json=False):
         for f in self.fields:
             value = self.__dict__[f[2]]
             if isinstance(f[1], str):
@@ -147,6 +147,7 @@ class Qcow2BitmapExt(Qcow2Struct):
 
     def dump(self):
         super().dump()
+
         for entry in self.bitmap_directory:
             print()
             entry.dump()
@@ -399,7 +400,7 @@ class QcowHeader(Qcow2Struct):
         buf = buf[0:header_bytes-1]
         fd.write(buf)
 
-    def dump_extensions(self):
+    def dump_extensions(self, is_json=False):
         for ex in self.extensions:
             print('Header extension:')
             ex.dump()
-- 
1.8.3.1

Re: [PATCH v11 08/11] qcow2.py: Introduce '-j' key to dump in JSON format

[Vladimir Sementsov-Ogievskiy]

17.07.2020 11:14, Andrey Shinkevich wrote:
> Add the command key to the qcow2.py arguments list to dump QCOW2
> metadata in JSON format. Here is the suggested way to do that. The
> implementation of the dump in JSON format is in the patch that follows.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2.py        | 18 ++++++++++++++----
>   tests/qemu-iotests/qcow2_format.py |  5 +++--
>   2 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 0910e6a..77ca59c 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -26,16 +26,19 @@ from qcow2_format import (
>   )
>   
>   
> +is_json = False
> +
> +
>   def cmd_dump_header(fd):
>       h = QcowHeader(fd)
> -    h.dump()
> +    h.dump(is_json)
>       print()
> -    h.dump_extensions()
> +    h.dump_extensions(is_json)
>   
>   
>   def cmd_dump_header_exts(fd):
>       h = QcowHeader(fd)
> -    h.dump_extensions()
> +    h.dump_extensions(is_json)
>   
>   
>   def cmd_set_header(fd, name, value):
> @@ -151,11 +154,14 @@ def main(filename, cmd, args):
>   
>   
>   def usage():
> -    print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0])
> +    print("Usage: %s <file> <cmd> [<arg>, ...] [<key>, ...]" % sys.argv[0])
>       print("")
>       print("Supported commands:")
>       for name, handler, num_args, desc in cmds:
>           print("    %-20s - %s" % (name, desc))
> +    print("")
> +    print("Supported keys:")
> +    print("    %-20s - %s" % ('-j', 'Dump in JSON format'))
>   
>   
>   if __name__ == '__main__':
> @@ -163,4 +169,8 @@ if __name__ == '__main__':
>           usage()
>           sys.exit(1)
>   
> +    is_json = '-j' in sys.argv
> +    if is_json:
> +        sys.argv.remove('-j')
> +
>       main(sys.argv[1], sys.argv[2], sys.argv[3:])
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index ad1918c..2921a27 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
>           self.__dict__ = dict((field[2], values[i])
>                                for i, field in enumerate(self.fields))
>   
> -    def dump(self):
> +    def dump(self, is_json=False):
>           for f in self.fields:
>               value = self.__dict__[f[2]]
>               if isinstance(f[1], str):
> @@ -147,6 +147,7 @@ class Qcow2BitmapExt(Qcow2Struct):
>   
>       def dump(self):
>           super().dump()
> +

Unrelated code-style. Please don't do so. With it dropped:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

>           for entry in self.bitmap_directory:
>               print()
>               entry.dump()
> @@ -399,7 +400,7 @@ class QcowHeader(Qcow2Struct):
>           buf = buf[0:header_bytes-1]
>           fd.write(buf)
>   
> -    def dump_extensions(self):
> +    def dump_extensions(self, is_json=False):
>           for ex in self.extensions:
>               print('Header extension:')
>               ex.dump()
> 


-- 
Best regards,
Vladimir

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

[Andrey Shinkevich]

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

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

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
>> +
...

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

[Vladimir Sementsov-Ogievskiy]

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
>>> +
> ...


-- 
Best regards,
Vladimir

[PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Andrey Shinkevich]

Per original script design, QcowHeader class may dump the QCOW2 header
info separately from the QCOW2 extensions info. To implement the
to_dict() method for dumping extensions, let us introduce the class
Qcow2HeaderExtensionsDoc.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 19d29b8..d2a8659 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -248,6 +248,15 @@ class Qcow2BitmapTable:
         return dict(entries=self.entries)
 
 
+class Qcow2HeaderExtensionsDoc:
+
+    def __init__(self, extensions):
+        self.extensions = extensions
+
+    def to_dict(self):
+        return dict(Header_extensions=self.extensions)
+
+
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
 
-- 
1.8.3.1

Re: [PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Vladimir Sementsov-Ogievskiy]

17.07.2020 11:14, Andrey Shinkevich wrote:
> Per original script design, QcowHeader class may dump the QCOW2 header
> info separately from the QCOW2 extensions info. To implement the
> to_dict() method for dumping extensions, let us introduce the class
> Qcow2HeaderExtensionsDoc.

I think, when dumping to qcow2, no needs to omit extensions, let's just always dump them.

> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2_format.py | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index 19d29b8..d2a8659 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -248,6 +248,15 @@ class Qcow2BitmapTable:
>           return dict(entries=self.entries)
>   
>   
> +class Qcow2HeaderExtensionsDoc:
> +
> +    def __init__(self, extensions):
> +        self.extensions = extensions
> +
> +    def to_dict(self):
> +        return dict(Header_extensions=self.extensions)

s/H/h/

> +
> +
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>   
>   
> 



-- 
Best regards,
Vladimir

Re: [PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Andrey Shinkevich]

On 28.07.2020 14:36, Vladimir Sementsov-Ogievskiy wrote:
> 17.07.2020 11:14, Andrey Shinkevich wrote:
>> Per original script design, QcowHeader class may dump the QCOW2 header
>> info separately from the QCOW2 extensions info. To implement the
>> to_dict() method for dumping extensions, let us introduce the class
>> Qcow2HeaderExtensionsDoc.
>
> I think, when dumping to qcow2, no needs to omit extensions, let's 
> just always dump them.
>

Do you like to eliminate the command 'dump-header-exts' and the relevant 
handler 'cmd_dump_header_exts' from the script qcow2.py ?

Andrey


>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/qcow2_format.py | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/qcow2_format.py 
>> b/tests/qemu-iotests/qcow2_format.py
>> index 19d29b8..d2a8659 100644
>> --- a/tests/qemu-iotests/qcow2_format.py
>> +++ b/tests/qemu-iotests/qcow2_format.py
>> @@ -248,6 +248,15 @@ class Qcow2BitmapTable:
>>           return dict(entries=self.entries)
>>     +class Qcow2HeaderExtensionsDoc:
>> +
>> +    def __init__(self, extensions):
>> +        self.extensions = extensions
>> +
>> +    def to_dict(self):
>> +        return dict(Header_extensions=self.extensions)
>
> s/H/h/
>
>> +
>> +
>>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>>
>
>
>

Re: [PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Vladimir Sementsov-Ogievskiy]

28.07.2020 16:27, Andrey Shinkevich wrote:
> On 28.07.2020 14:36, Vladimir Sementsov-Ogievskiy wrote:
>> 17.07.2020 11:14, Andrey Shinkevich wrote:
>>> Per original script design, QcowHeader class may dump the QCOW2 header
>>> info separately from the QCOW2 extensions info. To implement the
>>> to_dict() method for dumping extensions, let us introduce the class
>>> Qcow2HeaderExtensionsDoc.
>>
>> I think, when dumping to qcow2, no needs to omit extensions, let's just always dump them.
>>
> 
> Do you like to eliminate the command 'dump-header-exts' and the relevant handler 'cmd_dump_header_exts' from the script qcow2.py ?
> 

No, what already works should work of course. But we can not support -j for them. Still, not problem to support it. I just don't like this patch with an extra class. See my later comment, we can easily support json without this class.

> 
> 
>>>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/qcow2_format.py | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
>>> index 19d29b8..d2a8659 100644
>>> --- a/tests/qemu-iotests/qcow2_format.py
>>> +++ b/tests/qemu-iotests/qcow2_format.py
>>> @@ -248,6 +248,15 @@ class Qcow2BitmapTable:
>>>           return dict(entries=self.entries)
>>>     +class Qcow2HeaderExtensionsDoc:
>>> +
>>> +    def __init__(self, extensions):
>>> +        self.extensions = extensions
>>> +
>>> +    def to_dict(self):
>>> +        return dict(Header_extensions=self.extensions)
>>
>> s/H/h/
>>
>>> +
>>> +
>>>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>>>
>>
>>
>>


-- 
Best regards,
Vladimir

Re: [PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Andrey Shinkevich]

On 28.07.2020 14:36, Vladimir Sementsov-Ogievskiy wrote:
> 17.07.2020 11:14, Andrey Shinkevich wrote:
>> Per original script design, QcowHeader class may dump the QCOW2 header
>> info separately from the QCOW2 extensions info. To implement the
>> to_dict() method for dumping extensions, let us introduce the class
>> Qcow2HeaderExtensionsDoc.
>
> I think, when dumping to qcow2, no needs to omit extensions, let's 
> just always dump them.
>
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/qcow2_format.py | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/qcow2_format.py 
>> b/tests/qemu-iotests/qcow2_format.py
>> index 19d29b8..d2a8659 100644
>> --- a/tests/qemu-iotests/qcow2_format.py
>> +++ b/tests/qemu-iotests/qcow2_format.py
>> @@ -248,6 +248,15 @@ class Qcow2BitmapTable:
>>           return dict(entries=self.entries)
>>     +class Qcow2HeaderExtensionsDoc:
>> +
>> +    def __init__(self, extensions):
>> +        self.extensions = extensions
>> +
>> +    def to_dict(self):
>> +        return dict(Header_extensions=self.extensions)
>
> s/H/h/
>

It is the original code and the change would be unralated to my patch.

Should I make a separate patch for this change?

Andrey


>> +
>> +
>>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>>
>
>
>

Re: [PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Andrey Shinkevich]

On 28.07.2020 14:36, Vladimir Sementsov-Ogievskiy wrote:
> 17.07.2020 11:14, Andrey Shinkevich wrote:
>> Per original script design, QcowHeader class may dump the QCOW2 header
>> info separately from the QCOW2 extensions info. To implement the
>> to_dict() method for dumping extensions, let us introduce the class
>> Qcow2HeaderExtensionsDoc.
>
> I think, when dumping to qcow2, no needs to omit extensions, let's 
> just always dump them.
>
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/qcow2_format.py | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/qcow2_format.py 
>> b/tests/qemu-iotests/qcow2_format.py
>> index 19d29b8..d2a8659 100644
>> --- a/tests/qemu-iotests/qcow2_format.py
>> +++ b/tests/qemu-iotests/qcow2_format.py
>> @@ -248,6 +248,15 @@ class Qcow2BitmapTable:
>>           return dict(entries=self.entries)
>>     +class Qcow2HeaderExtensionsDoc:
>> +
>> +    def __init__(self, extensions):
>> +        self.extensions = extensions
>> +
>> +    def to_dict(self):
>> +        return dict(Header_extensions=self.extensions)
>
> s/H/h/
>

Sorry. I ment the non-JSON format dump as the original capitalized 'H'. 
The class Qcow2HeaderExtensionsDoc has been removed in the next v12.

Andrey


>> +
>> +
>>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>>
>
>
>

[PATCH v11 11/11] qcow2_format.py: support dumping metadata in JSON format

[Andrey Shinkevich]

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>
---
 tests/qemu-iotests/qcow2_format.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index d2a8659..d40eb49 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, 'to_dict'):
+            return obj.to_dict()
+        else:
+            return json.JSONEncoder.default(self, obj)
 
 
 class Qcow2Field:
@@ -110,6 +119,11 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
                              for i, field in enumerate(self.fields))
 
     def dump(self, is_json=False):
+        if is_json:
+            print(json.dumps(self.to_dict(), indent=4,
+                             cls=ComplexEncoder))
+            return
+
         for f in self.fields:
             value = self.__dict__[f[2]]
             if isinstance(f[1], str):
@@ -445,6 +459,12 @@ class QcowHeader(Qcow2Struct):
         fd.write(buf)
 
     def dump_extensions(self, is_json=False):
+        if is_json:
+            ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
+            print(json.dumps(ext_doc.to_dict(), indent=4,
+                             cls=ComplexEncoder))
+            return
+
         for ex in self.extensions:
             print('Header extension:')
             ex.dump()
-- 
1.8.3.1

Re: [PATCH v11 11/11] qcow2_format.py: support dumping metadata in JSON format

[Vladimir Sementsov-Ogievskiy]

17.07.2020 11:14, 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": [

bitmap_directy is just a list of dicts..

>                      {
>                          "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": {

.. can bitmap_table be just a list of dicts as well, not {entries: <list of dicts>} ?
>                              "entries": [
>                                  {
>                                      "type": "serialized",
>                                      "offset": 655360
>                                  },
>                                  ...
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2_format.py | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index d2a8659..d40eb49 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, 'to_dict'):
> +            return obj.to_dict()
> +        else:
> +            return json.JSONEncoder.default(self, obj)
>   
>   
>   class Qcow2Field:
> @@ -110,6 +119,11 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
>                                for i, field in enumerate(self.fields))
>   
>       def dump(self, is_json=False):
> +        if is_json:
> +            print(json.dumps(self.to_dict(), indent=4,
> +                             cls=ComplexEncoder))
> +            return
> +
>           for f in self.fields:
>               value = self.__dict__[f[2]]
>               if isinstance(f[1], str):
> @@ -445,6 +459,12 @@ class QcowHeader(Qcow2Struct):
>           fd.write(buf)
>   
>       def dump_extensions(self, is_json=False):
> +        if is_json:
> +            ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
> +            print(json.dumps(ext_doc.to_dict(), indent=4,
> +                             cls=ComplexEncoder))
> +            return

I'd prefer to get rid of Qcow2HeaderExtensionsDoc.

Could we just do something like json.dumps(self.extensions, indent=4, cls=ComplexEncoder) ?


> +
>           for ex in self.extensions:
>               print('Header extension:')
>               ex.dump()
> 


-- 
Best regards,
Vladimir

Re: [PATCH v11 00/11] iotests: Dump QCOW2 dirty bitmaps metadata

[Eric Blake]

On 7/17/20 3:14 AM, Andrey Shinkevich wrote:
> Add dirty bitmap information to QCOW2 metadata dump in the qcow2_format.py.
> 
> v10:
>    01: Fixing of issues in QCOW2 extension classes noted by Vladimir.
>    02: Reading bitmap tables was moved into Qcow2BitmapTable class.
>    03: Handling '-j' key was moved into "if __name__" section.
>    04: Making copy of __dict__ was replaced with the method to_dict().
>    05: Qcow2HeaderExtensionsDoc is introduced in the separate patch.
> 
> Andrey Shinkevich (11):
>    qcow2: Fix capitalization of header extension constant.
>    qcow2_format.py: make printable data an extension class member
>    qcow2_format.py: change Qcow2BitmapExt initialization method
>    qcow2_format.py: dump bitmap flags in human readable way.
>    qcow2_format.py: Dump bitmap directory information
>    qcow2_format.py: pass cluster size to substructures
>    qcow2_format.py: Dump bitmap table serialized entries
>    qcow2.py: Introduce '-j' key to dump in JSON format
>    qcow2_format.py: collect fields to dump in JSON format
>    qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class
>    qcow2_format.py: support dumping metadata in JSON format
> 
>   block/qcow2.c                      |   2 +-
>   docs/interop/qcow2.txt             |   2 +-
>   tests/qemu-iotests/qcow2.py        |  18 ++-
>   tests/qemu-iotests/qcow2_format.py | 221 ++++++++++++++++++++++++++++++++++---
>   4 files changed, 220 insertions(+), 23 deletions(-)

I still don't see any obvious coverage of the new output, which makes it 
harder to test (I have to manually run qcow2.py on a file rather than 
seeing what changes in a ???.out file).  I know we said back in v9 that 
test 291 is not the right test, but that does not stop you from adding a 
new test just for that purpose.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v11 00/11] iotests: Dump QCOW2 dirty bitmaps metadata

[Eric Blake]

On 7/23/20 2:42 PM, Eric Blake wrote:
> On 7/17/20 3:14 AM, Andrey Shinkevich wrote:
>> Add dirty bitmap information to QCOW2 metadata dump in the 
>> qcow2_format.py.
>>

>>   block/qcow2.c                      |   2 +-
>>   docs/interop/qcow2.txt             |   2 +-
>>   tests/qemu-iotests/qcow2.py        |  18 ++-
>>   tests/qemu-iotests/qcow2_format.py | 221 
>> ++++++++++++++++++++++++++++++++++---
>>   4 files changed, 220 insertions(+), 23 deletions(-)
> 
> I still don't see any obvious coverage of the new output, which makes it 
> harder to test (I have to manually run qcow2.py on a file rather than 
> seeing what changes in a ???.out file).  I know we said back in v9 that 
> test 291 is not the right test, but that does not stop you from adding a 
> new test just for that purpose.

The bulk of this series is touching a non-installed utility. At this 
point, I feel safer deferring it to 5.2 (it is a feature addition for 
testsuite use only, and we missed soft freeze), even though it has no 
negative impact to installed binaries.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v11 00/11] iotests: Dump QCOW2 dirty bitmaps metadata

[Vladimir Sementsov-Ogievskiy]

23.07.2020 23:18, Eric Blake wrote:
> On 7/23/20 2:42 PM, Eric Blake wrote:
>> On 7/17/20 3:14 AM, Andrey Shinkevich wrote:
>>> Add dirty bitmap information to QCOW2 metadata dump in the qcow2_format.py.
>>>
> 
>>>   block/qcow2.c                      |   2 +-
>>>   docs/interop/qcow2.txt             |   2 +-
>>>   tests/qemu-iotests/qcow2.py        |  18 ++-
>>>   tests/qemu-iotests/qcow2_format.py | 221 ++++++++++++++++++++++++++++++++++---
>>>   4 files changed, 220 insertions(+), 23 deletions(-)
>>
>> I still don't see any obvious coverage of the new output, which makes it harder to test (I have to manually run qcow2.py on a file rather than seeing what changes in a ???.out file).  I know we said back in v9 that test 291 is not the right test, but that does not stop you from adding a new test just for that purpose.
> 
> The bulk of this series is touching a non-installed utility. At this point, I feel safer deferring it to 5.2 (it is a feature addition for testsuite use only, and we missed soft freeze), even though it has no negative impact to installed binaries.
> 

Yes, it's absolutely OK to defer to 5.2.

Thanks a lot for taking a look at our series!

-- 
Best regards,
Vladimir