All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata
@ 2020-06-12  0:04 Andrey Shinkevich
  2020-06-12  0:04 ` [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291 Andrey Shinkevich
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

Note: based on the Vladimir's series
      [v5 00/13] iotests: Dump QCOW2 dirty bitmaps metadata

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

v7:
  01: Fix for magic hexadecimal output in 291
  02: Bitmap table output format improvement.
  03: Incremental change in the test 291 output.

v6:
  01: Fixing capitalization of header extension constant.
      (Suggested by Eric)
  02: The cluster size global variable discarded and passed as a parameter.
  03: Re-based to Vladimir's v5 series.
  04: The code of passing qcow2.py JSON format key moved to separate patch.
  05: Making dict(s) for dumping in JSON format was substituted with a copy
      of __dict__.

v5: The Vladimir's preliminary series
v4: The Vladimir's preliminary series

Andrey Shinkevich (9):
  iotests: Fix for magic hexadecimal output in 291
  qcow2: Fix capitalization of header extension constant.
  qcow2_format.py: make printable data an extension class member
  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: support dumping metadata in JSON format

 block/qcow2.c                      |   2 +-
 docs/interop/qcow2.txt             |   2 +-
 tests/qemu-iotests/291.out         | 112 ++++++++++++++++++-
 tests/qemu-iotests/qcow2.py        |  20 +++-
 tests/qemu-iotests/qcow2_format.py | 217 ++++++++++++++++++++++++++++++++++---
 5 files changed, 327 insertions(+), 26 deletions(-)

-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
@ 2020-06-12  0:04 ` Andrey Shinkevich
  2020-06-15  9:43   ` Vladimir Sementsov-Ogievskiy
  2020-06-12  0:04 ` [PATCH v7 2/9] qcow2: Fix capitalization of header extension constant Andrey Shinkevich
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

This issue was introduced in the earlier patch:
"qcow2_format: refactor QcowHeaderExtension as a subclass of
Qcow2Struct".

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/291.out | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
index 1d4f9cd..ccfcdc5 100644
--- a/tests/qemu-iotests/291.out
+++ b/tests/qemu-iotests/291.out
@@ -16,17 +16,17 @@ wrote 1048576/1048576 bytes at offset 2097152
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 Check resulting qcow2 header extensions:
 Header extension:
-magic                     3799591626 (Backing format)
+magic                     0xe2792aca (Backing format)
 length                    5
 data                      'qcow2'
 
 Header extension:
-magic                     1745090647 (Feature table)
+magic                     0x6803f857 (Feature table)
 length                    336
 data                      <binary>
 
 Header extension:
-magic                     595929205 (Bitmaps)
+magic                     0x23852875 (Bitmaps)
 length                    24
 nb_bitmaps                2
 reserved32                0
@@ -86,12 +86,12 @@ Format specific information:
     corrupt: false
 Check resulting qcow2 header extensions:
 Header extension:
-magic                     1745090647 (Feature table)
+magic                     0x6803f857 (Feature table)
 length                    336
 data                      <binary>
 
 Header extension:
-magic                     595929205 (Bitmaps)
+magic                     0x23852875 (Bitmaps)
 length                    24
 nb_bitmaps                3
 reserved32                0
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 2/9] qcow2: Fix capitalization of header extension constant.
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
  2020-06-12  0:04 ` [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291 Andrey Shinkevich
@ 2020-06-12  0:04 ` Andrey Shinkevich
  2020-06-15  9:44   ` Vladimir Sementsov-Ogievskiy
  2020-06-12  0:04 ` [PATCH v7 3/9] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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>
---
 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 0cd2e67..80dfe5f 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 3/9] qcow2_format.py: make printable data an extension class member
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
  2020-06-12  0:04 ` [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291 Andrey Shinkevich
  2020-06-12  0:04 ` [PATCH v7 2/9] qcow2: Fix capitalization of header extension constant Andrey Shinkevich
@ 2020-06-12  0:04 ` Andrey Shinkevich
  2020-06-15  9:47   ` Vladimir Sementsov-Ogievskiy
  2020-06-12  0:04 ` [PATCH v7 4/9] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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 0f65fd1..d4f0000 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -164,6 +164,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:
@@ -173,12 +180,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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 4/9] qcow2_format.py: Dump bitmap directory information
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (2 preceding siblings ...)
  2020-06-12  0:04 ` [PATCH v7 3/9] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
@ 2020-06-12  0:04 ` Andrey Shinkevich
  2020-06-15 10:34   ` Vladimir Sementsov-Ogievskiy
  2020-06-12  0:04 ` [PATCH v7 5/9] qcow2_format.py: pass cluster size to substructures Andrey Shinkevich
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

Read and dump entries from the bitmap directory of QCOW2 image.
It extends the output in the test case #291.

Header extension:
magic                     0x23852875 (Bitmaps)
...
Bitmap name               bitmap-1
flag                      auto
table size                8 (bytes)
bitmap_table_offset       0x90000
bitmap_table_size         1
flags                     0
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: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/291.out         | 52 ++++++++++++++++++++++++++
 tests/qemu-iotests/qcow2_format.py | 75 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)

diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
index ccfcdc5..d847419 100644
--- a/tests/qemu-iotests/291.out
+++ b/tests/qemu-iotests/291.out
@@ -33,6 +33,27 @@ reserved32                0
 bitmap_directory_size     0x40
 bitmap_directory_offset   0x510000
 
+Bitmap name               b1
+table size                8 (bytes)
+bitmap_table_offset       0x4e0000
+bitmap_table_size         1
+flags                     0
+type                      1
+granularity_bits          19
+name_size                 2
+extra_data_size           0
+
+Bitmap name               b2
+flag                      auto
+table size                8 (bytes)
+bitmap_table_offset       0x500000
+bitmap_table_size         1
+flags                     2
+type                      1
+granularity_bits          16
+name_size                 2
+extra_data_size           0
+
 
 === Bitmap preservation not possible to non-qcow2 ===
 
@@ -98,6 +119,37 @@ reserved32                0
 bitmap_directory_size     0x60
 bitmap_directory_offset   0x520000
 
+Bitmap name               b1
+table size                8 (bytes)
+bitmap_table_offset       0x470000
+bitmap_table_size         1
+flags                     0
+type                      1
+granularity_bits          19
+name_size                 2
+extra_data_size           0
+
+Bitmap name               b2
+flag                      auto
+table size                8 (bytes)
+bitmap_table_offset       0x490000
+bitmap_table_size         1
+flags                     2
+type                      1
+granularity_bits          16
+name_size                 2
+extra_data_size           0
+
+Bitmap name               b0
+table size                8 (bytes)
+bitmap_table_offset       0x510000
+bitmap_table_size         1
+flags                     0
+type                      1
+granularity_bits          16
+name_size                 2
+extra_data_size           0
+
 
 === Check bitmap contents ===
 
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index d4f0000..90eff92 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -103,6 +103,10 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
             print('{:<25} {}'.format(f[2], value_str))
 
 
+# seek relative to the current position in the file
+FROM_CURRENT = 1
+
+
 class Qcow2BitmapExt(Qcow2Struct):
 
     fields = (
@@ -112,6 +116,73 @@ class Qcow2BitmapExt(Qcow2Struct):
         ('u64', '{:#x}', 'bitmap_directory_offset')
     )
 
+    def read_bitmap_directory(self, fd):
+        self.bitmaps = []
+        fd.seek(self.bitmap_directory_offset)
+        buf_size = struct.calcsize(Qcow2BitmapDirEntry.fmt)
+
+        for n in range(self.nb_bitmaps):
+            buf = fd.read(buf_size)
+            dir_entry = Qcow2BitmapDirEntry(data=buf)
+            fd.seek(dir_entry.extra_data_size, FROM_CURRENT)
+            bitmap_name = fd.read(dir_entry.name_size)
+            dir_entry.name = bitmap_name.decode('ascii')
+            self.bitmaps.append(dir_entry)
+            entry_raw_size = dir_entry.bitmap_dir_entry_raw_size()
+            shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
+            fd.seek(shift, FROM_CURRENT)
+
+    def load(self, fd):
+        self.read_bitmap_directory(fd)
+
+    def dump(self):
+        super().dump()
+        for bm in self.bitmaps:
+            bm.dump_bitmap_dir_entry()
+
+
+BME_FLAG_IN_USE = 1 << 0
+BME_FLAG_AUTO = 1 << 1
+
+
+class Qcow2BitmapDirEntry(Qcow2Struct):
+
+    name = ''
+
+    fields = (
+        ('u64', '{:#x}', 'bitmap_table_offset'),
+        ('u32', '{}',    'bitmap_table_size'),
+        ('u32', '{}',    'flags'),
+        ('u8',  '{}',    'type'),
+        ('u8',  '{}',    'granularity_bits'),
+        ('u16', '{}',    'name_size'),
+        ('u32', '{}',    'extra_data_size')
+    )
+
+    def __init__(self, data):
+        super().__init__(data=data)
+
+        self.bitmap_table_bytes = self.bitmap_table_size \
+            * struct.calcsize('Q')
+
+        self.bitmap_flags = []
+        if (self.flags & BME_FLAG_IN_USE):
+            self.bitmap_flags.append("in-use")
+        if (self.flags & BME_FLAG_AUTO):
+            self.bitmap_flags.append("auto")
+
+    def bitmap_dir_entry_raw_size(self):
+        return struct.calcsize(self.fmt) + self.name_size + \
+            self.extra_data_size
+
+    def dump_bitmap_dir_entry(self):
+        print()
+        print(f'{"Bitmap name":<25} {self.name}')
+        for fl in self.bitmap_flags:
+            print(f'{"flag":<25} {fl}')
+        print(f'{"table size":<25} {self.bitmap_table_bytes} {"(bytes)"}')
+        super().dump()
+
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
@@ -253,6 +324,10 @@ class QcowHeader(Qcow2Struct):
             else:
                 self.extensions.append(ext)
 
+        for ext in self.extensions:
+            if ext.obj is not None:
+                ext.obj.load(fd)
+
     def update_extensions(self, fd):
 
         fd.seek(self.header_length)
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 5/9] qcow2_format.py: pass cluster size to substructures
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (3 preceding siblings ...)
  2020-06-12  0:04 ` [PATCH v7 4/9] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
@ 2020-06-12  0:04 ` Andrey Shinkevich
  2020-06-12  0:04 ` [PATCH v7 6/9] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 90eff92..eb99119 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -116,6 +116,10 @@ class Qcow2BitmapExt(Qcow2Struct):
         ('u64', '{:#x}', 'bitmap_directory_offset')
     )
 
+    def __init__(self, data, cluster_size):
+        super().__init__(data=data)
+        self.cluster_size = cluster_size
+
     def read_bitmap_directory(self, fd):
         self.bitmaps = []
         fd.seek(self.bitmap_directory_offset)
@@ -123,7 +127,8 @@ class Qcow2BitmapExt(Qcow2Struct):
 
         for n in range(self.nb_bitmaps):
             buf = fd.read(buf_size)
-            dir_entry = Qcow2BitmapDirEntry(data=buf)
+            dir_entry = Qcow2BitmapDirEntry(data=buf,
+                                            cluster_size=self.cluster_size)
             fd.seek(dir_entry.extra_data_size, FROM_CURRENT)
             bitmap_name = fd.read(dir_entry.name_size)
             dir_entry.name = bitmap_name.decode('ascii')
@@ -159,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         ('u32', '{}',    'extra_data_size')
     )
 
-    def __init__(self, data):
+    def __init__(self, data, cluster_size):
         super().__init__(data=data)
+        self.cluster_size = cluster_size
 
         self.bitmap_table_bytes = self.bitmap_table_size \
             * struct.calcsize('Q')
@@ -205,11 +211,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
@@ -243,7 +251,8 @@ class QcowHeaderExtension(Qcow2Struct):
         self.data_str = data_str
 
         if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-            self.obj = Qcow2BitmapExt(data=self.data)
+            self.obj = Qcow2BitmapExt(data=self.data,
+                                      cluster_size=cluster_size)
         else:
             self.obj = None
 
@@ -318,7 +327,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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 6/9] qcow2_format.py: Dump bitmap table serialized entries
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (4 preceding siblings ...)
  2020-06-12  0:04 ` [PATCH v7 5/9] qcow2_format.py: pass cluster size to substructures Andrey Shinkevich
@ 2020-06-12  0:04 ` Andrey Shinkevich
  2020-06-12  0:05 ` [PATCH v7 7/9] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:04 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

Add bitmap table information to the QCOW2 metadata dump.
It extends the output of the test case #291

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>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/291.out         | 50 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/qcow2_format.py | 40 ++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
index d847419..595327c 100644
--- a/tests/qemu-iotests/291.out
+++ b/tests/qemu-iotests/291.out
@@ -42,6 +42,16 @@ type                      1
 granularity_bits          19
 name_size                 2
 extra_data_size           0
+Bitmap table   type            offset                   size
+0              serialized      5046272                  65536
+1              all-zeroes      0                        65536
+2              all-zeroes      0                        65536
+3              all-zeroes      0                        65536
+4              all-zeroes      0                        65536
+5              all-zeroes      0                        65536
+6              all-zeroes      0                        65536
+7              all-zeroes      0                        65536
+
 
 Bitmap name               b2
 flag                      auto
@@ -53,6 +63,16 @@ type                      1
 granularity_bits          16
 name_size                 2
 extra_data_size           0
+Bitmap table   type            offset                   size
+0              serialized      5177344                  65536
+1              all-zeroes      0                        65536
+2              all-zeroes      0                        65536
+3              all-zeroes      0                        65536
+4              all-zeroes      0                        65536
+5              all-zeroes      0                        65536
+6              all-zeroes      0                        65536
+7              all-zeroes      0                        65536
+
 
 
 === Bitmap preservation not possible to non-qcow2 ===
@@ -128,6 +148,16 @@ type                      1
 granularity_bits          19
 name_size                 2
 extra_data_size           0
+Bitmap table   type            offset                   size
+0              serialized      4587520                  65536
+1              all-zeroes      0                        65536
+2              all-zeroes      0                        65536
+3              all-zeroes      0                        65536
+4              all-zeroes      0                        65536
+5              all-zeroes      0                        65536
+6              all-zeroes      0                        65536
+7              all-zeroes      0                        65536
+
 
 Bitmap name               b2
 flag                      auto
@@ -139,6 +169,16 @@ type                      1
 granularity_bits          16
 name_size                 2
 extra_data_size           0
+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
+
 
 Bitmap name               b0
 table size                8 (bytes)
@@ -149,6 +189,16 @@ type                      1
 granularity_bits          16
 name_size                 2
 extra_data_size           0
+Bitmap table   type            offset                   size
+0              serialized      5242880                  65536
+1              all-zeroes      0                        65536
+2              all-zeroes      0                        65536
+3              all-zeroes      0                        65536
+4              all-zeroes      0                        65536
+5              all-zeroes      0                        65536
+6              all-zeroes      0                        65536
+7              all-zeroes      0                        65536
+
 
 
 === Check bitmap contents ===
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index eb99119..c1c2773 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -137,6 +137,9 @@ class Qcow2BitmapExt(Qcow2Struct):
             shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
             fd.seek(shift, FROM_CURRENT)
 
+        for bm in self.bitmaps:
+            bm.read_bitmap_table(fd)
+
     def load(self, fd):
         self.read_bitmap_directory(fd)
 
@@ -181,6 +184,12 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         return struct.calcsize(self.fmt) + self.name_size + \
             self.extra_data_size
 
+    def read_bitmap_table(self, fd):
+        fd.seek(self.bitmap_table_offset)
+        table_size = self.bitmap_table_bytes * struct.calcsize('Q')
+        table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
+        self.bitmap_table = Qcow2BitmapTable(table)
+
     def dump_bitmap_dir_entry(self):
         print()
         print(f'{"Bitmap name":<25} {self.name}')
@@ -188,6 +197,37 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
             print(f'{"flag":<25} {fl}')
         print(f'{"table size":<25} {self.bitmap_table_bytes} {"(bytes)"}')
         super().dump()
+        self.bitmap_table.print_bitmap_table(self.cluster_size)
+
+
+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, raw_table):
+        self.entries = []
+        for entry in raw_table:
+            self.entries.append(Qcow2BitmapTableEntry(entry))
+
+    def print_bitmap_table(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} {cluster_size}')
+        print("")
 
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 7/9] qcow2.py: Introduce '-j' key to dump in JSON format
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (5 preceding siblings ...)
  2020-06-12  0:04 ` [PATCH v7 6/9] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
@ 2020-06-12  0:05 ` Andrey Shinkevich
  2020-06-12  0:05 ` [PATCH v7 8/9] qcow2_format.py: collect fields " Andrey Shinkevich
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:05 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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        | 20 +++++++++++++++-----
 tests/qemu-iotests/qcow2_format.py | 18 +++++++++---------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 8c187e9..b08d8fc 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -24,16 +24,19 @@ from qcow2_format import (
 )
 
 
+dump_json = False
+
+
 def cmd_dump_header(fd):
     h = QcowHeader(fd)
-    h.dump()
+    h.dump(dump_json)
     print()
-    h.dump_extensions()
+    h.dump_extensions(dump_json)
 
 
 def cmd_dump_header_exts(fd):
     h = QcowHeader(fd)
-    h.dump_extensions()
+    h.dump_extensions(dump_json)
 
 
 def cmd_set_header(fd, name, value):
@@ -132,6 +135,11 @@ cmds = [
 
 
 def main(filename, cmd, args):
+    global dump_json
+    dump_json = '-j' in sys.argv
+    if dump_json:
+        sys.argv.remove('-j')
+        args.remove('-j')
     fd = open(filename, "r+b")
     try:
         for name, handler, num_args, desc in cmds:
@@ -149,12 +157,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__':
     if len(sys.argv) < 3:
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index c1c2773..a19f3b3 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -92,7 +92,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, dump_json=None):
         for f in self.fields:
             value = self.__dict__[f[2]]
             if isinstance(f[1], str):
@@ -143,10 +143,10 @@ class Qcow2BitmapExt(Qcow2Struct):
     def load(self, fd):
         self.read_bitmap_directory(fd)
 
-    def dump(self):
-        super().dump()
+    def dump(self, dump_json=None):
+        super().dump(dump_json)
         for bm in self.bitmaps:
-            bm.dump_bitmap_dir_entry()
+            bm.dump_bitmap_dir_entry(dump_json)
 
 
 BME_FLAG_IN_USE = 1 << 0
@@ -190,7 +190,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
         self.bitmap_table = Qcow2BitmapTable(table)
 
-    def dump_bitmap_dir_entry(self):
+    def dump_bitmap_dir_entry(self, dump_json=None):
         print()
         print(f'{"Bitmap name":<25} {self.name}')
         for fl in self.bitmap_flags:
@@ -296,13 +296,13 @@ class QcowHeaderExtension(Qcow2Struct):
         else:
             self.obj = None
 
-    def dump(self):
+    def dump(self, dump_json=None):
         super().dump()
 
         if self.obj is None:
             print(f'{"data":<25} {self.data_str}')
         else:
-            self.obj.dump()
+            self.obj.dump(dump_json)
 
     @classmethod
     def create(cls, magic, data):
@@ -405,8 +405,8 @@ class QcowHeader(Qcow2Struct):
         buf = buf[0:header_bytes-1]
         fd.write(buf)
 
-    def dump_extensions(self):
+    def dump_extensions(self, dump_json=None):
         for ex in self.extensions:
             print('Header extension:')
-            ex.dump()
+            ex.dump(dump_json)
             print()
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 8/9] qcow2_format.py: collect fields to dump in JSON format
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (6 preceding siblings ...)
  2020-06-12  0:05 ` [PATCH v7 7/9] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
@ 2020-06-12  0:05 ` Andrey Shinkevich
  2020-06-12  0:05 ` [PATCH v7 9/9] qcow2_format.py: support dumping metadata " Andrey Shinkevich
  2020-06-15  9:42 ` [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
  9 siblings, 0 replies; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:05 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

As __dict__ is being extended with class members we do not want to
print in JSON format dump, make a light copy of the initial __dict__
and extend the copy by adding lists we have to print in the JSON
output.

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

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index a19f3b3..bdd5f68 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -92,6 +92,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
         self.__dict__ = dict((field[2], values[i])
                              for i, field in enumerate(self.fields))
 
+        self.fields_dict = self.__dict__.copy()
+
     def dump(self, dump_json=None):
         for f in self.fields:
             value = self.__dict__[f[2]]
@@ -102,7 +104,6 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
 
             print('{:<25} {}'.format(f[2], value_str))
 
-
 # seek relative to the current position in the file
 FROM_CURRENT = 1
 
@@ -142,6 +143,7 @@ class Qcow2BitmapExt(Qcow2Struct):
 
     def load(self, fd):
         self.read_bitmap_directory(fd)
+        self.fields_dict.update(entries=self.bitmaps)
 
     def dump(self, dump_json=None):
         super().dump(dump_json)
@@ -189,6 +191,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         table_size = self.bitmap_table_bytes * struct.calcsize('Q')
         table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
         self.bitmap_table = Qcow2BitmapTable(table)
+        self.fields_dict.update(bitmap_table=self.bitmap_table)
 
     def dump_bitmap_dir_entry(self, dump_json=None):
         print()
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v7 9/9] qcow2_format.py: support dumping metadata in JSON format
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (7 preceding siblings ...)
  2020-06-12  0:05 ` [PATCH v7 8/9] qcow2_format.py: collect fields " Andrey Shinkevich
@ 2020-06-12  0:05 ` Andrey Shinkevich
  2020-06-15  9:42 ` [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
  9 siblings, 0 replies; 15+ messages in thread
From: Andrey Shinkevich @ 2020-06-12  0:05 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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,
                "entries": [
                    {
                        "name": "bitmap-1",
                        "bitmap_table_offset": 589824,
                        "bitmap_table_size": 1,
                        "flags": [
                            "auto"
                        ],
                        "type": 1,
                        "granularity_bits": 16,
                        "name_size": 8,
                        "extra_data_size": 0,
                        "bitmap_table": {
                            "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 | 60 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index bdd5f68..35daec9 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -18,6 +18,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:
@@ -95,6 +104,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
+
         for f in self.fields:
             value = self.__dict__[f[2]]
             if isinstance(f[1], str):
@@ -150,6 +164,9 @@ class Qcow2BitmapExt(Qcow2Struct):
         for bm in self.bitmaps:
             bm.dump_bitmap_dir_entry(dump_json)
 
+    def get_fields_dict(self):
+        return self.fields_dict
+
 
 BME_FLAG_IN_USE = 1 << 0
 BME_FLAG_AUTO = 1 << 1
@@ -202,6 +219,12 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         super().dump()
         self.bitmap_table.print_bitmap_table(self.cluster_size)
 
+    def get_fields_dict(self):
+        bmp_name = dict(name=self.name)
+        bme_dict = {**bmp_name, **self.fields_dict}
+        bme_dict['flags'] = self.bitmap_flags
+        return bme_dict
+
 
 class Qcow2BitmapTableEntry:
 
@@ -217,6 +240,9 @@ class Qcow2BitmapTableEntry:
         else:
             self.type = 'all-zeroes'
 
+    def get_fields_dict(self):
+        return dict(type=self.type, offset=self.offset)
+
 
 class Qcow2BitmapTable:
 
@@ -232,6 +258,18 @@ class Qcow2BitmapTable:
             print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {cluster_size}')
         print("")
 
+    def get_fields_dict(self):
+        return dict(table_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
 
@@ -247,6 +285,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')
@@ -307,6 +348,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)
@@ -409,7 +460,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
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata
  2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (8 preceding siblings ...)
  2020-06-12  0:05 ` [PATCH v7 9/9] qcow2_format.py: support dumping metadata " Andrey Shinkevich
@ 2020-06-15  9:42 ` Vladimir Sementsov-Ogievskiy
  9 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-15  9:42 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

12.06.2020 03:04, Andrey Shinkevich wrote:
> Note: based on the Vladimir's series
>        [v5 00/13] iotests: Dump QCOW2 dirty bitmaps metadata

It's merged to master, so, based on master. (except for 01, which is not needed, thanks to Eric)

> 
> Add dirty bitmap information to QCOW2 metadata dump in the qcow2_format.py.
> 
> v7:
>    01: Fix for magic hexadecimal output in 291
>    02: Bitmap table output format improvement.
>    03: Incremental change in the test 291 output.
> 
> v6:
>    01: Fixing capitalization of header extension constant.
>        (Suggested by Eric)
>    02: The cluster size global variable discarded and passed as a parameter.
>    03: Re-based to Vladimir's v5 series.
>    04: The code of passing qcow2.py JSON format key moved to separate patch.
>    05: Making dict(s) for dumping in JSON format was substituted with a copy
>        of __dict__.
> 
> v5: The Vladimir's preliminary series
> v4: The Vladimir's preliminary series
> 
> Andrey Shinkevich (9):
>    iotests: Fix for magic hexadecimal output in 291
>    qcow2: Fix capitalization of header extension constant.
>    qcow2_format.py: make printable data an extension class member
>    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: support dumping metadata in JSON format
> 
>   block/qcow2.c                      |   2 +-
>   docs/interop/qcow2.txt             |   2 +-
>   tests/qemu-iotests/291.out         | 112 ++++++++++++++++++-
>   tests/qemu-iotests/qcow2.py        |  20 +++-
>   tests/qemu-iotests/qcow2_format.py | 217 ++++++++++++++++++++++++++++++++++---
>   5 files changed, 327 insertions(+), 26 deletions(-)
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291
  2020-06-12  0:04 ` [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291 Andrey Shinkevich
@ 2020-06-15  9:43   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-15  9:43 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

12.06.2020 03:04, Andrey Shinkevich wrote:
> This issue was introduced in the earlier patch:
> "qcow2_format: refactor QcowHeaderExtension as a subclass of
> Qcow2Struct".
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

This change was squashed to original commit

> ---
>   tests/qemu-iotests/291.out | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
> index 1d4f9cd..ccfcdc5 100644
> --- a/tests/qemu-iotests/291.out
> +++ b/tests/qemu-iotests/291.out
> @@ -16,17 +16,17 @@ wrote 1048576/1048576 bytes at offset 2097152
>   1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>   Check resulting qcow2 header extensions:
>   Header extension:
> -magic                     3799591626 (Backing format)
> +magic                     0xe2792aca (Backing format)
>   length                    5
>   data                      'qcow2'
>   
>   Header extension:
> -magic                     1745090647 (Feature table)
> +magic                     0x6803f857 (Feature table)
>   length                    336
>   data                      <binary>
>   
>   Header extension:
> -magic                     595929205 (Bitmaps)
> +magic                     0x23852875 (Bitmaps)
>   length                    24
>   nb_bitmaps                2
>   reserved32                0
> @@ -86,12 +86,12 @@ Format specific information:
>       corrupt: false
>   Check resulting qcow2 header extensions:
>   Header extension:
> -magic                     1745090647 (Feature table)
> +magic                     0x6803f857 (Feature table)
>   length                    336
>   data                      <binary>
>   
>   Header extension:
> -magic                     595929205 (Bitmaps)
> +magic                     0x23852875 (Bitmaps)
>   length                    24
>   nb_bitmaps                3
>   reserved32                0
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v7 2/9] qcow2: Fix capitalization of header extension constant.
  2020-06-12  0:04 ` [PATCH v7 2/9] qcow2: Fix capitalization of header extension constant Andrey Shinkevich
@ 2020-06-15  9:44   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-15  9:44 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

12.06.2020 03:04, 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>

-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v7 3/9] qcow2_format.py: make printable data an extension class member
  2020-06-12  0:04 ` [PATCH v7 3/9] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
@ 2020-06-15  9:47   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-15  9:47 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

12.06.2020 03:04, Andrey Shinkevich wrote:
> Let us differ binary data type from string one for the extension data
> variable and keep the string as the QcowHeaderExtension class member.

Keep my r-b, but I just want to note, that I missed, what is the actual aim of this change..

> 
> 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 0f65fd1..d4f0000 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -164,6 +164,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:
> @@ -173,12 +180,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()
>   
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v7 4/9] qcow2_format.py: Dump bitmap directory information
  2020-06-12  0:04 ` [PATCH v7 4/9] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
@ 2020-06-15 10:34   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-15 10:34 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

12.06.2020 03:04, Andrey Shinkevich wrote:
> Read and dump entries from the bitmap directory of QCOW2 image.
> It extends the output in the test case #291.
> 
> Header extension:
> magic                     0x23852875 (Bitmaps)
> ...
> Bitmap name               bitmap-1
> flag                      auto
> table size                8 (bytes)
> bitmap_table_offset       0x90000
> bitmap_table_size         1
> flags                     0
> 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: Eric Blake <eblake@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Hmm I don't remember me gave it..

> ---
>   tests/qemu-iotests/291.out         | 52 ++++++++++++++++++++++++++
>   tests/qemu-iotests/qcow2_format.py | 75 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 127 insertions(+)
> 
> diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
> index ccfcdc5..d847419 100644
> --- a/tests/qemu-iotests/291.out
> +++ b/tests/qemu-iotests/291.out
> @@ -33,6 +33,27 @@ reserved32                0
>   bitmap_directory_size     0x40
>   bitmap_directory_offset   0x510000
>   
> +Bitmap name               b1
> +table size                8 (bytes)
> +bitmap_table_offset       0x4e0000
> +bitmap_table_size         1
> +flags                     0
> +type                      1
> +granularity_bits          19
> +name_size                 2
> +extra_data_size           0
> +
> +Bitmap name               b2
> +flag                      auto
> +table size                8 (bytes)
> +bitmap_table_offset       0x500000
> +bitmap_table_size         1
> +flags                     2

both having flags and flag in the output looks strange.

If you want good human look of flags field, you should implement a special formatter-class for it, like Flags64.
Maybe, something like this:

flags      0x3 (in_use auto)


> +type                      1
> +granularity_bits          16
> +name_size                 2
> +extra_data_size           0
> +
>   
>   === Bitmap preservation not possible to non-qcow2 ===
>   
> @@ -98,6 +119,37 @@ reserved32                0
>   bitmap_directory_size     0x60
>   bitmap_directory_offset   0x520000
>   
> +Bitmap name               b1
> +table size                8 (bytes)
> +bitmap_table_offset       0x470000
> +bitmap_table_size         1
> +flags                     0
> +type                      1
> +granularity_bits          19
> +name_size                 2
> +extra_data_size           0
> +
> +Bitmap name               b2
> +flag                      auto
> +table size                8 (bytes)
> +bitmap_table_offset       0x490000
> +bitmap_table_size         1
> +flags                     2
> +type                      1
> +granularity_bits          16
> +name_size                 2
> +extra_data_size           0
> +
> +Bitmap name               b0
> +table size                8 (bytes)
> +bitmap_table_offset       0x510000
> +bitmap_table_size         1
> +flags                     0
> +type                      1
> +granularity_bits          16
> +name_size                 2
> +extra_data_size           0
> +
>   
>   === Check bitmap contents ===
>   
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index d4f0000..90eff92 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -103,6 +103,10 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
>               print('{:<25} {}'.format(f[2], value_str))
>   
>   
> +# seek relative to the current position in the file
> +FROM_CURRENT = 1

Firstly, I though that it's a global variable to control class behavior. Only than I understood that you just decided to use a named constant instead of just 1...

So, I'd prefer to use just 1.

> +
> +
>   class Qcow2BitmapExt(Qcow2Struct):
>   
>       fields = (
> @@ -112,6 +116,73 @@ class Qcow2BitmapExt(Qcow2Struct):
>           ('u64', '{:#x}', 'bitmap_directory_offset')
>       )
>   
> +    def read_bitmap_directory(self, fd):
> +        self.bitmaps = []
> +        fd.seek(self.bitmap_directory_offset)
> +        buf_size = struct.calcsize(Qcow2BitmapDirEntry.fmt)
> +
> +        for n in range(self.nb_bitmaps):
> +            buf = fd.read(buf_size)
> +            dir_entry = Qcow2BitmapDirEntry(data=buf)

Base class of Qcow2BitmapDirEntry can load from fd. We should definitely utilize this possibility, instead of writing it again.

> +            fd.seek(dir_entry.extra_data_size, FROM_CURRENT)
> +            bitmap_name = fd.read(dir_entry.name_size)
> +            dir_entry.name = bitmap_name.decode('ascii')

You should initialize object in its constructor, not by hand here.

Actually, the code here should look like this:

self.bitmap_directory = [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]

OK, you may leave a loop, and even calculating of final alignment here, but real fields should be read and initialized in Qcow2BitmapDirEntry constructor
    

> +            self.bitmaps.append(dir_entry)
> +            entry_raw_size = dir_entry.bitmap_dir_entry_raw_size()
> +            shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
> +            fd.seek(shift, FROM_CURRENT)
> +
> +    def load(self, fd):
> +        self.read_bitmap_directory(fd)

Let's load in __init__, why to force user call additional .load after object creation?

> +
> +    def dump(self):
> +        super().dump()
> +        for bm in self.bitmaps:
> +            bm.dump_bitmap_dir_entry()
> +
> +
> +BME_FLAG_IN_USE = 1 << 0
> +BME_FLAG_AUTO = 1 << 1
> +
> +
> +class Qcow2BitmapDirEntry(Qcow2Struct):
> +
> +    name = ''
> +
> +    fields = (
> +        ('u64', '{:#x}', 'bitmap_table_offset'),
> +        ('u32', '{}',    'bitmap_table_size'),

please, don't do internal indenting, I've dropped it from all other classes here.

> +        ('u32', '{}',    'flags'),
> +        ('u8',  '{}',    'type'),
> +        ('u8',  '{}',    'granularity_bits'),
> +        ('u16', '{}',    'name_size'),
> +        ('u32', '{}',    'extra_data_size')
> +    )
> +
> +    def __init__(self, data):
> +        super().__init__(data=data)
> +
> +        self.bitmap_table_bytes = self.bitmap_table_size \
> +            * struct.calcsize('Q')

struct.calcsize('Q') isn't more friendly than just 64.

Also, you tend to prepare everything you want to dump in a constructor. Why?
I'd prefer not to have attributes bitmap_table_bytes and bitmap_table_size
stored together, it's obviously too redundant. You can always do
print(self.bitmap_table_size * 64), no reason to store the calculated value.

> +
> +        self.bitmap_flags = []
> +        if (self.flags & BME_FLAG_IN_USE):
> +            self.bitmap_flags.append("in-use")
> +        if (self.flags & BME_FLAG_AUTO):
> +            self.bitmap_flags.append("auto")

Again, for this we need a separate formatter class, like Flags64

> +
> +    def bitmap_dir_entry_raw_size(self):
> +        return struct.calcsize(self.fmt) + self.name_size + \
> +            self.extra_data_size
> +
> +    def dump_bitmap_dir_entry(self):

this method should be called just dump

> +        print()

this is extra formatting print-line. Do it on upper level, it's unrelated to Qcow2BitmapDirEntry class itself

> +        print(f'{"Bitmap name":<25} {self.name}')
> +        for fl in self.bitmap_flags:
> +            print(f'{"flag":<25} {fl}')
> +        print(f'{"table size":<25} {self.bitmap_table_bytes} {"(bytes)"}')

Why do we need this additional representation of bitmap table size?

> +        super().dump()
> +
>   
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>   
> @@ -253,6 +324,10 @@ class QcowHeader(Qcow2Struct):
>               else:
>                   self.extensions.append(ext)
>   
> +        for ext in self.extensions:
> +            if ext.obj is not None:
> +                ext.obj.load(fd)
> +

I don't like this chunk, it breaks encapsulation. Why QcowHeader object should know that some of extensions may have internal obj, and to initialize it we need to call its .load method explicitly?
If extension wants to load some additional data it should do it in a constructor.

>       def update_extensions(self, fd):
>   
>           fd.seek(self.header_length)
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-06-15 10:35 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-12  0:04 [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
2020-06-12  0:04 ` [PATCH v7 1/9] iotests: Fix for magic hexadecimal output in 291 Andrey Shinkevich
2020-06-15  9:43   ` Vladimir Sementsov-Ogievskiy
2020-06-12  0:04 ` [PATCH v7 2/9] qcow2: Fix capitalization of header extension constant Andrey Shinkevich
2020-06-15  9:44   ` Vladimir Sementsov-Ogievskiy
2020-06-12  0:04 ` [PATCH v7 3/9] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
2020-06-15  9:47   ` Vladimir Sementsov-Ogievskiy
2020-06-12  0:04 ` [PATCH v7 4/9] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
2020-06-15 10:34   ` Vladimir Sementsov-Ogievskiy
2020-06-12  0:04 ` [PATCH v7 5/9] qcow2_format.py: pass cluster size to substructures Andrey Shinkevich
2020-06-12  0:04 ` [PATCH v7 6/9] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
2020-06-12  0:05 ` [PATCH v7 7/9] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
2020-06-12  0:05 ` [PATCH v7 8/9] qcow2_format.py: collect fields " Andrey Shinkevich
2020-06-12  0:05 ` [PATCH v7 9/9] qcow2_format.py: support dumping metadata " Andrey Shinkevich
2020-06-15  9:42 ` [PATCH v7 0/9] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.