qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata
@ 2020-07-30 14:15 Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 01/11] iotests: add test for QCOW2 header dump Andrey Shinkevich
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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

v10:
  01: New. The test case #303 added to visualize the QCOW2 metadata dumping.
      Suggested by Eric. The patch "Fix capitalization of header extension
      constant" has been pulled separately.
  07: The class Qcow2BitmapTableEntry is now derived from the Qcow2Struct
      one. The constructors and methods of Qcow2BitmapTable and of
      Qcow2BitmapTableEntry were modified.
  09: The python dict.update method was replaced with assignment operator.
      The interleaving dict 'entries' was removed from bitmap table dump.
  10: The class Qcow2HeaderExtensionsDoc was removed.
  11: New. The #303 was extended by dumping QCOW2 metadata in JSON format.


Andrey Shinkevich (11):
  iotests: add test for QCOW2 header dump
  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: support dumping metadata in JSON format
  iotests: dump QCOW2 header in JSON in #303

 tests/qemu-iotests/303             |  62 +++++++++++
 tests/qemu-iotests/303.out         | 162 ++++++++++++++++++++++++++++
 tests/qemu-iotests/group           |   1 +
 tests/qemu-iotests/qcow2.py        |  18 +++-
 tests/qemu-iotests/qcow2_format.py | 210 ++++++++++++++++++++++++++++++++++---
 5 files changed, 432 insertions(+), 21 deletions(-)
 create mode 100755 tests/qemu-iotests/303
 create mode 100644 tests/qemu-iotests/303.out

-- 
1.8.3.1



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

* [PATCH v12 01/11] iotests: add test for QCOW2 header dump
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-07-30 19:05   ` Eric Blake
  2020-08-05 11:23   ` Vladimir Sementsov-Ogievskiy
  2020-07-30 14:15 ` [PATCH v12 02/11] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

The simple script creates a QCOW2 image and fills it with some data.
Two bitmaps are created as well. Then the script reads the image header
with extensions from the disk by running the script qcow2.py and dumps
the information to the output. Other entities, such as snapshots, may
be added to the test later.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/303     | 59 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/303.out | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 124 insertions(+)
 create mode 100755 tests/qemu-iotests/303
 create mode 100644 tests/qemu-iotests/303.out

diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
new file mode 100755
index 0000000..3c7a611
--- /dev/null
+++ b/tests/qemu-iotests/303
@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+#
+# Test for dumping of qcow2 image metadata
+#
+# Copyright (c) 2020 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+import subprocess
+from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io
+
+iotests.script_initialize(supported_fmts=['qcow2'])
+
+disk = file_path('disk')
+chunk = 1024 * 1024
+
+
+def create_bitmap(bitmap_number, disabled):
+    granularity = 1 << (14 + bitmap_number)
+    bitmap_name = 'bitmap-' + str(bitmap_number)
+    vm = iotests.VM().add_drive(disk)
+    vm.launch()
+    vm.qmp_log('block-dirty-bitmap-add', node='drive0', name=bitmap_name,
+               granularity=granularity, persistent=True, disabled=disabled)
+    vm.shutdown()
+
+
+def write_to_disk(offset, size):
+    write = f'write {offset} {size}'
+    log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
+
+
+def add_bitmap(num, begin, end, disabled):
+    log(f'Add bitmap {num}')
+    create_bitmap(num, disabled)
+    for i in range(begin, end):
+        write_to_disk((i-1) * chunk, chunk)
+    log('')
+
+
+qemu_img_create('-f', iotests.imgfmt, disk, '10M')
+
+add_bitmap(1, 1, 7, False)
+add_bitmap(2, 7, 9, True)
+dump = ['qcow2.py', f'{disk}', 'dump-header']
+subprocess.run(dump)
diff --git a/tests/qemu-iotests/303.out b/tests/qemu-iotests/303.out
new file mode 100644
index 0000000..8b11169
--- /dev/null
+++ b/tests/qemu-iotests/303.out
@@ -0,0 +1,64 @@
+Add bitmap 1
+{"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 32768, "name": "bitmap-1", "node": "drive0", "persistent": true}}
+{"return": {}}
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 2097152
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 3145728
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 4194304
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 5242880
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+
+Add bitmap 2
+{"execute": "block-dirty-bitmap-add", "arguments": {"disabled": true, "granularity": 65536, "name": "bitmap-2", "node": "drive0", "persistent": true}}
+{"return": {}}
+wrote 1048576/1048576 bytes at offset 6291456
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 7340032
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      10485760
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     []
+compatible_features       []
+autoclear_features        [0]
+refcount_order            4
+header_length             112
+
+Header extension:
+magic                     0x6803f857 (Feature table)
+length                    336
+data                      <binary>
+
+Header extension:
+magic                     0x23852875 (Bitmaps)
+length                    24
+nb_bitmaps                2
+reserved32                0
+bitmap_directory_size     0x40
+bitmap_directory_offset   0x9d0000
+
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 025ed52..7e12e1f 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -309,3 +309,4 @@
 299 auto quick
 301 backing quick
 302 quick
+303 rw quick
-- 
1.8.3.1



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

* [PATCH v12 02/11] qcow2_format.py: make printable data an extension class member
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 01/11] iotests: add test for QCOW2 header dump Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method Andrey Shinkevich
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 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 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



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

* [PATCH v12 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 01/11] iotests: add test for QCOW2 header dump Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 02/11] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 04/11] qcow2_format.py: dump bitmap flags in human readable way Andrey Shinkevich
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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



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

* [PATCH v12 04/11] qcow2_format.py: dump bitmap flags in human readable way.
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (2 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 05/11] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

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



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

* [PATCH v12 05/11] qcow2_format.py: Dump bitmap directory information
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (3 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 04/11] qcow2_format.py: dump bitmap flags in human readable way Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 06/11] qcow2_format.py: pass cluster size to substructures Andrey Shinkevich
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 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.

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/303.out         | 18 +++++++++++++++
 tests/qemu-iotests/qcow2_format.py | 47 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/tests/qemu-iotests/303.out b/tests/qemu-iotests/303.out
index 8b11169..dc3739b 100644
--- a/tests/qemu-iotests/303.out
+++ b/tests/qemu-iotests/303.out
@@ -62,3 +62,21 @@ reserved32                0
 bitmap_directory_size     0x40
 bitmap_directory_offset   0x9d0000
 
+Bitmap name               bitmap-1
+bitmap_table_offset       0x9b0000
+bitmap_table_size         1
+flags                     0x2 (['auto'])
+type                      1
+granularity_bits          15
+name_size                 8
+extra_data_size           0
+
+Bitmap name               bitmap-2
+bitmap_table_offset       0x9c0000
+bitmap_table_size         1
+flags                     0x0 ([])
+type                      1
+granularity_bits          16
+name_size                 8
+extra_data_size           0
+
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



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

* [PATCH v12 06/11] qcow2_format.py: pass cluster size to substructures
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (4 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 05/11] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 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>
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



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

* [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (5 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 06/11] qcow2_format.py: pass cluster size to substructures Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-08-05 15:57   ` Vladimir Sementsov-Ogievskiy
  2020-07-30 14:15 ` [PATCH v12 08/11] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

Add bitmap table information to the QCOW2 metadata dump.

Bitmap name               bitmap-1
...
Bitmap table   type            size         offset
0              serialized      65536        10092544
1              all-zeroes      65536        0
2              all-zeroes      65536        0

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/303.out         |  4 ++++
 tests/qemu-iotests/qcow2_format.py | 47 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/tests/qemu-iotests/303.out b/tests/qemu-iotests/303.out
index dc3739b..d581fb4 100644
--- a/tests/qemu-iotests/303.out
+++ b/tests/qemu-iotests/303.out
@@ -70,6 +70,8 @@ type                      1
 granularity_bits          15
 name_size                 8
 extra_data_size           0
+Bitmap table   type            size         offset
+0              serialized      65536        10092544
 
 Bitmap name               bitmap-2
 bitmap_table_offset       0x9c0000
@@ -79,4 +81,6 @@ type                      1
 granularity_bits          16
 name_size                 8
 extra_data_size           0
+Bitmap table   type            size         offset
+0              all-zeroes      65536        0
 
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index ca0d350..1f033d4 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,
+                                             nb_entries=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,49 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
     def dump(self):
         print(f'{"Bitmap name":<25} {self.name}')
         super(Qcow2BitmapDirEntry, self).dump()
+        self.bitmap_table.dump()
+
+
+class Qcow2BitmapTableEntry(Qcow2Struct):
+
+    fields = (
+        ('u64',  '{}', 'entry'),
+    )
+
+    BME_TABLE_ENTRY_RESERVED_MASK = 0xff000000000001fe
+    BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
+    BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
+
+    def __init__(self, fd):
+        super().__init__(fd=fd)
+        self.reserved = self.entry & self.BME_TABLE_ENTRY_RESERVED_MASK
+        self.offset = self.entry & self.BME_TABLE_ENTRY_OFFSET_MASK
+        if self.offset:
+            if self.entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
+                self.type = 'invalid'
+            else:
+                self.type = 'serialized'
+        elif self.entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
+            self.type = 'all-ones'
+        else:
+            self.type = 'all-zeroes'
+
+
+class Qcow2BitmapTable:
+
+    def __init__(self, fd, offset, nb_entries, cluster_size):
+        self.cluster_size = cluster_size
+        position = fd.tell()
+        fd.seek(offset)
+        self.entries = [Qcow2BitmapTableEntry(fd) for _ in range(nb_entries)]
+        fd.seek(position)
+
+    def dump(self):
+        size = self.cluster_size
+        bitmap_table = enumerate(self.entries)
+        print(f'{"Bitmap table":<14} {"type":<15} {"size":<12} {"offset"}')
+        for i, entry in bitmap_table:
+            print(f'{i:<14} {entry.type:<15} {size:<12} {entry.offset}')
 
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
-- 
1.8.3.1



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

* [PATCH v12 08/11] qcow2.py: Introduce '-j' key to dump in JSON format
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (6 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-08-05 16:12   ` Vladimir Sementsov-Ogievskiy
  2020-07-30 14:15 ` [PATCH v12 09/11] qcow2_format.py: collect fields " Andrey Shinkevich
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 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        | 18 ++++++++++++++----
 tests/qemu-iotests/qcow2_format.py |  4 ++--
 2 files changed, 16 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 1f033d4..2000de3 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):
@@ -405,7 +405,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



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

* [PATCH v12 09/11] qcow2_format.py: collect fields to dump in JSON format
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (7 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 08/11] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-08-05 16:58   ` Vladimir Sementsov-Ogievskiy
  2020-07-30 14:15 ` [PATCH v12 10/11] qcow2_format.py: support dumping metadata " Andrey Shinkevich
  2020-07-30 14:15 ` [PATCH v12 11/11] iotests: dump QCOW2 header in JSON in #303 Andrey Shinkevich
  10 siblings, 1 reply; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 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, 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 | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 2000de3..a4114cb 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):
 
@@ -151,6 +154,11 @@ class Qcow2BitmapExt(Qcow2Struct):
             print()
             entry.dump()
 
+    def to_dict(self):
+        fields_dict = super().to_dict()
+        fields_dict['bitmap_directory'] = self.bitmap_directory
+        return fields_dict
+
 
 class Qcow2BitmapDirEntry(Qcow2Struct):
 
@@ -189,6 +197,14 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
         super(Qcow2BitmapDirEntry, self).dump()
         self.bitmap_table.dump()
 
+    def to_dict(self):
+        fields_dict = super().to_dict()
+        fields_dict['bitmap_table'] = self.bitmap_table.entries
+        bmp_name = dict(name=self.name)
+        # Put the name ahead of the dict
+        bme_dict = {**bmp_name, **fields_dict}
+        return bme_dict
+
 
 class Qcow2BitmapTableEntry(Qcow2Struct):
 
@@ -214,6 +230,9 @@ class Qcow2BitmapTableEntry(Qcow2Struct):
         else:
             self.type = 'all-zeroes'
 
+    def to_dict(self):
+        return dict(type=self.type, offset=self.offset, reserved=self.reserved)
+
 
 class Qcow2BitmapTable:
 
@@ -246,6 +265,9 @@ class QcowHeaderExtension(Qcow2Struct):
             0x44415441: 'Data file'
         }
 
+        def to_dict(self):
+            return self.mapping.get(self.value, "<unknown>")
+
     fields = (
         ('u32', Magic, 'magic'),
         ('u32', '{}', 'length')
@@ -308,6 +330,18 @@ class QcowHeaderExtension(Qcow2Struct):
         else:
             self.obj.dump()
 
+    def to_dict(self):
+        fields_dict = super().to_dict()
+        ext_name = dict(name=self.Magic(self.magic))
+        # Put the name ahead of the dict
+        he_dict = {**ext_name, **fields_dict}
+        if self.obj is not None:
+            he_dict['data'] = self.obj
+        else:
+            he_dict['data_str'] = self.data_str
+
+        return he_dict
+
     @classmethod
     def create(cls, magic, data):
         return QcowHeaderExtension(magic, len(data), data)
-- 
1.8.3.1



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

* [PATCH v12 10/11] qcow2_format.py: support dumping metadata in JSON format
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (8 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 09/11] qcow2_format.py: collect fields " Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-08-05 17:04   ` Vladimir Sementsov-Ogievskiy
  2020-07-30 14:15 ` [PATCH v12 11/11] iotests: dump QCOW2 header in JSON in #303 Andrey Shinkevich
  10 siblings, 1 reply; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 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,
                "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": [
                            {
                                "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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index a4114cb..7487720 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,10 @@ 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):
@@ -440,6 +453,10 @@ class QcowHeader(Qcow2Struct):
         fd.write(buf)
 
     def dump_extensions(self, is_json=False):
+        if is_json:
+            print(json.dumps(self.extensions, indent=4, cls=ComplexEncoder))
+            return
+
         for ex in self.extensions:
             print('Header extension:')
             ex.dump()
-- 
1.8.3.1



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

* [PATCH v12 11/11] iotests: dump QCOW2 header in JSON in #303
  2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
                   ` (9 preceding siblings ...)
  2020-07-30 14:15 ` [PATCH v12 10/11] qcow2_format.py: support dumping metadata " Andrey Shinkevich
@ 2020-07-30 14:15 ` Andrey Shinkevich
  2020-08-05 17:08   ` Vladimir Sementsov-Ogievskiy
  10 siblings, 1 reply; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-30 14:15 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den

Extend the test case #303 by dumping QCOW2 image metadata in JSON
format.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/303     |  3 ++
 tests/qemu-iotests/303.out | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
index 3c7a611..6821bd3 100755
--- a/tests/qemu-iotests/303
+++ b/tests/qemu-iotests/303
@@ -57,3 +57,6 @@ add_bitmap(1, 1, 7, False)
 add_bitmap(2, 7, 9, True)
 dump = ['qcow2.py', f'{disk}', 'dump-header']
 subprocess.run(dump)
+# Dump the metadata in JSON format
+dump.append('-j')
+subprocess.run(dump)
diff --git a/tests/qemu-iotests/303.out b/tests/qemu-iotests/303.out
index d581fb4..ead3b63 100644
--- a/tests/qemu-iotests/303.out
+++ b/tests/qemu-iotests/303.out
@@ -84,3 +84,79 @@ extra_data_size           0
 Bitmap table   type            size         offset
 0              all-zeroes      65536        0
 
+{
+    "magic": 1363560955,
+    "version": 3,
+    "backing_file_offset": 0,
+    "backing_file_size": 0,
+    "cluster_bits": 16,
+    "size": 10485760,
+    "crypt_method": 0,
+    "l1_size": 1,
+    "l1_table_offset": 196608,
+    "refcount_table_offset": 65536,
+    "refcount_table_clusters": 1,
+    "nb_snapshots": 0,
+    "snapshot_offset": 0,
+    "incompatible_features": 0,
+    "compatible_features": 0,
+    "autoclear_features": 1,
+    "refcount_order": 4,
+    "header_length": 112
+}
+
+[
+    {
+        "name": "Feature table",
+        "magic": 1745090647,
+        "length": 336,
+        "data_str": "<binary>"
+    },
+    {
+        "name": "Bitmaps",
+        "magic": 595929205,
+        "length": 24,
+        "data": {
+            "nb_bitmaps": 2,
+            "reserved32": 0,
+            "bitmap_directory_size": 64,
+            "bitmap_directory_offset": 10289152,
+            "bitmap_directory": [
+                {
+                    "name": "bitmap-1",
+                    "bitmap_table_offset": 10158080,
+                    "bitmap_table_size": 1,
+                    "flags": 2,
+                    "type": 1,
+                    "granularity_bits": 15,
+                    "name_size": 8,
+                    "extra_data_size": 0,
+                    "bitmap_table": [
+                        {
+                            "type": "serialized",
+                            "offset": 10092544,
+                            "reserved": 0
+                        }
+                    ]
+                },
+                {
+                    "name": "bitmap-2",
+                    "bitmap_table_offset": 10223616,
+                    "bitmap_table_size": 1,
+                    "flags": 0,
+                    "type": 1,
+                    "granularity_bits": 16,
+                    "name_size": 8,
+                    "extra_data_size": 0,
+                    "bitmap_table": [
+                        {
+                            "type": "all-zeroes",
+                            "offset": 0,
+                            "reserved": 0
+                        }
+                    ]
+                }
+            ]
+        }
+    }
+]
-- 
1.8.3.1



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

* Re: [PATCH v12 01/11] iotests: add test for QCOW2 header dump
  2020-07-30 14:15 ` [PATCH v12 01/11] iotests: add test for QCOW2 header dump Andrey Shinkevich
@ 2020-07-30 19:05   ` Eric Blake
  2020-07-31  7:28     ` Andrey Shinkevich
  2020-08-05 11:23   ` Vladimir Sementsov-Ogievskiy
  1 sibling, 1 reply; 23+ messages in thread
From: Eric Blake @ 2020-07-30 19:05 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

On 7/30/20 9:15 AM, Andrey Shinkevich wrote:
> The simple script creates a QCOW2 image and fills it with some data.
> Two bitmaps are created as well. Then the script reads the image header
> with extensions from the disk by running the script qcow2.py and dumps
> the information to the output. Other entities, such as snapshots, may
> be added to the test later.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/303     | 59 ++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/303.out | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 124 insertions(+)
>   create mode 100755 tests/qemu-iotests/303
>   create mode 100644 tests/qemu-iotests/303.out

> +import iotests
> +import subprocess
> +from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io
> +
> +iotests.script_initialize(supported_fmts=['qcow2'])
> +
> +disk = file_path('disk')
> +chunk = 1024 * 1024
> +
> +
> +def create_bitmap(bitmap_number, disabled):
> +    granularity = 1 << (14 + bitmap_number)
> +    bitmap_name = 'bitmap-' + str(bitmap_number)
> +    vm = iotests.VM().add_drive(disk)
> +    vm.launch()
> +    vm.qmp_log('block-dirty-bitmap-add', node='drive0', name=bitmap_name,
> +               granularity=granularity, persistent=True, disabled=disabled)
> +    vm.shutdown()

Would it be any easier to use qemu-img bitmap here instead of firing up 
a full VM?

At any rate, this is a nice starting point for tracking what the rest of 
your series improves.

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

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



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

* Re: [PATCH v12 01/11] iotests: add test for QCOW2 header dump
  2020-07-30 19:05   ` Eric Blake
@ 2020-07-31  7:28     ` Andrey Shinkevich
  0 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-07-31  7:28 UTC (permalink / raw)
  To: Eric Blake, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

On 30.07.2020 22:05, Eric Blake wrote:
> On 7/30/20 9:15 AM, Andrey Shinkevich wrote:
>> The simple script creates a QCOW2 image and fills it with some data.
>> Two bitmaps are created as well. Then the script reads the image header
>> with extensions from the disk by running the script qcow2.py and dumps
>> the information to the output. Other entities, such as snapshots, may
>> be added to the test later.
>>
>> Suggested-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/303     | 59 
>> ++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/303.out | 64 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 124 insertions(+)
>>   create mode 100755 tests/qemu-iotests/303
>>   create mode 100644 tests/qemu-iotests/303.out
>
>> +import iotests
>> +import subprocess
>> +from iotests import qemu_img_create, qemu_io, file_path, log, 
>> filter_qemu_io
>> +
>> +iotests.script_initialize(supported_fmts=['qcow2'])
>> +
>> +disk = file_path('disk')
>> +chunk = 1024 * 1024
>> +
>> +
>> +def create_bitmap(bitmap_number, disabled):
>> +    granularity = 1 << (14 + bitmap_number)
>> +    bitmap_name = 'bitmap-' + str(bitmap_number)
>> +    vm = iotests.VM().add_drive(disk)
>> +    vm.launch()
>> +    vm.qmp_log('block-dirty-bitmap-add', node='drive0', 
>> name=bitmap_name,
>> +               granularity=granularity, persistent=True, 
>> disabled=disabled)
>> +    vm.shutdown()
>
> Would it be any easier to use qemu-img bitmap here instead of firing 
> up a full VM?


I agree, you are right. I'll take it into account for the next version, 
if any.

Thank you for your R-b.

Andrey


>
> At any rate, this is a nice starting point for tracking what the rest 
> of your series improves.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>


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

* Re: [PATCH v12 01/11] iotests: add test for QCOW2 header dump
  2020-07-30 14:15 ` [PATCH v12 01/11] iotests: add test for QCOW2 header dump Andrey Shinkevich
  2020-07-30 19:05   ` Eric Blake
@ 2020-08-05 11:23   ` Vladimir Sementsov-Ogievskiy
  2020-08-05 12:10     ` Andrey Shinkevich
  1 sibling, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-05 11:23 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

30.07.2020 17:15, Andrey Shinkevich wrote:
> The simple script creates a QCOW2 image and fills it with some data.
> Two bitmaps are created as well. Then the script reads the image header
> with extensions from the disk by running the script qcow2.py and dumps
> the information to the output. Other entities, such as snapshots, may
> be added to the test later.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/303     | 59 ++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/303.out | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 124 insertions(+)
>   create mode 100755 tests/qemu-iotests/303
>   create mode 100644 tests/qemu-iotests/303.out
> 
> diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
> new file mode 100755
> index 0000000..3c7a611
> --- /dev/null
> +++ b/tests/qemu-iotests/303
> @@ -0,0 +1,59 @@
> +#!/usr/bin/env python3
> +#
> +# Test for dumping of qcow2 image metadata
> +#
> +# Copyright (c) 2020 Virtuozzo International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import iotests
> +import subprocess
> +from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io
> +
> +iotests.script_initialize(supported_fmts=['qcow2'])
> +
> +disk = file_path('disk')
> +chunk = 1024 * 1024
> +
> +
> +def create_bitmap(bitmap_number, disabled):
> +    granularity = 1 << (14 + bitmap_number)
> +    bitmap_name = 'bitmap-' + str(bitmap_number)
> +    vm = iotests.VM().add_drive(disk)
> +    vm.launch()
> +    vm.qmp_log('block-dirty-bitmap-add', node='drive0', name=bitmap_name,
> +               granularity=granularity, persistent=True, disabled=disabled)
> +    vm.shutdown()
> +
> +
> +def write_to_disk(offset, size):
> +    write = f'write {offset} {size}'
> +    log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
> +
> +
> +def add_bitmap(num, begin, end, disabled):
> +    log(f'Add bitmap {num}')
> +    create_bitmap(num, disabled)
> +    for i in range(begin, end):
> +        write_to_disk((i-1) * chunk, chunk)

a bit unusual to count chunks starting from "1"..

also, any difference with just

write_to_disk((i-1) * chunk, (end-begin) * chunk)

?

> +    log('')
> +
> +
> +qemu_img_create('-f', iotests.imgfmt, disk, '10M')
> +
> +add_bitmap(1, 1, 7, False)
> +add_bitmap(2, 7, 9, True)
> +dump = ['qcow2.py', f'{disk}', 'dump-header']

No reason to put disk into f-string, it's a string anyway: f'{disk}' is equal to just disk.

> +subprocess.run(dump)


And you may use just

    subprocess.run(['qcow2.py', disk, 'dump-header'])

without additional variable.


Still, I'm OK with it as is:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir


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

* Re: [PATCH v12 01/11] iotests: add test for QCOW2 header dump
  2020-08-05 11:23   ` Vladimir Sementsov-Ogievskiy
@ 2020-08-05 12:10     ` Andrey Shinkevich
  0 siblings, 0 replies; 23+ messages in thread
From: Andrey Shinkevich @ 2020-08-05 12:10 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

On 05.08.2020 14:23, Vladimir Sementsov-Ogievskiy wrote:
> 30.07.2020 17:15, Andrey Shinkevich wrote:
>> The simple script creates a QCOW2 image and fills it with some data.
>> Two bitmaps are created as well. Then the script reads the image header
>> with extensions from the disk by running the script qcow2.py and dumps
>> the information to the output. Other entities, such as snapshots, may
>> be added to the test later.
>>
>> Suggested-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/303     | 59 
>> ++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/303.out | 64 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 124 insertions(+)
>>   create mode 100755 tests/qemu-iotests/303
>>   create mode 100644 tests/qemu-iotests/303.out
>>
>> diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
>> new file mode 100755
>> index 0000000..3c7a611
>> --- /dev/null
>> +++ b/tests/qemu-iotests/303
>> @@ -0,0 +1,59 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Test for dumping of qcow2 image metadata
>> +#
>> +# Copyright (c) 2020 Virtuozzo International GmbH
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +import iotests
>> +import subprocess
>> +from iotests import qemu_img_create, qemu_io, file_path, log, 
>> filter_qemu_io
>> +
>> +iotests.script_initialize(supported_fmts=['qcow2'])
>> +
>> +disk = file_path('disk')
>> +chunk = 1024 * 1024
>> +
>> +
>> +def create_bitmap(bitmap_number, disabled):
>> +    granularity = 1 << (14 + bitmap_number)
>> +    bitmap_name = 'bitmap-' + str(bitmap_number)
>> +    vm = iotests.VM().add_drive(disk)
>> +    vm.launch()
>> +    vm.qmp_log('block-dirty-bitmap-add', node='drive0', 
>> name=bitmap_name,
>> +               granularity=granularity, persistent=True, 
>> disabled=disabled)
>> +    vm.shutdown()
>> +
>> +
>> +def write_to_disk(offset, size):
>> +    write = f'write {offset} {size}'
>> +    log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
>> +
>> +
>> +def add_bitmap(num, begin, end, disabled):
>> +    log(f'Add bitmap {num}')
>> +    create_bitmap(num, disabled)
>> +    for i in range(begin, end):
>> +        write_to_disk((i-1) * chunk, chunk)
>
> a bit unusual to count chunks starting from "1"..
>
> also, any difference with just
>
> write_to_disk((i-1) * chunk, (end-begin) * chunk)
>
> ?
>

QEMU-IMG limits the maximum number of bytes written with one command by 
... 2M, if I am not wrong.

Andrey


>> +    log('')
>> +
>> +
>> +qemu_img_create('-f', iotests.imgfmt, disk, '10M')
>> +
>> +add_bitmap(1, 1, 7, False)
>> +add_bitmap(2, 7, 9, True)
>> +dump = ['qcow2.py', f'{disk}', 'dump-header']
>
> No reason to put disk into f-string, it's a string anyway: f'{disk}' 
> is equal to just disk.


Thanks


>
>> +subprocess.run(dump)
>
>
> And you may use just
>
>    subprocess.run(['qcow2.py', disk, 'dump-header'])
>
> without additional variable.


Yes, but further adding the '-j' key to the list looks more elegant to me ))

Andrey


>
>
> Still, I'm OK with it as is:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
>


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

* Re: [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries
  2020-07-30 14:15 ` [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
@ 2020-08-05 15:57   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-05 15:57 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

30.07.2020 17:15, Andrey Shinkevich wrote:
> Add bitmap table information to the QCOW2 metadata dump.
> 
> Bitmap name               bitmap-1
> ...
> Bitmap table   type            size         offset
> 0              serialized      65536        10092544
> 1              all-zeroes      65536        0

For serialized, size and offset are size and offset of bitmap table.
But, all-zeroes bitmaps don't have any bitmap table, so size and offset
both are undefined (OK to print zero for them, but 65536 is unrelated).

> 2              all-zeroes      65536        0
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/303.out         |  4 ++++
>   tests/qemu-iotests/qcow2_format.py | 47 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 51 insertions(+)
> 
> diff --git a/tests/qemu-iotests/303.out b/tests/qemu-iotests/303.out
> index dc3739b..d581fb4 100644
> --- a/tests/qemu-iotests/303.out
> +++ b/tests/qemu-iotests/303.out
> @@ -70,6 +70,8 @@ type                      1
>   granularity_bits          15
>   name_size                 8
>   extra_data_size           0
> +Bitmap table   type            size         offset
> +0              serialized      65536        10092544
>   
>   Bitmap name               bitmap-2
>   bitmap_table_offset       0x9c0000
> @@ -79,4 +81,6 @@ type                      1
>   granularity_bits          16
>   name_size                 8
>   extra_data_size           0
> +Bitmap table   type            size         offset
> +0              all-zeroes      65536        0
>   
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index ca0d350..1f033d4 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,
> +                                             nb_entries=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,49 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>       def dump(self):
>           print(f'{"Bitmap name":<25} {self.name}')
>           super(Qcow2BitmapDirEntry, self).dump()
> +        self.bitmap_table.dump()
> +
> +
> +class Qcow2BitmapTableEntry(Qcow2Struct):
> +
> +    fields = (
> +        ('u64',  '{}', 'entry'),
> +    )
> +
> +    BME_TABLE_ENTRY_RESERVED_MASK = 0xff000000000001fe
> +    BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
> +    BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
> +
> +    def __init__(self, fd):
> +        super().__init__(fd=fd)
> +        self.reserved = self.entry & self.BME_TABLE_ENTRY_RESERVED_MASK
> +        self.offset = self.entry & self.BME_TABLE_ENTRY_OFFSET_MASK
> +        if self.offset:
> +            if self.entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
> +                self.type = 'invalid'
> +            else:
> +                self.type = 'serialized'
> +        elif self.entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
> +            self.type = 'all-ones'
> +        else:
> +            self.type = 'all-zeroes'
> +
> +
> +class Qcow2BitmapTable:
> +
> +    def __init__(self, fd, offset, nb_entries, cluster_size):
> +        self.cluster_size = cluster_size
> +        position = fd.tell()
> +        fd.seek(offset)
> +        self.entries = [Qcow2BitmapTableEntry(fd) for _ in range(nb_entries)]
> +        fd.seek(position)
> +
> +    def dump(self):
> +        size = self.cluster_size
> +        bitmap_table = enumerate(self.entries)
> +        print(f'{"Bitmap table":<14} {"type":<15} {"size":<12} {"offset"}')
> +        for i, entry in bitmap_table:
> +            print(f'{i:<14} {entry.type:<15} {size:<12} {entry.offset}')
>   

All this looks like 'cluster_size' is not really needed for Qcow2BitmapTable class (we can print only offsets).
Still, if you want to save it, can we print it only for entries with 'serialized' type?

It's minor, I'm OK with it as is:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


>   
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
> 


-- 
Best regards,
Vladimir


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

* Re: [PATCH v12 08/11] qcow2.py: Introduce '-j' key to dump in JSON format
  2020-07-30 14:15 ` [PATCH v12 08/11] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
@ 2020-08-05 16:12   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-05 16:12 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

30.07.2020 17:15, 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>

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

-- 
Best regards,
Vladimir


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

* Re: [PATCH v12 09/11] qcow2_format.py: collect fields to dump in JSON format
  2020-07-30 14:15 ` [PATCH v12 09/11] qcow2_format.py: collect fields " Andrey Shinkevich
@ 2020-08-05 16:58   ` Vladimir Sementsov-Ogievskiy
  2020-08-06  9:08     ` Andrey Shinkevich
  0 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-05 16:58 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

30.07.2020 17:15, 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 | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index 2000de3..a4114cb 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):
>   
> @@ -151,6 +154,11 @@ class Qcow2BitmapExt(Qcow2Struct):
>               print()
>               entry.dump()
>   
> +    def to_dict(self):
> +        fields_dict = super().to_dict()
> +        fields_dict['bitmap_directory'] = self.bitmap_directory
> +        return fields_dict
> +
>   
>   class Qcow2BitmapDirEntry(Qcow2Struct):
>   
> @@ -189,6 +197,14 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>           super(Qcow2BitmapDirEntry, self).dump()
>           self.bitmap_table.dump()
>   
> +    def to_dict(self):
> +        fields_dict = super().to_dict()
> +        fields_dict['bitmap_table'] = self.bitmap_table.entries

the fact that we have to access internals of bitmap_table is not nice, but let's refactor it later.

> +        bmp_name = dict(name=self.name)
> +        # Put the name ahead of the dict

Aha. I don't think that ordering is necessary for json, but why not to order it a bit.

> +        bme_dict = {**bmp_name, **fields_dict}


strange to create bmp_name dict just to unpack it. You may as well do

bme_dict = {'name': self.name, **fields_dict}

> +        return bme_dict

bme_dict is extra variable: it's created just to return it, so, how about just

return {'name': self.name, **fields_dict}


or, maybe

return {
            'name': self.name,
            **super().to_dict(),
            'bitmap_table': self.bitmap_table.entries
        }

> +
>   
>   class Qcow2BitmapTableEntry(Qcow2Struct):
>   
> @@ -214,6 +230,9 @@ class Qcow2BitmapTableEntry(Qcow2Struct):
>           else:
>               self.type = 'all-zeroes'
>   
> +    def to_dict(self):
> +        return dict(type=self.type, offset=self.offset, reserved=self.reserved)
> +

Python has a special syntax for creating dicts :), let's use:

return {'type': self.type, 'offset': self.offset, 'reserved': self.reserved}


>   
>   class Qcow2BitmapTable:
>   
> @@ -246,6 +265,9 @@ class QcowHeaderExtension(Qcow2Struct):
>               0x44415441: 'Data file'
>           }
>   
> +        def to_dict(self):
> +            return self.mapping.get(self.value, "<unknown>")

aha, so, actually, to_dict() returns not dict, but string. So it should better be named to_json() (and return any json-dumpable object, like string or dict)

and then, we can add to_json() method to Qcow2BitmapTable as well, to return list.


> +
>       fields = (
>           ('u32', Magic, 'magic'),
>           ('u32', '{}', 'length')
> @@ -308,6 +330,18 @@ class QcowHeaderExtension(Qcow2Struct):
>           else:
>               self.obj.dump()
>   
> +    def to_dict(self):
> +        fields_dict = super().to_dict()
> +        ext_name = dict(name=self.Magic(self.magic))

strange that we have to create Magic object here... Ok, let's refactor it later

> +        # Put the name ahead of the dict
> +        he_dict = {**ext_name, **fields_dict}
> +        if self.obj is not None:
> +            he_dict['data'] = self.obj
> +        else:
> +            he_dict['data_str'] = self.data_str
> +
> +        return he_dict

again, let's avoid extra dict variables:

res = {'name': self.Magic(self.magic), **super().to_dict()}
if ...


> +
>       @classmethod
>       def create(cls, magic, data):
>           return QcowHeaderExtension(magic, len(data), data)
> 


-- 
Best regards,
Vladimir


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

* Re: [PATCH v12 10/11] qcow2_format.py: support dumping metadata in JSON format
  2020-07-30 14:15 ` [PATCH v12 10/11] qcow2_format.py: support dumping metadata " Andrey Shinkevich
@ 2020-08-05 17:04   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-05 17:04 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

30.07.2020 17:15, Andrey Shinkevich wrote:
> Implementation of dumping QCOW2 image metadata.
> The sample output:
> {
>      "Header_extensions": [
>          {
>              "name": "Feature table",
>              "magic": 1745090647,
>              "length": 192,
>              "data_str": "<binary>"
>          },
>          {
>              "name": "Bitmaps",
>              "magic": 595929205,
>              "length": 24,
>              "data": {
>                  "nb_bitmaps": 2,
>                  "reserved32": 0,
>                  "bitmap_directory_size": 64,
>                  "bitmap_directory_offset": 1048576,
>                  "bitmap_directory": [
>                      {
>                          "name": "bitmap-1",
>                          "bitmap_table_offset": 589824,
>                          "bitmap_table_size": 1,
>                          "flags": 2,
>                          "type": 1,
>                          "granularity_bits": 15,
>                          "name_size": 8,
>                          "extra_data_size": 0,
>                          "bitmap_table": [
>                              {
>                                  "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 | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index a4114cb..7487720 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,10 @@ 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):
> @@ -440,6 +453,10 @@ class QcowHeader(Qcow2Struct):
>           fd.write(buf)
>   
>       def dump_extensions(self, is_json=False):
> +        if is_json:
> +            print(json.dumps(self.extensions, indent=4, cls=ComplexEncoder))
> +            return
> +
>           for ex in self.extensions:
>               print('Header extension:')
>               ex.dump()
> 

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

-- 
Best regards,
Vladimir


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

* Re: [PATCH v12 11/11] iotests: dump QCOW2 header in JSON in #303
  2020-07-30 14:15 ` [PATCH v12 11/11] iotests: dump QCOW2 header in JSON in #303 Andrey Shinkevich
@ 2020-08-05 17:08   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-05 17:08 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

30.07.2020 17:15, Andrey Shinkevich wrote:
> Extend the test case #303 by dumping QCOW2 image metadata in JSON
> format.
> 
> 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] 23+ messages in thread

* Re: [PATCH v12 09/11] qcow2_format.py: collect fields to dump in JSON format
  2020-08-05 16:58   ` Vladimir Sementsov-Ogievskiy
@ 2020-08-06  9:08     ` Andrey Shinkevich
  2020-08-06  9:18       ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 23+ messages in thread
From: Andrey Shinkevich @ 2020-08-06  9:08 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

On 05.08.2020 19:58, Vladimir Sementsov-Ogievskiy wrote:
> 30.07.2020 17:15, 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 | 34 
>> ++++++++++++++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/qcow2_format.py 
>> b/tests/qemu-iotests/qcow2_format.py
>> index 2000de3..a4114cb 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):
>>   @@ -151,6 +154,11 @@ class Qcow2BitmapExt(Qcow2Struct):
>>               print()
>>               entry.dump()
>>   +    def to_dict(self):
>> +        fields_dict = super().to_dict()
>> +        fields_dict['bitmap_directory'] = self.bitmap_directory
>> +        return fields_dict
>> +
>>     class Qcow2BitmapDirEntry(Qcow2Struct):
>>   @@ -189,6 +197,14 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>>           super(Qcow2BitmapDirEntry, self).dump()
>>           self.bitmap_table.dump()
>>   +    def to_dict(self):
>> +        fields_dict = super().to_dict()
>> +        fields_dict['bitmap_table'] = self.bitmap_table.entries
>
> the fact that we have to access internals of bitmap_table is not nice, 
> but let's refactor it later.
>
>> +        bmp_name = dict(name=self.name)
>> +        # Put the name ahead of the dict
>
> Aha. I don't think that ordering is necessary for json, but why not to 
> order it a bit.
>
>> +        bme_dict = {**bmp_name, **fields_dict}
>
>
> strange to create bmp_name dict just to unpack it. You may as well do
>
> bme_dict = {'name': self.name, **fields_dict}
>
>> +        return bme_dict
>
> bme_dict is extra variable: it's created just to return it, so, how 
> about just
>
> return {'name': self.name, **fields_dict}
>
>
> or, maybe
>
> return {
>            'name': self.name,
>            **super().to_dict(),
>            'bitmap_table': self.bitmap_table.entries
>        }
>
>> +
>>     class Qcow2BitmapTableEntry(Qcow2Struct):
>>   @@ -214,6 +230,9 @@ class Qcow2BitmapTableEntry(Qcow2Struct):
>>           else:
>>               self.type = 'all-zeroes'
>>   +    def to_dict(self):
>> +        return dict(type=self.type, offset=self.offset, 
>> reserved=self.reserved)
>> +
>
> Python has a special syntax for creating dicts :), let's use:
>
> return {'type': self.type, 'offset': self.offset, 'reserved': 
> self.reserved}
>
>
>>     class Qcow2BitmapTable:
>>   @@ -246,6 +265,9 @@ class QcowHeaderExtension(Qcow2Struct):
>>               0x44415441: 'Data file'
>>           }
>>   +        def to_dict(self):
>> +            return self.mapping.get(self.value, "<unknown>")
>
> aha, so, actually, to_dict() returns not dict, but string. So it 
> should better be named to_json() (and return any json-dumpable object, 
> like string or dict)
>
> and then, we can add to_json() method to Qcow2BitmapTable as well, to 
> return list.


So, should I refactor it now?

Andrey


>
>
>> +
>>       fields = (
>>           ('u32', Magic, 'magic'),
>>           ('u32', '{}', 'length')
>> @@ -308,6 +330,18 @@ class QcowHeaderExtension(Qcow2Struct):
>>           else:
>>               self.obj.dump()
>>   +    def to_dict(self):
>> +        fields_dict = super().to_dict()
>> +        ext_name = dict(name=self.Magic(self.magic))
>
> strange that we have to create Magic object here... Ok, let's refactor 
> it later
>
>> +        # Put the name ahead of the dict
>> +        he_dict = {**ext_name, **fields_dict}
>> +        if self.obj is not None:
>> +            he_dict['data'] = self.obj
>> +        else:
>> +            he_dict['data_str'] = self.data_str
>> +
>> +        return he_dict
>
> again, let's avoid extra dict variables:
>
> res = {'name': self.Magic(self.magic), **super().to_dict()}
> if ...
>
>
>> +
>>       @classmethod
>>       def create(cls, magic, data):
>>           return QcowHeaderExtension(magic, len(data), data)
>>
>
>


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

* Re: [PATCH v12 09/11] qcow2_format.py: collect fields to dump in JSON format
  2020-08-06  9:08     ` Andrey Shinkevich
@ 2020-08-06  9:18       ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-08-06  9:18 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz

06.08.2020 12:08, Andrey Shinkevich wrote:
> On 05.08.2020 19:58, Vladimir Sementsov-Ogievskiy wrote:
>> 30.07.2020 17:15, 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 | 34 ++++++++++++++++++++++++++++++++++
>>>   1 file changed, 34 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
>>> index 2000de3..a4114cb 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):
>>>   @@ -151,6 +154,11 @@ class Qcow2BitmapExt(Qcow2Struct):
>>>               print()
>>>               entry.dump()
>>>   +    def to_dict(self):
>>> +        fields_dict = super().to_dict()
>>> +        fields_dict['bitmap_directory'] = self.bitmap_directory
>>> +        return fields_dict
>>> +
>>>     class Qcow2BitmapDirEntry(Qcow2Struct):
>>>   @@ -189,6 +197,14 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
>>>           super(Qcow2BitmapDirEntry, self).dump()
>>>           self.bitmap_table.dump()
>>>   +    def to_dict(self):
>>> +        fields_dict = super().to_dict()
>>> +        fields_dict['bitmap_table'] = self.bitmap_table.entries
>>
>> the fact that we have to access internals of bitmap_table is not nice, but let's refactor it later.
>>
>>> +        bmp_name = dict(name=self.name)
>>> +        # Put the name ahead of the dict
>>
>> Aha. I don't think that ordering is necessary for json, but why not to order it a bit.
>>
>>> +        bme_dict = {**bmp_name, **fields_dict}
>>
>>
>> strange to create bmp_name dict just to unpack it. You may as well do
>>
>> bme_dict = {'name': self.name, **fields_dict}
>>
>>> +        return bme_dict
>>
>> bme_dict is extra variable: it's created just to return it, so, how about just
>>
>> return {'name': self.name, **fields_dict}
>>
>>
>> or, maybe
>>
>> return {
>>            'name': self.name,
>>            **super().to_dict(),
>>            'bitmap_table': self.bitmap_table.entries
>>        }
>>
>>> +
>>>     class Qcow2BitmapTableEntry(Qcow2Struct):
>>>   @@ -214,6 +230,9 @@ class Qcow2BitmapTableEntry(Qcow2Struct):
>>>           else:
>>>               self.type = 'all-zeroes'
>>>   +    def to_dict(self):
>>> +        return dict(type=self.type, offset=self.offset, reserved=self.reserved)
>>> +
>>
>> Python has a special syntax for creating dicts :), let's use:
>>
>> return {'type': self.type, 'offset': self.offset, 'reserved': self.reserved}
>>
>>
>>>     class Qcow2BitmapTable:
>>>   @@ -246,6 +265,9 @@ class QcowHeaderExtension(Qcow2Struct):
>>>               0x44415441: 'Data file'
>>>           }
>>>   +        def to_dict(self):
>>> +            return self.mapping.get(self.value, "<unknown>")
>>
>> aha, so, actually, to_dict() returns not dict, but string. So it should better be named to_json() (and return any json-dumpable object, like string or dict)
>>
>> and then, we can add to_json() method to Qcow2BitmapTable as well, to return list.
> 
> 
> So, should I refactor it now?
> 

Up to you. Better yes if not difficult


> 
> 
>>
>>
>>> +
>>>       fields = (
>>>           ('u32', Magic, 'magic'),
>>>           ('u32', '{}', 'length')
>>> @@ -308,6 +330,18 @@ class QcowHeaderExtension(Qcow2Struct):
>>>           else:
>>>               self.obj.dump()
>>>   +    def to_dict(self):
>>> +        fields_dict = super().to_dict()
>>> +        ext_name = dict(name=self.Magic(self.magic))
>>
>> strange that we have to create Magic object here... Ok, let's refactor it later
>>
>>> +        # Put the name ahead of the dict
>>> +        he_dict = {**ext_name, **fields_dict}
>>> +        if self.obj is not None:
>>> +            he_dict['data'] = self.obj
>>> +        else:
>>> +            he_dict['data_str'] = self.data_str
>>> +
>>> +        return he_dict
>>
>> again, let's avoid extra dict variables:
>>
>> res = {'name': self.Magic(self.magic), **super().to_dict()}
>> if ...
>>
>>
>>> +
>>>       @classmethod
>>>       def create(cls, magic, data):
>>>           return QcowHeaderExtension(magic, len(data), data)
>>>
>>
>>


-- 
Best regards,
Vladimir


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

end of thread, other threads:[~2020-08-06 12:09 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 14:15 [PATCH v12 00/11] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 01/11] iotests: add test for QCOW2 header dump Andrey Shinkevich
2020-07-30 19:05   ` Eric Blake
2020-07-31  7:28     ` Andrey Shinkevich
2020-08-05 11:23   ` Vladimir Sementsov-Ogievskiy
2020-08-05 12:10     ` Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 02/11] qcow2_format.py: make printable data an extension class member Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 04/11] qcow2_format.py: dump bitmap flags in human readable way Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 05/11] qcow2_format.py: Dump bitmap directory information Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 06/11] qcow2_format.py: pass cluster size to substructures Andrey Shinkevich
2020-07-30 14:15 ` [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries Andrey Shinkevich
2020-08-05 15:57   ` Vladimir Sementsov-Ogievskiy
2020-07-30 14:15 ` [PATCH v12 08/11] qcow2.py: Introduce '-j' key to dump in JSON format Andrey Shinkevich
2020-08-05 16:12   ` Vladimir Sementsov-Ogievskiy
2020-07-30 14:15 ` [PATCH v12 09/11] qcow2_format.py: collect fields " Andrey Shinkevich
2020-08-05 16:58   ` Vladimir Sementsov-Ogievskiy
2020-08-06  9:08     ` Andrey Shinkevich
2020-08-06  9:18       ` Vladimir Sementsov-Ogievskiy
2020-07-30 14:15 ` [PATCH v12 10/11] qcow2_format.py: support dumping metadata " Andrey Shinkevich
2020-08-05 17:04   ` Vladimir Sementsov-Ogievskiy
2020-07-30 14:15 ` [PATCH v12 11/11] iotests: dump QCOW2 header in JSON in #303 Andrey Shinkevich
2020-08-05 17:08   ` Vladimir Sementsov-Ogievskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).