* [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
@ 2020-06-01 13:48 ` Andrey Shinkevich
2020-06-02 16:05 ` Eric Blake
2020-06-02 19:25 ` Vladimir Sementsov-Ogievskiy
2020-06-01 13:48 ` [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class Andrey Shinkevich
` (5 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:48 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Header extension: Feature table
magic 0x6803f857
length 192
data <binary>
The change incurs modification of the output in 031, 036 and 061 test
cases.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/031.out | 22 +++++++++++-----------
tests/qemu-iotests/036.out | 4 ++--
tests/qemu-iotests/061.out | 18 +++++++++---------
tests/qemu-iotests/qcow2.py | 23 ++++++++++++++++++++---
4 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
index 5a4beda..966c8d9 100644
--- a/tests/qemu-iotests/031.out
+++ b/tests/qemu-iotests/031.out
@@ -24,7 +24,7 @@ autoclear_features []
refcount_order 4
header_length 72
-Header extension:
+Header extension: Unknown
magic 0x12345678
length 31
data 'This is a test header extension'
@@ -52,7 +52,7 @@ autoclear_features []
refcount_order 4
header_length 72
-Header extension:
+Header extension: Unknown
magic 0x12345678
length 31
data 'This is a test header extension'
@@ -80,12 +80,12 @@ autoclear_features []
refcount_order 4
header_length 72
-Header extension:
+Header extension: Backing format
magic 0xe2792aca
length 11
data 'host_device'
-Header extension:
+Header extension: Unknown
magic 0x12345678
length 31
data 'This is a test header extension'
@@ -115,12 +115,12 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
-Header extension:
+Header extension: Unknown
magic 0x12345678
length 31
data 'This is a test header extension'
@@ -148,12 +148,12 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
-Header extension:
+Header extension: Unknown
magic 0x12345678
length 31
data 'This is a test header extension'
@@ -181,17 +181,17 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Backing format
magic 0xe2792aca
length 11
data 'host_device'
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
-Header extension:
+Header extension: Unknown
magic 0x12345678
length 31
data 'This is a test header extension'
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index e409acf..81a7366 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -24,7 +24,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
incompatible_features []
compatible_features []
autoclear_features [63]
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -36,7 +36,7 @@ No errors were found on the image.
incompatible_features []
compatible_features []
autoclear_features []
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index a51ad1b..7821b7f 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -24,7 +24,7 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -82,7 +82,7 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -138,7 +138,7 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -193,7 +193,7 @@ autoclear_features [42]
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -262,7 +262,7 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -324,7 +324,7 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -353,7 +353,7 @@ autoclear_features []
refcount_order 4
header_length 112
-Header extension:
+Header extension: Feature table
magic 0x6803f857
length 336
data <binary>
@@ -491,7 +491,7 @@ wrote 65536/65536 bytes at offset 2147483648
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 3221225472
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- (0.00/100%)
(12.50/100%)
(25.00/100%)
(37.50/100%)
(50.00/100%)
(62.50/100%)
(75.00/100%)
(87.50/100%)
(100.00/100%)
(100.00/100%)
+ (0.00/100%) (12.50/100%) (25.00/100%) (37.50/100%) (50.00/100%) (62.50/100%) (75.00/100%) (87.50/100%) (100.00/100%) (100.00/100%)
No errors were found on the image.
=== Testing progress report with snapshot ===
@@ -506,7 +506,7 @@ wrote 65536/65536 bytes at offset 2147483648
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 3221225472
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- (0.00/100%)
(6.25/100%)
(12.50/100%)
(18.75/100%)
(25.00/100%)
(31.25/100%)
(37.50/100%)
(43.75/100%)
(50.00/100%)
(56.25/100%)
(62.50/100%)
(68.75/100%)
(75.00/100%)
(81.25/100%)
(87.50/100%)
(93.75/100%)
(100.00/100%)
(100.00/100%)
+ (0.00/100%) (6.25/100%) (12.50/100%) (18.75/100%) (25.00/100%) (31.25/100%) (37.50/100%) (43.75/100%) (50.00/100%) (56.25/100%) (62.50/100%) (68.75/100%) (75.00/100%) (81.25/100%) (87.50/100%) (93.75/100%) (100.00/100%) (100.00/100%)
No errors were found on the image.
=== Testing version downgrade with external data file ===
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 94a07b2..e824b09 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -6,19 +6,36 @@ import string
class QcowHeaderExtension:
+ QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
+ QCOW2_EXT_MAGIC_FEATURE_TABLE = 0x6803f857
+ QCOW2_EXT_MAGIC_CRYPTO_HEADER = 0x0537be77
+ QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
+ QCOW2_EXT_MAGIC_DATA_FILE = 0x44415441
+
def __init__(self, magic, length, data):
if length % 8 != 0:
padding = 8 - (length % 8)
data += b"\0" * padding
- self.magic = magic
+ self.magic = magic
self.length = length
- self.data = data
+ self.data = data
+ self.name = self.extension_name(magic)
@classmethod
def create(cls, magic, data):
return QcowHeaderExtension(magic, len(data), data)
+ def extension_name(self, magic):
+ return {
+ self.QCOW2_EXT_MAGIC_BACKING_FORMAT: 'Backing format',
+ self.QCOW2_EXT_MAGIC_FEATURE_TABLE: 'Feature table',
+ self.QCOW2_EXT_MAGIC_CRYPTO_HEADER: 'Crypto header',
+ self.QCOW2_EXT_MAGIC_BITMAPS: 'Bitmaps',
+ self.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
+ }.get(magic, 'Unknown')
+
+
class QcowHeader:
uint32_t = 'I'
@@ -151,7 +168,7 @@ class QcowHeader:
else:
data = "<binary>"
- print("Header extension:")
+ print("%-25s %s" % ("Header extension:", ex.name))
print("%-25s %#x" % ("magic", ex.magic))
print("%-25s %d" % ("length", ex.length))
print("%-25s %s" % ("data", data))
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump
2020-06-01 13:48 ` [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump Andrey Shinkevich
@ 2020-06-02 16:05 ` Eric Blake
2020-06-02 16:07 ` Eric Blake
2020-06-02 19:25 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 22+ messages in thread
From: Eric Blake @ 2020-06-02 16:05 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Header extension: Feature table
> magic 0x6803f857
> length 192
> data <binary>
>
> The change incurs modification of the output in 031, 036 and 061 test
> cases.
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/031.out | 22 +++++++++++-----------
> tests/qemu-iotests/036.out | 4 ++--
> tests/qemu-iotests/061.out | 18 +++++++++---------
> tests/qemu-iotests/qcow2.py | 23 ++++++++++++++++++++---
> 4 files changed, 42 insertions(+), 25 deletions(-)
>
> +++ b/tests/qemu-iotests/061.out
> @@ -491,7 +491,7 @@ wrote 65536/65536 bytes at offset 2147483648
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> wrote 65536/65536 bytes at offset 3221225472
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> - (0.00/100%)
> (12.50/100%)
> (25.00/100%)
> (37.50/100%)
> (50.00/100%)
> (62.50/100%)
> (75.00/100%)
> (87.50/100%)
> (100.00/100%)
> (100.00/100%)
> + (0.00/100%) (12.50/100%) (25.00/100%) (37.50/100%) (50.00/100%) (62.50/100%) (75.00/100%) (87.50/100%) (100.00/100%) (100.00/100%)
> No errors were found on the image.
This hunk looks odd. But it is not necessarily wrong. Occurs a couple
of times.
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -6,19 +6,36 @@ import string
>
> class QcowHeaderExtension:
>
> + QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
> + QCOW2_EXT_MAGIC_FEATURE_TABLE = 0x6803f857
Why the inconsistency between capitalization in these constants?
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] 22+ messages in thread
* Re: [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump
2020-06-02 16:05 ` Eric Blake
@ 2020-06-02 16:07 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-02 16:07 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/2/20 11:05 AM, Eric Blake wrote:
[I hit send too soon...]
>> +++ b/tests/qemu-iotests/qcow2.py
>> @@ -6,19 +6,36 @@ import string
>> class QcowHeaderExtension:
>> + QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
>> + QCOW2_EXT_MAGIC_FEATURE_TABLE = 0x6803f857
>
> Why the inconsistency between capitalization in these constants?
At least it matches the inconsistency in docs/interop/qcow2.txt; a patch
to fix that wouldn't hurt.
>
> 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] 22+ messages in thread
* Re: [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump
2020-06-01 13:48 ` [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump Andrey Shinkevich
2020-06-02 16:05 ` Eric Blake
@ 2020-06-02 19:25 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-02 19:25 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
01.06.2020 16:48, Andrey Shinkevich wrote:
> Header extension: Feature table
> magic 0x6803f857
> length 192
> data <binary>
>
> The change incurs modification of the output in 031, 036 and 061 test
> cases.
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/031.out | 22 +++++++++++-----------
> tests/qemu-iotests/036.out | 4 ++--
> tests/qemu-iotests/061.out | 18 +++++++++---------
> tests/qemu-iotests/qcow2.py | 23 ++++++++++++++++++++---
> 4 files changed, 42 insertions(+), 25 deletions(-)
>
> diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
> index 5a4beda..966c8d9 100644
> --- a/tests/qemu-iotests/031.out
> +++ b/tests/qemu-iotests/031.out
> @@ -24,7 +24,7 @@ autoclear_features []
> refcount_order 4
> header_length 72
>
> -Header extension:
> +Header extension: Unknown
> magic 0x12345678
> length 31
> data 'This is a test header extension'
> @@ -52,7 +52,7 @@ autoclear_features []
> refcount_order 4
> header_length 72
>
> -Header extension:
> +Header extension: Unknown
> magic 0x12345678
> length 31
> data 'This is a test header extension'
> @@ -80,12 +80,12 @@ autoclear_features []
> refcount_order 4
> header_length 72
>
> -Header extension:
> +Header extension: Backing format
> magic 0xe2792aca
> length 11
> data 'host_device'
>
> -Header extension:
> +Header extension: Unknown
> magic 0x12345678
> length 31
> data 'This is a test header extension'
> @@ -115,12 +115,12 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
>
> -Header extension:
> +Header extension: Unknown
> magic 0x12345678
> length 31
> data 'This is a test header extension'
> @@ -148,12 +148,12 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
>
> -Header extension:
> +Header extension: Unknown
> magic 0x12345678
> length 31
> data 'This is a test header extension'
> @@ -181,17 +181,17 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Backing format
> magic 0xe2792aca
> length 11
> data 'host_device'
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
>
> -Header extension:
> +Header extension: Unknown
> magic 0x12345678
> length 31
> data 'This is a test header extension'
> diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
> index e409acf..81a7366 100644
> --- a/tests/qemu-iotests/036.out
> +++ b/tests/qemu-iotests/036.out
> @@ -24,7 +24,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> incompatible_features []
> compatible_features []
> autoclear_features [63]
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -36,7 +36,7 @@ No errors were found on the image.
> incompatible_features []
> compatible_features []
> autoclear_features []
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
> index a51ad1b..7821b7f 100644
> --- a/tests/qemu-iotests/061.out
> +++ b/tests/qemu-iotests/061.out
> @@ -24,7 +24,7 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -82,7 +82,7 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -138,7 +138,7 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -193,7 +193,7 @@ autoclear_features [42]
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -262,7 +262,7 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -324,7 +324,7 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -353,7 +353,7 @@ autoclear_features []
> refcount_order 4
> header_length 112
>
> -Header extension:
> +Header extension: Feature table
> magic 0x6803f857
> length 336
> data <binary>
> @@ -491,7 +491,7 @@ wrote 65536/65536 bytes at offset 2147483648
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> wrote 65536/65536 bytes at offset 3221225472
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> - (0.00/100%)
> (12.50/100%)
> (25.00/100%)
> (37.50/100%)
> (50.00/100%)
> (62.50/100%)
> (75.00/100%)
> (87.50/100%)
> (100.00/100%)
> (100.00/100%)
> + (0.00/100%) (12.50/100%) (25.00/100%) (37.50/100%) (50.00/100%) (62.50/100%) (75.00/100%) (87.50/100%) (100.00/100%) (100.00/100%)
> No errors were found on the image.
>
> === Testing progress report with snapshot ===
> @@ -506,7 +506,7 @@ wrote 65536/65536 bytes at offset 2147483648
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> wrote 65536/65536 bytes at offset 3221225472
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> - (0.00/100%)
> (6.25/100%)
> (12.50/100%)
> (18.75/100%)
> (25.00/100%)
> (31.25/100%)
> (37.50/100%)
> (43.75/100%)
> (50.00/100%)
> (56.25/100%)
> (62.50/100%)
> (68.75/100%)
> (75.00/100%)
> (81.25/100%)
> (87.50/100%)
> (93.75/100%)
> (100.00/100%)
> (100.00/100%)
> + (0.00/100%) (6.25/100%) (12.50/100%) (18.75/100%) (25.00/100%) (31.25/100%) (37.50/100%) (43.75/100%) (50.00/100%) (56.25/100%) (62.50/100%) (68.75/100%) (75.00/100%) (81.25/100%) (87.50/100%) (93.75/100%) (100.00/100%) (100.00/100%)
> No errors were found on the image.
>
> === Testing version downgrade with external data file ===
^^^ these two chunks are some mistake, I even can't apply the patch with them. And with them dropped, tests pass for me. [*]
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 94a07b2..e824b09 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -6,19 +6,36 @@ import string
>
while being here, may add one more newline for PEP8
> class QcowHeaderExtension:
>
> + QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
> + QCOW2_EXT_MAGIC_FEATURE_TABLE = 0x6803f857
> + QCOW2_EXT_MAGIC_CRYPTO_HEADER = 0x0537be77
> + QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
> + QCOW2_EXT_MAGIC_DATA_FILE = 0x44415441
> +
> def __init__(self, magic, length, data):
> if length % 8 != 0:
> padding = 8 - (length % 8)
> data += b"\0" * padding
>
> - self.magic = magic
> + self.magic = magic
> self.length = length
> - self.data = data
> + self.data = data
> + self.name = self.extension_name(magic)
>
> @classmethod
> def create(cls, magic, data):
> return QcowHeaderExtension(magic, len(data), data)
>
> + def extension_name(self, magic):
let this be classmethod
> + return {
> + self.QCOW2_EXT_MAGIC_BACKING_FORMAT: 'Backing format',
> + self.QCOW2_EXT_MAGIC_FEATURE_TABLE: 'Feature table',
> + self.QCOW2_EXT_MAGIC_CRYPTO_HEADER: 'Crypto header',
> + self.QCOW2_EXT_MAGIC_BITMAPS: 'Bitmaps',
> + self.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
> + }.get(magic, 'Unknown')
> +
> +
> class QcowHeader:
>
> uint32_t = 'I'
> @@ -151,7 +168,7 @@ class QcowHeader:
> else:
> data = "<binary>"
>
> - print("Header extension:")
> + print("%-25s %s" % ("Header extension:", ex.name))
> print("%-25s %#x" % ("magic", ex.magic))
> print("%-25s %d" % ("length", ex.length))
> print("%-25s %s" % ("data", data))
>
With dropped two strange [*] hunks, and with (or without) the following diff additionally applied:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
recommended additional change:
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -4,6 +4,7 @@ import sys
import struct
import string
+
class QcowHeaderExtension:
QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
@@ -26,13 +27,14 @@ class QcowHeaderExtension:
def create(cls, magic, data):
return QcowHeaderExtension(magic, len(data), data)
- def extension_name(self, magic):
+ @classmethod
+ def extension_name(cls, magic):
return {
- self.QCOW2_EXT_MAGIC_BACKING_FORMAT: 'Backing format',
- self.QCOW2_EXT_MAGIC_FEATURE_TABLE: 'Feature table',
- self.QCOW2_EXT_MAGIC_CRYPTO_HEADER: 'Crypto header',
- self.QCOW2_EXT_MAGIC_BITMAPS: 'Bitmaps',
- self.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
+ cls.QCOW2_EXT_MAGIC_BACKING_FORMAT: 'Backing format',
+ cls.QCOW2_EXT_MAGIC_FEATURE_TABLE: 'Feature table',
+ cls.QCOW2_EXT_MAGIC_CRYPTO_HEADER: 'Crypto header',
+ cls.QCOW2_EXT_MAGIC_BITMAPS: 'Bitmaps',
+ cls.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
}.get(magic, 'Unknown')
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
2020-06-01 13:48 ` [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump Andrey Shinkevich
@ 2020-06-01 13:48 ` Andrey Shinkevich
2020-06-02 16:14 ` Eric Blake
2020-06-02 19:32 ` Vladimir Sementsov-Ogievskiy
2020-06-01 13:48 ` [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py Andrey Shinkevich
` (4 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:48 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
in the script qcow2.py.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index e824b09..18e4923 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -13,6 +13,12 @@ class QcowHeaderExtension:
QCOW2_EXT_MAGIC_DATA_FILE = 0x44415441
def __init__(self, magic, length, data):
+ data_str = data[:length]
+ if all(c in string.printable.encode('ascii') for c in data_str):
+ data_str = "'%s'" % data_str.decode('ascii')
+ else:
+ data_str = "<binary>"
+
if length % 8 != 0:
padding = 8 - (length % 8)
data += b"\0" * padding
@@ -21,6 +27,7 @@ class QcowHeaderExtension:
self.length = length
self.data = data
self.name = self.extension_name(magic)
+ self.data_str = data_str
@classmethod
def create(cls, magic, data):
@@ -162,16 +169,10 @@ class QcowHeader:
def dump_extensions(self):
for ex in self.extensions:
- data = ex.data[:ex.length]
- if all(c in string.printable.encode('ascii') for c in data):
- data = "'%s'" % data.decode('ascii')
- else:
- data = "<binary>"
-
print("%-25s %s" % ("Header extension:", ex.name))
print("%-25s %#x" % ("magic", ex.magic))
print("%-25s %d" % ("length", ex.length))
- print("%-25s %s" % ("data", data))
+ print("%-25s %s" % ("data", ex.data_str))
print("")
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class
2020-06-01 13:48 ` [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class Andrey Shinkevich
@ 2020-06-02 16:14 ` Eric Blake
2020-06-02 19:32 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-02 16:14 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, 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
> in the script qcow2.py.
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
Sane code motion.
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] 22+ messages in thread
* Re: [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class
2020-06-01 13:48 ` [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class Andrey Shinkevich
2020-06-02 16:14 ` Eric Blake
@ 2020-06-02 19:32 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-02 19:32 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
01.06.2020 16:48, 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
> in the script qcow2.py.
>
> 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] 22+ messages in thread
* [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
2020-06-01 13:48 ` [PATCH v3 1/6] iotests: Add extension names to qcow2.py dump Andrey Shinkevich
2020-06-01 13:48 ` [PATCH v3 2/6] iotests: move check for printable data to QcowHeaderExtension class Andrey Shinkevich
@ 2020-06-01 13:48 ` Andrey Shinkevich
2020-06-02 16:16 ` Eric Blake
2020-06-02 20:10 ` Vladimir Sementsov-Ogievskiy
2020-06-01 13:48 ` [PATCH v3 4/6] iotests: Dump bitmap directory info " Andrey Shinkevich
` (3 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:48 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Add bitmap header extension data, if any, to the dump in qcow2.py.
Header extension: Bitmaps
magic 0x23852875
length 24
nb_bitmaps 2
reserved32 0
bitmap_directory_size 0x40
bitmap_directory_offset 0x100000
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 18e4923..8286115 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -4,6 +4,39 @@ import sys
import struct
import string
+
+class Qcow2BitmapExt:
+
+ uint32_t = 'I'
+ uint64_t = 'Q'
+
+ fields = [
+ [uint32_t, '%d', 'nb_bitmaps'],
+ [uint32_t, '%d', 'reserved32'],
+ [uint64_t, '%#x', 'bitmap_directory_size'],
+ [uint64_t, '%#x', 'bitmap_directory_offset']
+ ]
+
+ fmt = '>' + ''.join(field[0] for field in fields)
+
+ def __init__(self, data):
+
+ extension = struct.unpack(Qcow2BitmapExt.fmt, data)
+ self.__dict__ = dict((field[2], extension[i])
+ for i, field in enumerate(Qcow2BitmapExt.fields))
+
+ def dump_bitmap_ext(self):
+ for f in Qcow2BitmapExt.fields:
+ value = self.__dict__[f[2]]
+ value_str = f[1] % value
+
+ print("%-25s" % f[2], value_str)
+ print("")
+
+ def dump_ext(self):
+ self.dump_bitmap_ext()
+
+
class QcowHeaderExtension:
QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
@@ -13,12 +46,16 @@ class QcowHeaderExtension:
QCOW2_EXT_MAGIC_DATA_FILE = 0x44415441
def __init__(self, magic, length, data):
+ self.obj = None
data_str = data[:length]
if all(c in string.printable.encode('ascii') for c in data_str):
data_str = "'%s'" % data_str.decode('ascii')
else:
data_str = "<binary>"
+ if magic == self.QCOW2_EXT_MAGIC_BITMAPS:
+ self.obj = Qcow2BitmapExt(data)
+
if length % 8 != 0:
padding = 8 - (length % 8)
data += b"\0" * padding
@@ -172,7 +209,10 @@ class QcowHeader:
print("%-25s %s" % ("Header extension:", ex.name))
print("%-25s %#x" % ("magic", ex.magic))
print("%-25s %d" % ("length", ex.length))
- print("%-25s %s" % ("data", ex.data_str))
+ if ex.obj is not None:
+ ex.obj.dump_ext()
+ else:
+ print("%-25s %s" % ("data", ex.data_str))
print("")
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py
2020-06-01 13:48 ` [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py Andrey Shinkevich
@ 2020-06-02 16:16 ` Eric Blake
2020-06-02 20:10 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-02 16:16 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Add bitmap header extension data, if any, to the dump in qcow2.py.
>
> Header extension: Bitmaps
> magic 0x23852875
> length 24
> nb_bitmaps 2
> reserved32 0
> bitmap_directory_size 0x40
> bitmap_directory_offset 0x100000
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 42 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
Nice improvement.
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] 22+ messages in thread
* Re: [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py
2020-06-01 13:48 ` [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py Andrey Shinkevich
2020-06-02 16:16 ` Eric Blake
@ 2020-06-02 20:10 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-02 20:10 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
01.06.2020 16:48, Andrey Shinkevich wrote:
> Add bitmap header extension data, if any, to the dump in qcow2.py.
>
> Header extension: Bitmaps
> magic 0x23852875
> length 24
> nb_bitmaps 2
> reserved32 0
> bitmap_directory_size 0x40
> bitmap_directory_offset 0x100000
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 42 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 18e4923..8286115 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -4,6 +4,39 @@ import sys
> import struct
> import string
>
> +
> +class Qcow2BitmapExt:
> +
> + uint32_t = 'I'
> + uint64_t = 'Q'
> +
> + fields = [
> + [uint32_t, '%d', 'nb_bitmaps'],
> + [uint32_t, '%d', 'reserved32'],
> + [uint64_t, '%#x', 'bitmap_directory_size'],
> + [uint64_t, '%#x', 'bitmap_directory_offset']
> + ]
> +
> + fmt = '>' + ''.join(field[0] for field in fields)
> +
> + def __init__(self, data):
> +
> + extension = struct.unpack(Qcow2BitmapExt.fmt, data)
> + self.__dict__ = dict((field[2], extension[i])
> + for i, field in enumerate(Qcow2BitmapExt.fields))
> +
> + def dump_bitmap_ext(self):
> + for f in Qcow2BitmapExt.fields:
> + value = self.__dict__[f[2]]
> + value_str = f[1] % value
> +
> + print("%-25s" % f[2], value_str)
> + print("")
> +
> + def dump_ext(self):
> + self.dump_bitmap_ext()
Hmm, don't see, why this can't be one function named "dump".
Still, I do think it should be moved to parent class of both Qcow2BitmapExt and QcowHeader,
I'll send refactoring follow-up, so it doesn't really matter now.
> +
> +
> class QcowHeaderExtension:
>
> QCOW2_EXT_MAGIC_BACKING_FORMAT = 0xE2792ACA
> @@ -13,12 +46,16 @@ class QcowHeaderExtension:
> QCOW2_EXT_MAGIC_DATA_FILE = 0x44415441
>
> def __init__(self, magic, length, data):
> + self.obj = None
> data_str = data[:length]
> if all(c in string.printable.encode('ascii') for c in data_str):
> data_str = "'%s'" % data_str.decode('ascii')
> else:
> data_str = "<binary>"
>
> + if magic == self.QCOW2_EXT_MAGIC_BITMAPS:
> + self.obj = Qcow2BitmapExt(data)
> +
> if length % 8 != 0:
> padding = 8 - (length % 8)
> data += b"\0" * padding
> @@ -172,7 +209,10 @@ class QcowHeader:
> print("%-25s %s" % ("Header extension:", ex.name))
> print("%-25s %#x" % ("magic", ex.magic))
> print("%-25s %d" % ("length", ex.length))
> - print("%-25s %s" % ("data", ex.data_str))
> + if ex.obj is not None:
> + ex.obj.dump_ext()
> + else:
> + print("%-25s %s" % ("data", ex.data_str))
this change make it more obvious that this should be method of QcowHeaderExtension.
Still, this works as is, nothing serious:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 4/6] iotests: Dump bitmap directory info with qcow2.py
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
` (2 preceding siblings ...)
2020-06-01 13:48 ` [PATCH v3 3/6] iotests: dump bitmap extension data with qcow2.py Andrey Shinkevich
@ 2020-06-01 13:48 ` Andrey Shinkevich
2020-06-02 17:35 ` Eric Blake
2020-06-02 21:15 ` Vladimir Sementsov-Ogievskiy
2020-06-01 13:48 ` [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image Andrey Shinkevich
` (2 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:48 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 with the
script qcow2.py.
Header extension: Bitmaps
...
Bitmap name bitmap-1
flag auto
bitmap_table_offset 0xf0000
bitmap_table_size 8
flag_bits 2
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>
---
tests/qemu-iotests/qcow2.py | 104 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 103 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 8286115..e4453f6 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -5,6 +5,88 @@ import struct
import string
+class Qcow2BitmapDirEntry:
+
+ name = ''
+
+ uint8_t = 'B'
+ uint16_t = 'H'
+ uint32_t = 'I'
+ uint64_t = 'Q'
+
+ fields = [
+ [uint64_t, '%#x', 'bitmap_table_offset'],
+ [uint32_t, '%d', 'bitmap_table_size'],
+ [uint32_t, '%d', 'flag_bits'],
+ [uint8_t, '%d', 'type'],
+ [uint8_t, '%d', 'granularity_bits'],
+ [uint16_t, '%d', 'name_size'],
+ [uint32_t, '%d', 'extra_data_size']
+ ]
+
+ fmt = '>' + ''.join(field[0] for field in fields)
+
+ def __init__(self, data):
+
+ entry = struct.unpack(Qcow2BitmapDirEntry.fmt, data)
+ self.__dict__ = dict((field[2], entry[i])
+ for i, field in enumerate(
+ Qcow2BitmapDirEntry.fields))
+
+ self.bitmap_table_size = self.bitmap_table_size \
+ * struct.calcsize(self.uint64_t)
+
+ self.bitmap_flags = []
+ BME_FLAG_IN_USE = 1
+ BME_FLAG_AUTO = 1 << 1
+ if (self.flag_bits & BME_FLAG_IN_USE) != 0:
+ self.bitmap_flags.append("in-use")
+ if (self.flag_bits & BME_FLAG_AUTO) != 0:
+ 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("%-25s" % 'Bitmap name', self.name)
+
+ for fl in self.bitmap_flags:
+ print("%-25s" % 'flag', fl)
+
+ for f in Qcow2BitmapDirEntry.fields:
+ value = self.__dict__[f[2]]
+ value_str = f[1] % value
+ print("%-25s" % f[2], value_str)
+
+
+class Qcow2BitmapDirectory:
+
+ def __init__(self, bm_header_ext):
+ self.nb_bitmaps = bm_header_ext.nb_bitmaps
+ self.bitmap_directory_offset = bm_header_ext.bitmap_directory_offset
+ self.bitmap_directory_size = bm_header_ext.bitmap_directory_size
+
+ 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(buf)
+ fd.seek(dir_entry.extra_data_size, 1)
+ 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, 1)
+
+ def get_bitmaps(self):
+ return self.bitmaps
+
+
class Qcow2BitmapExt:
uint32_t = 'I'
@@ -33,8 +115,21 @@ class Qcow2BitmapExt:
print("%-25s" % f[2], value_str)
print("")
+ def read_bitmap_directory(self, fd):
+ bm_directory = Qcow2BitmapDirectory(self)
+ bm_directory.read_bitmap_directory(fd)
+ self.bitmaps = bm_directory.get_bitmaps()
+
+ def load(self, fd):
+ self.read_bitmap_directory(fd)
+
+ def dump_bitmap_directory(self):
+ for bm in self.bitmaps:
+ bm.dump_bitmap_dir_entry()
+
def dump_ext(self):
self.dump_bitmap_ext()
+ self.dump_bitmap_directory()
class QcowHeaderExtension:
@@ -79,6 +174,10 @@ class QcowHeaderExtension:
self.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
}.get(magic, 'Unknown')
+ def load(self, fd):
+ if self.obj is not None:
+ self.obj.load(fd)
+
class QcowHeader:
@@ -157,7 +256,10 @@ class QcowHeader:
else:
padded = (length + 7) & ~7
data = fd.read(padded)
- self.extensions.append(QcowHeaderExtension(magic, length, data))
+ self.extensions.append(QcowHeaderExtension(magic, length,
+ data))
+ for ex in self.extensions:
+ ex.load(fd)
def update_extensions(self, fd):
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 4/6] iotests: Dump bitmap directory info with qcow2.py
2020-06-01 13:48 ` [PATCH v3 4/6] iotests: Dump bitmap directory info " Andrey Shinkevich
@ 2020-06-02 17:35 ` Eric Blake
2020-06-02 21:15 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-02 17:35 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Read and dump entries from the bitmap directory of QCOW2 image with the
> script qcow2.py.
>
> Header extension: Bitmaps
> ...
> Bitmap name bitmap-1
> flag auto
> bitmap_table_offset 0xf0000
> bitmap_table_size 8
> flag_bits 2
> 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>
> ---
> tests/qemu-iotests/qcow2.py | 104 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 103 insertions(+), 1 deletion(-)
>
> + self.bitmap_flags = []
> + BME_FLAG_IN_USE = 1
> + BME_FLAG_AUTO = 1 << 1
Would it be worth using '1 << 0' for BME_FLAG_IN_USE, to make it obvious
that these are consecutive bits, especially if we later add a third bit?
> + for n in range(self.nb_bitmaps):
> + buf = fd.read(buf_size)
> + dir_entry = Qcow2BitmapDirEntry(buf)
> + fd.seek(dir_entry.extra_data_size, 1)
> + 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, 1)
Is there a symbolic constant instead of the magic '1' for fd.seek()?
> @@ -157,7 +256,10 @@ class QcowHeader:
> else:
> padded = (length + 7) & ~7
> data = fd.read(padded)
> - self.extensions.append(QcowHeaderExtension(magic, length, data))
> + self.extensions.append(QcowHeaderExtension(magic, length,
> + data))
Should this reformatting be done earlier in the series to minimize churn?
> + for ex in self.extensions:
> + ex.load(fd)
>
> def update_extensions(self, fd):
>
>
Fixing the things I pointed out does not seem major, so
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] 22+ messages in thread* Re: [PATCH v3 4/6] iotests: Dump bitmap directory info with qcow2.py
2020-06-01 13:48 ` [PATCH v3 4/6] iotests: Dump bitmap directory info " Andrey Shinkevich
2020-06-02 17:35 ` Eric Blake
@ 2020-06-02 21:15 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-02 21:15 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
01.06.2020 16:48, Andrey Shinkevich wrote:
> Read and dump entries from the bitmap directory of QCOW2 image with the
> script qcow2.py.
>
> Header extension: Bitmaps
> ...
> Bitmap name bitmap-1
> flag auto
> bitmap_table_offset 0xf0000
> bitmap_table_size 8
> flag_bits 2
> 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>
> ---
> tests/qemu-iotests/qcow2.py | 104 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 8286115..e4453f6 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -5,6 +5,88 @@ import struct
> import string
>
>
> +class Qcow2BitmapDirEntry:
> +
> + name = ''
> +
> + uint8_t = 'B'
> + uint16_t = 'H'
> + uint32_t = 'I'
> + uint64_t = 'Q'
> +
> + fields = [
> + [uint64_t, '%#x', 'bitmap_table_offset'],
> + [uint32_t, '%d', 'bitmap_table_size'],
> + [uint32_t, '%d', 'flag_bits'],
I'd keep exact field name from spec: "flags"
> + [uint8_t, '%d', 'type'],
> + [uint8_t, '%d', 'granularity_bits'],
> + [uint16_t, '%d', 'name_size'],
> + [uint32_t, '%d', 'extra_data_size']
> + ]
> +
> + fmt = '>' + ''.join(field[0] for field in fields)
> +
> + def __init__(self, data):
> +
You are not consistent about adding this empty line. I'd avoid it.
> + entry = struct.unpack(Qcow2BitmapDirEntry.fmt, data)
> + self.__dict__ = dict((field[2], entry[i])
> + for i, field in enumerate(
> + Qcow2BitmapDirEntry.fields))
> +
> + self.bitmap_table_size = self.bitmap_table_size \
> + * struct.calcsize(self.uint64_t)
I don't like this: you keep name from specification, but change its
meaning.
> +
> + self.bitmap_flags = []
> + BME_FLAG_IN_USE = 1
> + BME_FLAG_AUTO = 1 << 1
I'd define all constants copied from C code in global space, to be simply
available from the module.
> + if (self.flag_bits & BME_FLAG_IN_USE) != 0:
in python zero is false enough :) (you may omit comparison to 0)
> + self.bitmap_flags.append("in-use")
> + if (self.flag_bits & BME_FLAG_AUTO) != 0:
> + 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("%-25s" % 'Bitmap name', self.name)
> +
> + for fl in self.bitmap_flags:
> + print("%-25s" % 'flag', fl)
> +
> + for f in Qcow2BitmapDirEntry.fields:
> + value = self.__dict__[f[2]]
> + value_str = f[1] % value
> + print("%-25s" % f[2], value_str)
> +
> +
> +class Qcow2BitmapDirectory:
> +
> + def __init__(self, bm_header_ext):
> + self.nb_bitmaps = bm_header_ext.nb_bitmaps
> + self.bitmap_directory_offset = bm_header_ext.bitmap_directory_offset
> + self.bitmap_directory_size = bm_header_ext.bitmap_directory_size
Honestly, I don't like duplicating attributes between different objects.
> +
> + def read_bitmap_directory(self, fd):
Why not do it in constructor? The only use of the class
> + 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(buf)
> + fd.seek(dir_entry.extra_data_size, 1)
> + 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, 1)
> +
> + def get_bitmaps(self):
> + return self.bitmaps
Why we need getter for this field?
> +
> +
> class Qcow2BitmapExt:
>
> uint32_t = 'I'
> @@ -33,8 +115,21 @@ class Qcow2BitmapExt:
> print("%-25s" % f[2], value_str)
> print("")
>
> + def read_bitmap_directory(self, fd):
> + bm_directory = Qcow2BitmapDirectory(self)
> + bm_directory.read_bitmap_directory(fd)
> + self.bitmaps = bm_directory.get_bitmaps()
> +
> + def load(self, fd):
> + self.read_bitmap_directory(fd)
> +
> + def dump_bitmap_directory(self):
> + for bm in self.bitmaps:
> + bm.dump_bitmap_dir_entry()
> +
> def dump_ext(self):
> self.dump_bitmap_ext()
> + self.dump_bitmap_directory()
>
>
> class QcowHeaderExtension:
> @@ -79,6 +174,10 @@ class QcowHeaderExtension:
> self.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
> }.get(magic, 'Unknown')
>
> + def load(self, fd):
> + if self.obj is not None:
> + self.obj.load(fd)
> +
>
> class QcowHeader:
>
> @@ -157,7 +256,10 @@ class QcowHeader:
> else:
> padded = (length + 7) & ~7
> data = fd.read(padded)
> - self.extensions.append(QcowHeaderExtension(magic, length, data))
> + self.extensions.append(QcowHeaderExtension(magic, length,
> + data))
> + for ex in self.extensions:
> + ex.load(fd)
>
> def update_extensions(self, fd):
>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
` (3 preceding siblings ...)
2020-06-01 13:48 ` [PATCH v3 4/6] iotests: Dump bitmap directory info " Andrey Shinkevich
@ 2020-06-01 13:48 ` Andrey Shinkevich
2020-06-02 17:38 ` Eric Blake
2020-06-02 21:26 ` Vladimir Sementsov-Ogievskiy
2020-06-01 13:48 ` [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py Andrey Shinkevich
2020-06-01 21:46 ` [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Eric Blake
6 siblings, 2 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:48 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Add bitmap table info to the QCOW2 metadata dump with qcow2.py.
Bitmap name bitmap-1
...
itmap table type offset size
0 serialized 0xa0000 65536
1 all-zeroes 0x0 65536
2 all-zeroes 0x0 65536
3 all-zeroes 0x0 65536
4 all-zeroes 0x0 65536
5 all-zeroes 0x0 65536
6 all-zeroes 0x0 65536
7 all-zeroes 0x0 65536
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 48 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index e4453f6..76e0c69 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -5,6 +5,41 @@ import struct
import string
+cluster_size = 0
+
+
+class Qcow2BitmapTableEntry:
+
+ BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
+ BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
+ bmte_type = ['all-zeroes', 'all-ones', 'serialized']
+
+ def __init__(self, entry):
+ self.cluster_size = cluster_size
+ self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
+ if self.offset != 0:
+ index = 2
+ else:
+ index = entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES
+ self.type = self.bmte_type[index]
+
+
+class Qcow2BitmapTable:
+
+ def __init__(self, raw_table):
+ self.entries = []
+ for entry in raw_table:
+ self.entries.append(Qcow2BitmapTableEntry(entry))
+
+ def print_bitmap_table(self):
+ bitmap_table = enumerate(self.entries)
+ print("Bitmap table\ttype\t\toffset\t\tsize")
+ for i, entry in bitmap_table:
+ print("\t%-4d\t%s\t%#x\t\t%d" % (i, entry.type, entry.offset,
+ entry.cluster_size))
+ print("")
+
+
class Qcow2BitmapDirEntry:
name = ''
@@ -48,6 +83,12 @@ class Qcow2BitmapDirEntry:
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_size * struct.calcsize(self.uint64_t)
+ 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("%-25s" % 'Bitmap name', self.name)
@@ -59,6 +100,8 @@ class Qcow2BitmapDirEntry:
value_str = f[1] % value
print("%-25s" % f[2], value_str)
+ self.bitmap_table.print_bitmap_table()
+
class Qcow2BitmapDirectory:
@@ -83,6 +126,9 @@ class Qcow2BitmapDirectory:
shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
fd.seek(shift, 1)
+ for bm in self.bitmaps:
+ bm.read_bitmap_table(fd)
+
def get_bitmaps(self):
return self.bitmaps
@@ -223,6 +269,8 @@ class QcowHeader:
self.set_defaults()
self.cluster_size = 1 << self.cluster_bits
+ global cluster_size
+ cluster_size = self.cluster_size
fd.seek(self.header_length)
self.load_extensions(fd)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image
2020-06-01 13:48 ` [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image Andrey Shinkevich
@ 2020-06-02 17:38 ` Eric Blake
2020-06-02 21:26 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-02 17:38 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Add bitmap table info to the QCOW2 metadata dump with qcow2.py.
>
> Bitmap name bitmap-1
> ...
> itmap table type offset size
Missed a character from the paste
> 0 serialized 0xa0000 65536
> 1 all-zeroes 0x0 65536
> 2 all-zeroes 0x0 65536
> 3 all-zeroes 0x0 65536
> 4 all-zeroes 0x0 65536
> 5 all-zeroes 0x0 65536
> 6 all-zeroes 0x0 65536
> 7 all-zeroes 0x0 65536
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 48 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
Otherwise looks fine.
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] 22+ messages in thread
* Re: [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image
2020-06-01 13:48 ` [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image Andrey Shinkevich
2020-06-02 17:38 ` Eric Blake
@ 2020-06-02 21:26 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-02 21:26 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
01.06.2020 16:48, Andrey Shinkevich wrote:
> Add bitmap table info to the QCOW2 metadata dump with qcow2.py.
>
> Bitmap name bitmap-1
> ...
> itmap table type offset size
> 0 serialized 0xa0000 65536
> 1 all-zeroes 0x0 65536
> 2 all-zeroes 0x0 65536
> 3 all-zeroes 0x0 65536
> 4 all-zeroes 0x0 65536
> 5 all-zeroes 0x0 65536
> 6 all-zeroes 0x0 65536
> 7 all-zeroes 0x0 65536
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 48 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index e4453f6..76e0c69 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -5,6 +5,41 @@ import struct
> import string
>
>
> +cluster_size = 0
> +
> +
> +class Qcow2BitmapTableEntry:
> +
> + BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
> + BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
> + bmte_type = ['all-zeroes', 'all-ones', 'serialized']
> +
> + def __init__(self, entry):
> + self.cluster_size = cluster_size
> + self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
> + if self.offset != 0:
> + index = 2
> + else:
> + index = entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES
> + self.type = self.bmte_type[index]
IMHO, it would be clearer without extra list layer:
if self.offset != 0:
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):
> + bitmap_table = enumerate(self.entries)
> + print("Bitmap table\ttype\t\toffset\t\tsize")
> + for i, entry in bitmap_table:
> + print("\t%-4d\t%s\t%#x\t\t%d" % (i, entry.type, entry.offset,
> + entry.cluster_size))
> + print("")
> +
> +
> class Qcow2BitmapDirEntry:
>
> name = ''
> @@ -48,6 +83,12 @@ class Qcow2BitmapDirEntry:
> 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_size * struct.calcsize(self.uint64_t)
> + 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("%-25s" % 'Bitmap name', self.name)
>
> @@ -59,6 +100,8 @@ class Qcow2BitmapDirEntry:
> value_str = f[1] % value
> print("%-25s" % f[2], value_str)
>
> + self.bitmap_table.print_bitmap_table()
> +
>
> class Qcow2BitmapDirectory:
>
> @@ -83,6 +126,9 @@ class Qcow2BitmapDirectory:
> shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
> fd.seek(shift, 1)
>
> + for bm in self.bitmaps:
> + bm.read_bitmap_table(fd)
> +
> def get_bitmaps(self):
> return self.bitmaps
>
> @@ -223,6 +269,8 @@ class QcowHeader:
>
> self.set_defaults()
> self.cluster_size = 1 << self.cluster_bits
> + global cluster_size
> + cluster_size = self.cluster_size
Oh.. We should avoid such thing. You set global variable here and use it in another class. It will definitely break, if we try load two qcow2 files with different cluster sizes in one context.
>
> fd.seek(self.header_length)
> self.load_extensions(fd)
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
` (4 preceding siblings ...)
2020-06-01 13:48 ` [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image Andrey Shinkevich
@ 2020-06-01 13:48 ` Andrey Shinkevich
2020-06-02 17:40 ` Eric Blake
2020-06-02 21:36 ` Vladimir Sementsov-Ogievskiy
2020-06-01 21:46 ` [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Eric Blake
6 siblings, 2 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:48 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Represent QCOW2 metadata dumping with qcow2.py script in JSON format
{
"QCOW2_header_extensions": [
{
"Header_extension": "Feature table",
"magic": "0x6803f857",
"length": 192,
"data_str": "<binary>"
},
{
"Header_extension": "Bitmaps",
"magic": "0x23852875",
"length": 24,
"data": {
"nb_bitmaps": 2,
"reserved32": 0,
"bitmap_directory_size": 64,
"bitmap_directory_offset": 1048576,
"entries": [
{
"name": "bitmap-1",
"flags": [],
"flag_bits": 0,
"bitmap_table_offset": 589824,
"bitmap_table_size": 8,
"type": 1,
"granularity": 16,
"name_size": 8,
"extra_data_size": 0,
"bitmap_table": {
"table_entries": [
{
"type": "serialized",
"offset": 655360,
"size": 65536
},
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 108 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 105 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 76e0c69..fd1ef4f 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -3,11 +3,21 @@
import sys
import struct
import string
+import json
+dump_json = False
cluster_size = 0
+class ComplexEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if hasattr(obj, 'get_info_dict'):
+ return obj.get_info_dict()
+ else:
+ return json.JSONEncoder.default(self, obj)
+
+
class Qcow2BitmapTableEntry:
BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
@@ -23,6 +33,9 @@ class Qcow2BitmapTableEntry:
index = entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES
self.type = self.bmte_type[index]
+ def get_info_dict(self):
+ return dict(type=self.type, offset=self.offset, size=self.cluster_size)
+
class Qcow2BitmapTable:
@@ -39,6 +52,9 @@ class Qcow2BitmapTable:
entry.cluster_size))
print("")
+ def get_info_dict(self):
+ return dict(table_entries=self.entries)
+
class Qcow2BitmapDirEntry:
@@ -102,6 +118,18 @@ class Qcow2BitmapDirEntry:
self.bitmap_table.print_bitmap_table()
+ def get_info_dict(self):
+ return dict(name=self.name,
+ flags=self.bitmap_flags,
+ flag_bits=self.flag_bits,
+ bitmap_table_offset=self.bitmap_table_offset,
+ bitmap_table_size=self.bitmap_table_size,
+ type=self.type,
+ granularity=self.granularity_bits,
+ name_size=self.name_size,
+ extra_data_size=self.extra_data_size,
+ bitmap_table=self.bitmap_table)
+
class Qcow2BitmapDirectory:
@@ -177,6 +205,31 @@ class Qcow2BitmapExt:
self.dump_bitmap_ext()
self.dump_bitmap_directory()
+ def get_info_dict(self):
+ return dict(nb_bitmaps=self.nb_bitmaps,
+ reserved32=self.reserved32,
+ bitmap_directory_size=self.bitmap_directory_size,
+ bitmap_directory_offset=self.bitmap_directory_offset,
+ entries=self.bitmaps)
+
+
+class Qcow2HeaderDoc:
+
+ def __init__(self, h):
+ self.header = h
+
+ def get_info_dict(self):
+ return dict(QCOW2_header=self.header)
+
+
+class Qcow2HeaderExtensionsDoc:
+
+ def __init__(self, extensions):
+ self.extensions = extensions
+
+ def get_info_dict(self):
+ return dict(QCOW2_header_extensions=self.extensions)
+
class QcowHeaderExtension:
@@ -224,6 +277,17 @@ class QcowHeaderExtension:
if self.obj is not None:
self.obj.load(fd)
+ def get_info_dict(self):
+ he_dict = dict(Header_extension=self.name,
+ magic=hex(self.magic),
+ length=self.length)
+ if self.obj is not None:
+ he_dict.update(data=self.obj)
+ else:
+ he_dict.update(data_str=self.data_str)
+
+ return he_dict
+
class QcowHeader:
@@ -353,9 +417,34 @@ class QcowHeader:
print("%-25s" % f[2], value_str)
print("")
+ def get_info_dict(self):
+ return dict(magic=hex(self.magic),
+ version=self.version,
+ backing_file_offset=hex(self.backing_file_offset),
+ backing_file_size=self.backing_file_size,
+ cluster_bits=self.cluster_bits,
+ size=self.size,
+ crypt_method=self.crypt_method,
+ l1_size=self.l1_size,
+ l1_table_offset=hex(self.l1_table_offset),
+ refcount_table_offset=hex(self.refcount_table_offset),
+ refcount_table_clusters=self.refcount_table_clusters,
+ nb_snapshots=self.nb_snapshots,
+ snapshot_offset=hex(self.snapshot_offset),
+ incompatible_features=self.incompatible_features,
+ compatible_features=self.compatible_features,
+ autoclear_features=self.autoclear_features,
+ refcount_order=self.refcount_order,
+ header_length=self.header_length)
+
def dump_extensions(self):
- for ex in self.extensions:
+ if dump_json:
+ ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
+ print(json.dumps(ext_doc.get_info_dict(), indent=4,
+ cls=ComplexEncoder))
+ return
+ for ex in self.extensions:
print("%-25s %s" % ("Header extension:", ex.name))
print("%-25s %#x" % ("magic", ex.magic))
print("%-25s %d" % ("length", ex.length))
@@ -368,7 +457,11 @@ class QcowHeader:
def cmd_dump_header(fd):
h = QcowHeader(fd)
- h.dump()
+ if dump_json:
+ h_doc = Qcow2HeaderDoc(h)
+ print(json.dumps(h_doc.get_info_dict(), indent=4, cls=ComplexEncoder))
+ else:
+ h.dump()
h.dump_extensions()
def cmd_dump_header_exts(fd):
@@ -460,6 +553,12 @@ 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:
@@ -476,11 +575,14 @@ def main(filename, cmd, args):
fd.close()
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:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py
2020-06-01 13:48 ` [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py Andrey Shinkevich
@ 2020-06-02 17:40 ` Eric Blake
2020-06-02 21:36 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2020-06-02 17:40 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Represent QCOW2 metadata dumping with qcow2.py script in JSON format
>
> {
> "QCOW2_header_extensions": [
> {
> "Header_extension": "Feature table",
> "magic": "0x6803f857",
> "length": 192,
> "data_str": "<binary>"
> },
> {
> "Header_extension": "Bitmaps",
> "magic": "0x23852875",
> "length": 24,
> "data": {
> "nb_bitmaps": 2,
> "reserved32": 0,
Lengthy, but for a debug tool that's sometimes what you need.
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 108 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 105 insertions(+), 3 deletions(-)
>
My python is not the strongest, but nothing jumped out at me, and the
end results look good.
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] 22+ messages in thread* Re: [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py
2020-06-01 13:48 ` [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py Andrey Shinkevich
2020-06-02 17:40 ` Eric Blake
@ 2020-06-02 21:36 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-02 21:36 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
01.06.2020 16:48, Andrey Shinkevich wrote:
> Represent QCOW2 metadata dumping with qcow2.py script in JSON format
>
> {
> "QCOW2_header_extensions": [
> {
> "Header_extension": "Feature table",
> "magic": "0x6803f857",
> "length": 192,
> "data_str": "<binary>"
> },
> {
> "Header_extension": "Bitmaps",
> "magic": "0x23852875",
> "length": 24,
> "data": {
> "nb_bitmaps": 2,
> "reserved32": 0,
> "bitmap_directory_size": 64,
> "bitmap_directory_offset": 1048576,
> "entries": [
> {
> "name": "bitmap-1",
> "flags": [],
> "flag_bits": 0,
> "bitmap_table_offset": 589824,
> "bitmap_table_size": 8,
> "type": 1,
> "granularity": 16,
> "name_size": 8,
> "extra_data_size": 0,
> "bitmap_table": {
> "table_entries": [
> {
> "type": "serialized",
> "offset": 655360,
> "size": 65536
> },
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 108 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 105 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 76e0c69..fd1ef4f 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -3,11 +3,21 @@
> import sys
> import struct
> import string
> +import json
>
>
> +dump_json = False
> cluster_size = 0
>
>
> +class ComplexEncoder(json.JSONEncoder):
> + def default(self, obj):
> + if hasattr(obj, 'get_info_dict'):
> + return obj.get_info_dict()
> + else:
> + return json.JSONEncoder.default(self, obj)
> +
> +
> class Qcow2BitmapTableEntry:
>
> BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
> @@ -23,6 +33,9 @@ class Qcow2BitmapTableEntry:
> index = entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES
> self.type = self.bmte_type[index]
>
> + def get_info_dict(self):
> + return dict(type=self.type, offset=self.offset, size=self.cluster_size)
> +
>
> class Qcow2BitmapTable:
>
> @@ -39,6 +52,9 @@ class Qcow2BitmapTable:
> entry.cluster_size))
> print("")
>
> + def get_info_dict(self):
> + return dict(table_entries=self.entries)
> +
>
> class Qcow2BitmapDirEntry:
>
> @@ -102,6 +118,18 @@ class Qcow2BitmapDirEntry:
>
> self.bitmap_table.print_bitmap_table()
>
> + def get_info_dict(self):
> + return dict(name=self.name,
> + flags=self.bitmap_flags,
> + flag_bits=self.flag_bits,
> + bitmap_table_offset=self.bitmap_table_offset,
> + bitmap_table_size=self.bitmap_table_size,
> + type=self.type,
> + granularity=self.granularity_bits,
> + name_size=self.name_size,
> + extra_data_size=self.extra_data_size,
> + bitmap_table=self.bitmap_table)
> +
>
> class Qcow2BitmapDirectory:
>
> @@ -177,6 +205,31 @@ class Qcow2BitmapExt:
> self.dump_bitmap_ext()
> self.dump_bitmap_directory()
>
> + def get_info_dict(self):
> + return dict(nb_bitmaps=self.nb_bitmaps,
> + reserved32=self.reserved32,
> + bitmap_directory_size=self.bitmap_directory_size,
> + bitmap_directory_offset=self.bitmap_directory_offset,
> + entries=self.bitmaps)
> +
> +
> +class Qcow2HeaderDoc:
> +
> + def __init__(self, h):
> + self.header = h
> +
> + def get_info_dict(self):
> + return dict(QCOW2_header=self.header)
> +
> +
> +class Qcow2HeaderExtensionsDoc:
> +
> + def __init__(self, extensions):
> + self.extensions = extensions
> +
> + def get_info_dict(self):
> + return dict(QCOW2_header_extensions=self.extensions)
> +
>
> class QcowHeaderExtension:
>
> @@ -224,6 +277,17 @@ class QcowHeaderExtension:
> if self.obj is not None:
> self.obj.load(fd)
>
> + def get_info_dict(self):
> + he_dict = dict(Header_extension=self.name,
> + magic=hex(self.magic),
> + length=self.length)
> + if self.obj is not None:
> + he_dict.update(data=self.obj)
> + else:
> + he_dict.update(data_str=self.data_str)
> +
> + return he_dict
> +
>
> class QcowHeader:
>
> @@ -353,9 +417,34 @@ class QcowHeader:
> print("%-25s" % f[2], value_str)
> print("")
>
> + def get_info_dict(self):
> + return dict(magic=hex(self.magic),
> + version=self.version,
> + backing_file_offset=hex(self.backing_file_offset),
> + backing_file_size=self.backing_file_size,
> + cluster_bits=self.cluster_bits,
> + size=self.size,
> + crypt_method=self.crypt_method,
> + l1_size=self.l1_size,
> + l1_table_offset=hex(self.l1_table_offset),
Why to store string instead of int in qcow2? Better store numbers as numbers.
> + refcount_table_offset=hex(self.refcount_table_offset),
> + refcount_table_clusters=self.refcount_table_clusters,
> + nb_snapshots=self.nb_snapshots,
> + snapshot_offset=hex(self.snapshot_offset),
> + incompatible_features=self.incompatible_features,
> + compatible_features=self.compatible_features,
> + autoclear_features=self.autoclear_features,
> + refcount_order=self.refcount_order,
> + header_length=self.header_length)
isn't it mostly __dict__? We definitely want to automate this in base class.
> +
> def dump_extensions(self):
> - for ex in self.extensions:
> + if dump_json:
> + ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
> + print(json.dumps(ext_doc.get_info_dict(), indent=4,
> + cls=ComplexEncoder))
> + return
>
> + for ex in self.extensions:
> print("%-25s %s" % ("Header extension:", ex.name))
> print("%-25s %#x" % ("magic", ex.magic))
> print("%-25s %d" % ("length", ex.length))
> @@ -368,7 +457,11 @@ class QcowHeader:
>
> def cmd_dump_header(fd):
> h = QcowHeader(fd)
> - h.dump()
> + if dump_json:
> + h_doc = Qcow2HeaderDoc(h)
> + print(json.dumps(h_doc.get_info_dict(), indent=4, cls=ComplexEncoder))
> + else:
> + h.dump()
> h.dump_extensions()
>
> def cmd_dump_header_exts(fd):
> @@ -460,6 +553,12 @@ 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:
> @@ -476,11 +575,14 @@ def main(filename, cmd, args):
> fd.close()
>
> 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:
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata
2020-06-01 13:48 [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Andrey Shinkevich
` (5 preceding siblings ...)
2020-06-01 13:48 ` [PATCH v3 6/6] iotests: Dump QCOW2 image metadata in JSON format with qcow2.py Andrey Shinkevich
@ 2020-06-01 21:46 ` Eric Blake
2020-06-04 7:54 ` Andrey Shinkevich
6 siblings, 1 reply; 22+ messages in thread
From: Eric Blake @ 2020-06-01 21:46 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Add dirty bitmap information to QCOW2 metadata dump in qcow2.py script.
>
> v3:
> 01: JSON format output possibility added.
Also, you split it into a series. Thanks; this makes it easier to
review each step :)
>
> v2:
> 01: Refactoring of the Python code in the script qcow2.py.
> New methods were added. The bitmap dictionary was instantiated.
> The all of bitmaps information is read completely before
> printing the dictionary.
> 02: The outputs of the tests 031, 036 and 061 were modified.
>
> Andrey Shinkevich (6):
> iotests: Add extension names to qcow2.py dump
> iotests: move check for printable data to QcowHeaderExtension class
> iotests: dump bitmap extension data with qcow2.py
> iotests: Dump bitmap directory info with qcow2.py
> iotests: Dump bitmap table entries serialized in QCOW2 image
> iotests: Dump QCOW2 image metadata in JSON format with qcow2.py
>
> tests/qemu-iotests/031.out | 22 +--
> tests/qemu-iotests/036.out | 4 +-
> tests/qemu-iotests/061.out | 18 +--
> tests/qemu-iotests/qcow2.py | 338 ++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 346 insertions(+), 36 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata
2020-06-01 21:46 ` [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata Eric Blake
@ 2020-06-04 7:54 ` Andrey Shinkevich
0 siblings, 0 replies; 22+ messages in thread
From: Andrey Shinkevich @ 2020-06-04 7:54 UTC (permalink / raw)
To: Eric Blake, qemu-block@nongnu.org
Cc: kwolf@redhat.com, Denis Lunev, Vladimir Sementsov-Ogievskiy,
qemu-devel@nongnu.org, mreitz@redhat.com
[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]
Thank you very much dear colleagues for your collaboration. Your reviews comments are well noted.
Andrey
________________________________
From: Eric Blake <eblake@redhat.com>
Sent: Tuesday, June 2, 2020 12:46 AM
To: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>; qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; kwolf@redhat.com <kwolf@redhat.com>; mreitz@redhat.com <mreitz@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Subject: Re: [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata
On 6/1/20 8:48 AM, Andrey Shinkevich wrote:
> Add dirty bitmap information to QCOW2 metadata dump in qcow2.py script.
>
> v3:
> 01: JSON format output possibility added.
Also, you split it into a series. Thanks; this makes it easier to
review each step :)
>
> v2:
> 01: Refactoring of the Python code in the script qcow2.py.
> New methods were added. The bitmap dictionary was instantiated.
> The all of bitmaps information is read completely before
> printing the dictionary.
> 02: The outputs of the tests 031, 036 and 061 were modified.
>
> Andrey Shinkevich (6):
> iotests: Add extension names to qcow2.py dump
> iotests: move check for printable data to QcowHeaderExtension class
> iotests: dump bitmap extension data with qcow2.py
> iotests: Dump bitmap directory info with qcow2.py
> iotests: Dump bitmap table entries serialized in QCOW2 image
> iotests: Dump QCOW2 image metadata in JSON format with qcow2.py
>
> tests/qemu-iotests/031.out | 22 +--
> tests/qemu-iotests/036.out | 4 +-
> tests/qemu-iotests/061.out | 18 +--
> tests/qemu-iotests/qcow2.py | 338 ++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 346 insertions(+), 36 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: Type: text/html, Size: 3396 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread