* [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
@ 2020-06-04 17:41 Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 01/12] qcow2.py: python style fixes Vladimir Sementsov-Ogievskiy
` (13 more replies)
0 siblings, 14 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Hi all!
Here is my suggestion to substitute only first three patches :) of
Andrey's [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata
so, I called it v4 for convenience.
What is here:
1. First, update code style
2. Next, try to refactor in a manner which will make adding new data
structures simple (look at Qcow2BitmapExt class in last patch)
I think, next step is to add type hints. Then add more structures.
And, anyway, at some point we should move it into python/ directory (at
least qcow2_format.py lib)
Vladimir Sementsov-Ogievskiy (12):
qcow2.py: python style fixes
qcow2.py: move qcow2 format classes to separate module
qcow2_format.py: drop new line printing at end of dump()
qcow2_format.py: use tuples instead of lists for fields
qcow2_format.py: use modern string formatting
qcow2_format.py: use strings to specify c-type of struct fields
qcow2_format.py: separate generic functionality of structure classes
qcow2_format.py: add field-formatting class
qcow2_format.py: QcowHeaderExtension: add dump method
qcow2_format: refactor QcowHeaderExtension as a subclass of
Qcow2Struct
qcow2: QcowHeaderExtension print names for extension magics
qcow2_format.py: dump bitmaps header extension
tests/qemu-iotests/031.out | 22 +--
tests/qemu-iotests/036.out | 4 +-
tests/qemu-iotests/061.out | 14 +-
tests/qemu-iotests/291 | 4 +
tests/qemu-iotests/291.out | 33 ++++
tests/qemu-iotests/qcow2.py | 200 ++++------------------
tests/qemu-iotests/qcow2_format.py | 260 +++++++++++++++++++++++++++++
7 files changed, 348 insertions(+), 189 deletions(-)
create mode 100644 tests/qemu-iotests/qcow2_format.py
--
2.21.0
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v4 01/12] qcow2.py: python style fixes
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 10:23 ` Andrey Shinkevich
2020-06-05 19:43 ` Eric Blake
2020-06-04 17:41 ` [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module Vladimir Sementsov-Ogievskiy
` (12 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Fix flake8 complains. Leave the only chunk of lines over 79 characters:
initialization of cmds variable. Leave it for another day, when it
should be refactored to utilize argparse instead of hand-written
parsing.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 92 +++++++++++++++++++++----------------
1 file changed, 53 insertions(+), 39 deletions(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 94a07b2f6f..539f5a186b 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -4,6 +4,7 @@ import sys
import struct
import string
+
class QcowHeaderExtension:
def __init__(self, magic, length, data):
@@ -11,14 +12,15 @@ class QcowHeaderExtension:
padding = 8 - (length % 8)
data += b"\0" * padding
- self.magic = magic
+ self.magic = magic
self.length = length
- self.data = data
+ self.data = data
@classmethod
def create(cls, magic, data):
return QcowHeaderExtension(magic, len(data), data)
+
class QcowHeader:
uint32_t = 'I'
@@ -26,27 +28,27 @@ class QcowHeader:
fields = [
# Version 2 header fields
- [ uint32_t, '%#x', 'magic' ],
- [ uint32_t, '%d', 'version' ],
- [ uint64_t, '%#x', 'backing_file_offset' ],
- [ uint32_t, '%#x', 'backing_file_size' ],
- [ uint32_t, '%d', 'cluster_bits' ],
- [ uint64_t, '%d', 'size' ],
- [ uint32_t, '%d', 'crypt_method' ],
- [ uint32_t, '%d', 'l1_size' ],
- [ uint64_t, '%#x', 'l1_table_offset' ],
- [ uint64_t, '%#x', 'refcount_table_offset' ],
- [ uint32_t, '%d', 'refcount_table_clusters' ],
- [ uint32_t, '%d', 'nb_snapshots' ],
- [ uint64_t, '%#x', 'snapshot_offset' ],
+ [uint32_t, '%#x', 'magic'],
+ [uint32_t, '%d', 'version'],
+ [uint64_t, '%#x', 'backing_file_offset'],
+ [uint32_t, '%#x', 'backing_file_size'],
+ [uint32_t, '%d', 'cluster_bits'],
+ [uint64_t, '%d', 'size'],
+ [uint32_t, '%d', 'crypt_method'],
+ [uint32_t, '%d', 'l1_size'],
+ [uint64_t, '%#x', 'l1_table_offset'],
+ [uint64_t, '%#x', 'refcount_table_offset'],
+ [uint32_t, '%d', 'refcount_table_clusters'],
+ [uint32_t, '%d', 'nb_snapshots'],
+ [uint64_t, '%#x', 'snapshot_offset'],
# Version 3 header fields
- [ uint64_t, 'mask', 'incompatible_features' ],
- [ uint64_t, 'mask', 'compatible_features' ],
- [ uint64_t, 'mask', 'autoclear_features' ],
- [ uint32_t, '%d', 'refcount_order' ],
- [ uint32_t, '%d', 'header_length' ],
- ];
+ [uint64_t, 'mask', 'incompatible_features'],
+ [uint64_t, 'mask', 'compatible_features'],
+ [uint64_t, 'mask', 'autoclear_features'],
+ [uint32_t, '%d', 'refcount_order'],
+ [uint32_t, '%d', 'header_length'],
+ ]
fmt = '>' + ''.join(field[0] for field in fields)
@@ -59,7 +61,7 @@ class QcowHeader:
header = struct.unpack(QcowHeader.fmt, buf)
self.__dict__ = dict((field[2], header[i])
- for i, field in enumerate(QcowHeader.fields))
+ for i, field in enumerate(QcowHeader.fields))
self.set_defaults()
self.cluster_size = 1 << self.cluster_bits
@@ -96,7 +98,8 @@ 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))
def update_extensions(self, fd):
@@ -108,14 +111,13 @@ class QcowHeader:
fd.write(buf)
fd.write(ex.data)
- if self.backing_file != None:
+ if self.backing_file is not None:
self.backing_file_offset = fd.tell()
fd.write(self.backing_file)
if fd.tell() > self.cluster_size:
raise Exception("I think I just broke the image...")
-
def update(self, fd):
header_bytes = self.header_length
@@ -163,19 +165,21 @@ def cmd_dump_header(fd):
h.dump()
h.dump_extensions()
+
def cmd_dump_header_exts(fd):
h = QcowHeader(fd)
h.dump_extensions()
+
def cmd_set_header(fd, name, value):
try:
value = int(value, 0)
- except:
+ except ValueError:
print("'%s' is not a valid number" % value)
sys.exit(1)
fields = (field[2] for field in QcowHeader.fields)
- if not name in fields:
+ if name not in fields:
print("'%s' is not a known header field" % name)
sys.exit(1)
@@ -183,25 +187,29 @@ def cmd_set_header(fd, name, value):
h.__dict__[name] = value
h.update(fd)
+
def cmd_add_header_ext(fd, magic, data):
try:
magic = int(magic, 0)
- except:
+ except ValueError:
print("'%s' is not a valid magic number" % magic)
sys.exit(1)
h = QcowHeader(fd)
- h.extensions.append(QcowHeaderExtension.create(magic, data.encode('ascii')))
+ h.extensions.append(QcowHeaderExtension.create(magic,
+ data.encode('ascii')))
h.update(fd)
+
def cmd_add_header_ext_stdio(fd, magic):
data = sys.stdin.read()
cmd_add_header_ext(fd, magic, data)
+
def cmd_del_header_ext(fd, magic):
try:
magic = int(magic, 0)
- except:
+ except ValueError:
print("'%s' is not a valid magic number" % magic)
sys.exit(1)
@@ -219,12 +227,13 @@ def cmd_del_header_ext(fd, magic):
h.update(fd)
+
def cmd_set_feature_bit(fd, group, bit):
try:
bit = int(bit, 0)
if bit < 0 or bit >= 64:
raise ValueError
- except:
+ except ValueError:
print("'%s' is not a valid bit number in range [0, 64)" % bit)
sys.exit(1)
@@ -236,21 +245,24 @@ def cmd_set_feature_bit(fd, group, bit):
elif group == 'autoclear':
h.autoclear_features |= 1 << bit
else:
- print("'%s' is not a valid group, try 'incompatible', 'compatible', or 'autoclear'" % group)
+ print("'%s' is not a valid group, try "
+ "'incompatible', 'compatible', or 'autoclear'" % group)
sys.exit(1)
h.update(fd)
+
cmds = [
- [ 'dump-header', cmd_dump_header, 0, 'Dump image header and header extensions' ],
- [ 'dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions' ],
- [ 'set-header', cmd_set_header, 2, 'Set a field in the header'],
- [ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header extension' ],
- [ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin' ],
- [ 'del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension' ],
- [ 'set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'],
+ ['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'],
+ ['dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions'],
+ ['set-header', cmd_set_header, 2, 'Set a field in the header'],
+ ['add-header-ext', cmd_add_header_ext, 2, 'Add a header extension'],
+ ['add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin'],
+ ['del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension'],
+ ['set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'],
]
+
def main(filename, cmd, args):
fd = open(filename, "r+b")
try:
@@ -267,6 +279,7 @@ def main(filename, cmd, args):
finally:
fd.close()
+
def usage():
print("Usage: %s <file> <cmd> [<arg>, ...]" % sys.argv[0])
print("")
@@ -274,6 +287,7 @@ def usage():
for name, handler, num_args, desc in cmds:
print(" %-20s - %s" % (name, desc))
+
if __name__ == '__main__':
if len(sys.argv) < 3:
usage()
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 01/12] qcow2.py: python style fixes Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 10:51 ` Andrey Shinkevich
2020-06-05 20:14 ` Eric Blake
2020-06-04 17:41 ` [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump() Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
We are going to enhance qcow2 format parsing by adding more structure
classes. Let's split format parsing from utility code.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 161 +----------------------------
tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
2 files changed, 161 insertions(+), 157 deletions(-)
create mode 100644 tests/qemu-iotests/qcow2_format.py
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 539f5a186b..d9c41668fd 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -1,163 +1,10 @@
#!/usr/bin/env python3
-
import sys
-import struct
-import string
-
-
-class QcowHeaderExtension:
-
- def __init__(self, magic, length, data):
- if length % 8 != 0:
- padding = 8 - (length % 8)
- data += b"\0" * padding
-
- self.magic = magic
- self.length = length
- self.data = data
-
- @classmethod
- def create(cls, magic, data):
- return QcowHeaderExtension(magic, len(data), data)
-
-
-class QcowHeader:
-
- uint32_t = 'I'
- uint64_t = 'Q'
-
- fields = [
- # Version 2 header fields
- [uint32_t, '%#x', 'magic'],
- [uint32_t, '%d', 'version'],
- [uint64_t, '%#x', 'backing_file_offset'],
- [uint32_t, '%#x', 'backing_file_size'],
- [uint32_t, '%d', 'cluster_bits'],
- [uint64_t, '%d', 'size'],
- [uint32_t, '%d', 'crypt_method'],
- [uint32_t, '%d', 'l1_size'],
- [uint64_t, '%#x', 'l1_table_offset'],
- [uint64_t, '%#x', 'refcount_table_offset'],
- [uint32_t, '%d', 'refcount_table_clusters'],
- [uint32_t, '%d', 'nb_snapshots'],
- [uint64_t, '%#x', 'snapshot_offset'],
-
- # Version 3 header fields
- [uint64_t, 'mask', 'incompatible_features'],
- [uint64_t, 'mask', 'compatible_features'],
- [uint64_t, 'mask', 'autoclear_features'],
- [uint32_t, '%d', 'refcount_order'],
- [uint32_t, '%d', 'header_length'],
- ]
-
- fmt = '>' + ''.join(field[0] for field in fields)
-
- def __init__(self, fd):
-
- buf_size = struct.calcsize(QcowHeader.fmt)
-
- fd.seek(0)
- buf = fd.read(buf_size)
-
- header = struct.unpack(QcowHeader.fmt, buf)
- self.__dict__ = dict((field[2], header[i])
- for i, field in enumerate(QcowHeader.fields))
-
- self.set_defaults()
- self.cluster_size = 1 << self.cluster_bits
-
- fd.seek(self.header_length)
- self.load_extensions(fd)
-
- if self.backing_file_offset:
- fd.seek(self.backing_file_offset)
- self.backing_file = fd.read(self.backing_file_size)
- else:
- self.backing_file = None
-
- def set_defaults(self):
- if self.version == 2:
- self.incompatible_features = 0
- self.compatible_features = 0
- self.autoclear_features = 0
- self.refcount_order = 4
- self.header_length = 72
-
- def load_extensions(self, fd):
- self.extensions = []
-
- if self.backing_file_offset != 0:
- end = min(self.cluster_size, self.backing_file_offset)
- else:
- end = self.cluster_size
-
- while fd.tell() < end:
- (magic, length) = struct.unpack('>II', fd.read(8))
- if magic == 0:
- break
- else:
- padded = (length + 7) & ~7
- data = fd.read(padded)
- self.extensions.append(QcowHeaderExtension(magic, length,
- data))
-
- def update_extensions(self, fd):
-
- fd.seek(self.header_length)
- extensions = self.extensions
- extensions.append(QcowHeaderExtension(0, 0, b""))
- for ex in extensions:
- buf = struct.pack('>II', ex.magic, ex.length)
- fd.write(buf)
- fd.write(ex.data)
-
- if self.backing_file is not None:
- self.backing_file_offset = fd.tell()
- fd.write(self.backing_file)
-
- if fd.tell() > self.cluster_size:
- raise Exception("I think I just broke the image...")
-
- def update(self, fd):
- header_bytes = self.header_length
-
- self.update_extensions(fd)
-
- fd.seek(0)
- header = tuple(self.__dict__[f] for t, p, f in QcowHeader.fields)
- buf = struct.pack(QcowHeader.fmt, *header)
- buf = buf[0:header_bytes-1]
- fd.write(buf)
-
- def dump(self):
- for f in QcowHeader.fields:
- value = self.__dict__[f[2]]
- if f[1] == 'mask':
- bits = []
- for bit in range(64):
- if value & (1 << bit):
- bits.append(bit)
- value_str = str(bits)
- else:
- value_str = f[1] % value
-
- print("%-25s" % f[2], value_str)
- print("")
-
- 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("Header extension:")
- print("%-25s %#x" % ("magic", ex.magic))
- print("%-25s %d" % ("length", ex.length))
- print("%-25s %s" % ("data", data))
- print("")
+from qcow2_format import (
+ QcowHeader,
+ QcowHeaderExtension
+)
def cmd_dump_header(fd):
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
new file mode 100644
index 0000000000..c7270a0a6e
--- /dev/null
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -0,0 +1,157 @@
+import struct
+import string
+
+
+class QcowHeaderExtension:
+
+ def __init__(self, magic, length, data):
+ if length % 8 != 0:
+ padding = 8 - (length % 8)
+ data += b"\0" * padding
+
+ self.magic = magic
+ self.length = length
+ self.data = data
+
+ @classmethod
+ def create(cls, magic, data):
+ return QcowHeaderExtension(magic, len(data), data)
+
+
+class QcowHeader:
+
+ uint32_t = 'I'
+ uint64_t = 'Q'
+
+ fields = [
+ # Version 2 header fields
+ [uint32_t, '%#x', 'magic'],
+ [uint32_t, '%d', 'version'],
+ [uint64_t, '%#x', 'backing_file_offset'],
+ [uint32_t, '%#x', 'backing_file_size'],
+ [uint32_t, '%d', 'cluster_bits'],
+ [uint64_t, '%d', 'size'],
+ [uint32_t, '%d', 'crypt_method'],
+ [uint32_t, '%d', 'l1_size'],
+ [uint64_t, '%#x', 'l1_table_offset'],
+ [uint64_t, '%#x', 'refcount_table_offset'],
+ [uint32_t, '%d', 'refcount_table_clusters'],
+ [uint32_t, '%d', 'nb_snapshots'],
+ [uint64_t, '%#x', 'snapshot_offset'],
+
+ # Version 3 header fields
+ [uint64_t, 'mask', 'incompatible_features'],
+ [uint64_t, 'mask', 'compatible_features'],
+ [uint64_t, 'mask', 'autoclear_features'],
+ [uint32_t, '%d', 'refcount_order'],
+ [uint32_t, '%d', 'header_length'],
+ ]
+
+ fmt = '>' + ''.join(field[0] for field in fields)
+
+ def __init__(self, fd):
+
+ buf_size = struct.calcsize(QcowHeader.fmt)
+
+ fd.seek(0)
+ buf = fd.read(buf_size)
+
+ header = struct.unpack(QcowHeader.fmt, buf)
+ self.__dict__ = dict((field[2], header[i])
+ for i, field in enumerate(QcowHeader.fields))
+
+ self.set_defaults()
+ self.cluster_size = 1 << self.cluster_bits
+
+ fd.seek(self.header_length)
+ self.load_extensions(fd)
+
+ if self.backing_file_offset:
+ fd.seek(self.backing_file_offset)
+ self.backing_file = fd.read(self.backing_file_size)
+ else:
+ self.backing_file = None
+
+ def set_defaults(self):
+ if self.version == 2:
+ self.incompatible_features = 0
+ self.compatible_features = 0
+ self.autoclear_features = 0
+ self.refcount_order = 4
+ self.header_length = 72
+
+ def load_extensions(self, fd):
+ self.extensions = []
+
+ if self.backing_file_offset != 0:
+ end = min(self.cluster_size, self.backing_file_offset)
+ else:
+ end = self.cluster_size
+
+ while fd.tell() < end:
+ (magic, length) = struct.unpack('>II', fd.read(8))
+ if magic == 0:
+ break
+ else:
+ padded = (length + 7) & ~7
+ data = fd.read(padded)
+ self.extensions.append(QcowHeaderExtension(magic, length,
+ data))
+
+ def update_extensions(self, fd):
+
+ fd.seek(self.header_length)
+ extensions = self.extensions
+ extensions.append(QcowHeaderExtension(0, 0, b""))
+ for ex in extensions:
+ buf = struct.pack('>II', ex.magic, ex.length)
+ fd.write(buf)
+ fd.write(ex.data)
+
+ if self.backing_file is not None:
+ self.backing_file_offset = fd.tell()
+ fd.write(self.backing_file)
+
+ if fd.tell() > self.cluster_size:
+ raise Exception("I think I just broke the image...")
+
+ def update(self, fd):
+ header_bytes = self.header_length
+
+ self.update_extensions(fd)
+
+ fd.seek(0)
+ header = tuple(self.__dict__[f] for t, p, f in QcowHeader.fields)
+ buf = struct.pack(QcowHeader.fmt, *header)
+ buf = buf[0:header_bytes-1]
+ fd.write(buf)
+
+ def dump(self):
+ for f in QcowHeader.fields:
+ value = self.__dict__[f[2]]
+ if f[1] == 'mask':
+ bits = []
+ for bit in range(64):
+ if value & (1 << bit):
+ bits.append(bit)
+ value_str = str(bits)
+ else:
+ value_str = f[1] % value
+
+ print("%-25s" % f[2], value_str)
+ print("")
+
+ 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("Header extension:")
+ print("%-25s %#x" % ("magic", ex.magic))
+ print("%-25s %d" % ("length", ex.length))
+ print("%-25s %s" % ("data", data))
+ print("")
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump()
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 01/12] qcow2.py: python style fixes Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 10:54 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
This will simplify further conversion. To compensate, print this empty
line directly in cmd_dump_header().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 1 +
tests/qemu-iotests/qcow2_format.py | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index d9c41668fd..79db81b040 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -10,6 +10,7 @@ from qcow2_format import (
def cmd_dump_header(fd):
h = QcowHeader(fd)
h.dump()
+ print()
h.dump_extensions()
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index c7270a0a6e..99e5248e73 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -139,7 +139,6 @@ class QcowHeader:
value_str = f[1] % value
print("%-25s" % f[2], value_str)
- print("")
def dump_extensions(self):
for ex in self.extensions:
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump() Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 11:20 ` Andrey Shinkevich
2020-06-05 20:16 ` Eric Blake
2020-06-04 17:41 ` [PATCH v4 05/12] qcow2_format.py: use modern string formatting Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
No need in lists: it's a constant variable.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 99e5248e73..5d242c4aa4 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -23,29 +23,29 @@ class QcowHeader:
uint32_t = 'I'
uint64_t = 'Q'
- fields = [
+ fields = (
# Version 2 header fields
- [uint32_t, '%#x', 'magic'],
- [uint32_t, '%d', 'version'],
- [uint64_t, '%#x', 'backing_file_offset'],
- [uint32_t, '%#x', 'backing_file_size'],
- [uint32_t, '%d', 'cluster_bits'],
- [uint64_t, '%d', 'size'],
- [uint32_t, '%d', 'crypt_method'],
- [uint32_t, '%d', 'l1_size'],
- [uint64_t, '%#x', 'l1_table_offset'],
- [uint64_t, '%#x', 'refcount_table_offset'],
- [uint32_t, '%d', 'refcount_table_clusters'],
- [uint32_t, '%d', 'nb_snapshots'],
- [uint64_t, '%#x', 'snapshot_offset'],
+ (uint32_t, '%#x', 'magic'),
+ (uint32_t, '%d', 'version'),
+ (uint64_t, '%#x', 'backing_file_offset'),
+ (uint32_t, '%#x', 'backing_file_size'),
+ (uint32_t, '%d', 'cluster_bits'),
+ (uint64_t, '%d', 'size'),
+ (uint32_t, '%d', 'crypt_method'),
+ (uint32_t, '%d', 'l1_size'),
+ (uint64_t, '%#x', 'l1_table_offset'),
+ (uint64_t, '%#x', 'refcount_table_offset'),
+ (uint32_t, '%d', 'refcount_table_clusters'),
+ (uint32_t, '%d', 'nb_snapshots'),
+ (uint64_t, '%#x', 'snapshot_offset'),
# Version 3 header fields
- [uint64_t, 'mask', 'incompatible_features'],
- [uint64_t, 'mask', 'compatible_features'],
- [uint64_t, 'mask', 'autoclear_features'],
- [uint32_t, '%d', 'refcount_order'],
- [uint32_t, '%d', 'header_length'],
- ]
+ (uint64_t, 'mask', 'incompatible_features'),
+ (uint64_t, 'mask', 'compatible_features'),
+ (uint64_t, 'mask', 'autoclear_features'),
+ (uint32_t, '%d', 'refcount_order'),
+ (uint32_t, '%d', 'header_length'),
+ )
fmt = '>' + ''.join(field[0] for field in fields)
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 05/12] qcow2_format.py: use modern string formatting
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 11:27 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Use .format and f-strings instead of old %style. Also, the file uses
both '' and "" quotes, for consistency let's use '', except for cases
when we need '' inside the string (use "" to avoid extra escaping).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 54 +++++++++++++++---------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 5d242c4aa4..80e7c3f81d 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -7,7 +7,7 @@ class QcowHeaderExtension:
def __init__(self, magic, length, data):
if length % 8 != 0:
padding = 8 - (length % 8)
- data += b"\0" * padding
+ data += b'\0' * padding
self.magic = magic
self.length = length
@@ -25,26 +25,26 @@ class QcowHeader:
fields = (
# Version 2 header fields
- (uint32_t, '%#x', 'magic'),
- (uint32_t, '%d', 'version'),
- (uint64_t, '%#x', 'backing_file_offset'),
- (uint32_t, '%#x', 'backing_file_size'),
- (uint32_t, '%d', 'cluster_bits'),
- (uint64_t, '%d', 'size'),
- (uint32_t, '%d', 'crypt_method'),
- (uint32_t, '%d', 'l1_size'),
- (uint64_t, '%#x', 'l1_table_offset'),
- (uint64_t, '%#x', 'refcount_table_offset'),
- (uint32_t, '%d', 'refcount_table_clusters'),
- (uint32_t, '%d', 'nb_snapshots'),
- (uint64_t, '%#x', 'snapshot_offset'),
+ (uint32_t, '{:#x}', 'magic'),
+ (uint32_t, '{}', 'version'),
+ (uint64_t, '{:#x}', 'backing_file_offset'),
+ (uint32_t, '{:#x}', 'backing_file_size'),
+ (uint32_t, '{}', 'cluster_bits'),
+ (uint64_t, '{}', 'size'),
+ (uint32_t, '{}', 'crypt_method'),
+ (uint32_t, '{}', 'l1_size'),
+ (uint64_t, '{:#x}', 'l1_table_offset'),
+ (uint64_t, '{:#x}', 'refcount_table_offset'),
+ (uint32_t, '{}', 'refcount_table_clusters'),
+ (uint32_t, '{}', 'nb_snapshots'),
+ (uint64_t, '{:#x}', 'snapshot_offset'),
# Version 3 header fields
(uint64_t, 'mask', 'incompatible_features'),
(uint64_t, 'mask', 'compatible_features'),
(uint64_t, 'mask', 'autoclear_features'),
- (uint32_t, '%d', 'refcount_order'),
- (uint32_t, '%d', 'header_length'),
+ (uint32_t, '{}', 'refcount_order'),
+ (uint32_t, '{}', 'header_length'),
)
fmt = '>' + ''.join(field[0] for field in fields)
@@ -102,7 +102,7 @@ class QcowHeader:
fd.seek(self.header_length)
extensions = self.extensions
- extensions.append(QcowHeaderExtension(0, 0, b""))
+ extensions.append(QcowHeaderExtension(0, 0, b''))
for ex in extensions:
buf = struct.pack('>II', ex.magic, ex.length)
fd.write(buf)
@@ -113,7 +113,7 @@ class QcowHeader:
fd.write(self.backing_file)
if fd.tell() > self.cluster_size:
- raise Exception("I think I just broke the image...")
+ raise Exception('I think I just broke the image...')
def update(self, fd):
header_bytes = self.header_length
@@ -136,21 +136,21 @@ class QcowHeader:
bits.append(bit)
value_str = str(bits)
else:
- value_str = f[1] % value
+ value_str = f[1].format(value)
- print("%-25s" % f[2], value_str)
+ print(f'{f[2]:<25} {value_str}')
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')
+ data = f"'{ data.decode('ascii') }'"
else:
- data = "<binary>"
+ data = '<binary>'
- print("Header extension:")
- print("%-25s %#x" % ("magic", ex.magic))
- print("%-25s %d" % ("length", ex.length))
- print("%-25s %s" % ("data", data))
- print("")
+ print('Header extension:')
+ print(f'{"magic":<25} {ex.magic:#x}')
+ print(f'{"length":<25} {ex.length}')
+ print(f'{"data":<25} {data}')
+ print()
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 05/12] qcow2_format.py: use modern string formatting Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 11:36 ` Andrey Shinkevich
2020-06-05 20:19 ` Eric Blake
2020-06-04 17:41 ` [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
We are going to move field-parsing to super-class, this will be simpler
with simple string specifiers instead of variables.
For some reason python doesn't allow to define ctypes in class too, as
well as fields: it's not available than in 'for' operator. Don't worry:
ctypes will be moved to metaclass soon.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 50 +++++++++++++++++-------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 80e7c3f81d..1fd9473b7f 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -18,36 +18,42 @@ class QcowHeaderExtension:
return QcowHeaderExtension(magic, len(data), data)
-class QcowHeader:
+# Mapping from c types to python struct format
+ctypes = {
+ 'u8': 'B',
+ 'u16': 'H',
+ 'u32': 'I',
+ 'u64': 'Q'
+}
+
- uint32_t = 'I'
- uint64_t = 'Q'
+class QcowHeader:
fields = (
# Version 2 header fields
- (uint32_t, '{:#x}', 'magic'),
- (uint32_t, '{}', 'version'),
- (uint64_t, '{:#x}', 'backing_file_offset'),
- (uint32_t, '{:#x}', 'backing_file_size'),
- (uint32_t, '{}', 'cluster_bits'),
- (uint64_t, '{}', 'size'),
- (uint32_t, '{}', 'crypt_method'),
- (uint32_t, '{}', 'l1_size'),
- (uint64_t, '{:#x}', 'l1_table_offset'),
- (uint64_t, '{:#x}', 'refcount_table_offset'),
- (uint32_t, '{}', 'refcount_table_clusters'),
- (uint32_t, '{}', 'nb_snapshots'),
- (uint64_t, '{:#x}', 'snapshot_offset'),
+ ('u32', '{:#x}', 'magic'),
+ ('u32', '{}', 'version'),
+ ('u64', '{:#x}', 'backing_file_offset'),
+ ('u32', '{:#x}', 'backing_file_size'),
+ ('u32', '{}', 'cluster_bits'),
+ ('u64', '{}', 'size'),
+ ('u32', '{}', 'crypt_method'),
+ ('u32', '{}', 'l1_size'),
+ ('u64', '{:#x}', 'l1_table_offset'),
+ ('u64', '{:#x}', 'refcount_table_offset'),
+ ('u32', '{}', 'refcount_table_clusters'),
+ ('u32', '{}', 'nb_snapshots'),
+ ('u64', '{:#x}', 'snapshot_offset'),
# Version 3 header fields
- (uint64_t, 'mask', 'incompatible_features'),
- (uint64_t, 'mask', 'compatible_features'),
- (uint64_t, 'mask', 'autoclear_features'),
- (uint32_t, '{}', 'refcount_order'),
- (uint32_t, '{}', 'header_length'),
+ ('u64', 'mask', 'incompatible_features'),
+ ('u64', 'mask', 'compatible_features'),
+ ('u64', 'mask', 'autoclear_features'),
+ ('u32', '{}', 'refcount_order'),
+ ('u32', '{}', 'header_length'),
)
- fmt = '>' + ''.join(field[0] for field in fields)
+ fmt = '>' + ''.join(ctypes[f[0]] for f in fields)
def __init__(self, fd):
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 12:29 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 08/12] qcow2_format.py: add field-formatting class Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
We are going to introduce more Qcow2 structure types, defined like
QcowHeader. Move generic functionality into base class to be reused for
further structure classes.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 93 +++++++++++++++++++-----------
1 file changed, 58 insertions(+), 35 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 1fd9473b7f..d71f578377 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -2,6 +2,62 @@ import struct
import string
+class Qcow2StructMeta(type):
+
+ # Mapping from c types to python struct format
+ ctypes = {
+ 'u8': 'B',
+ 'u16': 'H',
+ 'u32': 'I',
+ 'u64': 'Q'
+ }
+
+ def __init__(self, name, bases, attrs):
+ if 'fields' in attrs:
+ self.fmt = '>' + ''.join(self.ctypes[f[0]] for f in self.fields)
+
+
+class Qcow2Struct(metaclass=Qcow2StructMeta):
+
+ """Qcow2Struct: base class for qcow2 data structures
+
+ Successors should define fields class variable, which is: list of tuples,
+ each of three elements:
+ - c-type (one of 'u32', 'u64')
+ - format (format_spec to use with .format() when dump or 'mask' to dump
+ bitmasks)
+ - field name
+ """
+
+ def __init__(self, fd=None, offset=None, data=None):
+ if data is None:
+ assert fd is not None
+ buf_size = struct.calcsize(self.fmt)
+ if offset is not None:
+ fd.seek(offset)
+ data = fd.read(buf_size)
+ else:
+ assert fd is None and offset is None
+
+ values = struct.unpack(self.fmt, data)
+ self.__dict__ = dict((field[2], values[i])
+ for i, field in enumerate(self.fields))
+
+ def dump(self):
+ for f in self.fields:
+ value = self.__dict__[f[2]]
+ if f[1] == 'mask':
+ bits = []
+ for bit in range(64):
+ if value & (1 << bit):
+ bits.append(bit)
+ value_str = str(bits)
+ else:
+ value_str = f[1].format(value)
+
+ print('{:<25} {}'.format(f[2], value_str))
+
+
class QcowHeaderExtension:
def __init__(self, magic, length, data):
@@ -18,16 +74,7 @@ class QcowHeaderExtension:
return QcowHeaderExtension(magic, len(data), data)
-# Mapping from c types to python struct format
-ctypes = {
- 'u8': 'B',
- 'u16': 'H',
- 'u32': 'I',
- 'u64': 'Q'
-}
-
-
-class QcowHeader:
+class QcowHeader(Qcow2Struct):
fields = (
# Version 2 header fields
@@ -53,18 +100,8 @@ class QcowHeader:
('u32', '{}', 'header_length'),
)
- fmt = '>' + ''.join(ctypes[f[0]] for f in fields)
-
def __init__(self, fd):
-
- buf_size = struct.calcsize(QcowHeader.fmt)
-
- fd.seek(0)
- buf = fd.read(buf_size)
-
- header = struct.unpack(QcowHeader.fmt, buf)
- self.__dict__ = dict((field[2], header[i])
- for i, field in enumerate(QcowHeader.fields))
+ super().__init__(fd=fd, offset=0)
self.set_defaults()
self.cluster_size = 1 << self.cluster_bits
@@ -132,20 +169,6 @@ class QcowHeader:
buf = buf[0:header_bytes-1]
fd.write(buf)
- def dump(self):
- for f in QcowHeader.fields:
- value = self.__dict__[f[2]]
- if f[1] == 'mask':
- bits = []
- for bit in range(64):
- if value & (1 << bit):
- bits.append(bit)
- value_str = str(bits)
- else:
- value_str = f[1].format(value)
-
- print(f'{f[2]:<25} {value_str}')
-
def dump_extensions(self):
for ex in self.extensions:
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 08/12] qcow2_format.py: add field-formatting class
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 13:27 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Allow formatter class in structure definition instead of hacking with
'mask'. This will simplify further introduction of new formatters.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 35 +++++++++++++++++++++---------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index d71f578377..ceb09bde5a 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -2,6 +2,25 @@ import struct
import string
+class Qcow2Field:
+
+ def __init__(self, value):
+ self.value = value
+
+ def __str__(self):
+ return str(self.value)
+
+
+class Flags64(Qcow2Field):
+
+ def __str__(self):
+ bits = []
+ for bit in range(64):
+ if self.value & (1 << bit):
+ bits.append(bit)
+ return str(bits)
+
+
class Qcow2StructMeta(type):
# Mapping from c types to python struct format
@@ -46,14 +65,10 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
def dump(self):
for f in self.fields:
value = self.__dict__[f[2]]
- if f[1] == 'mask':
- bits = []
- for bit in range(64):
- if value & (1 << bit):
- bits.append(bit)
- value_str = str(bits)
- else:
+ if isinstance(f[1], str):
value_str = f[1].format(value)
+ else:
+ value_str = str(f[1](value))
print('{:<25} {}'.format(f[2], value_str))
@@ -93,9 +108,9 @@ class QcowHeader(Qcow2Struct):
('u64', '{:#x}', 'snapshot_offset'),
# Version 3 header fields
- ('u64', 'mask', 'incompatible_features'),
- ('u64', 'mask', 'compatible_features'),
- ('u64', 'mask', 'autoclear_features'),
+ ('u64', Flags64, 'incompatible_features'),
+ ('u64', Flags64, 'compatible_features'),
+ ('u64', Flags64, 'autoclear_features'),
('u32', '{}', 'refcount_order'),
('u32', '{}', 'header_length'),
)
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 08/12] qcow2_format.py: add field-formatting class Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 14:43 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Obviously, for-loop body in dump_extensions should be the dump method
of extension.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index ceb09bde5a..ffc7c35b18 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -84,6 +84,17 @@ class QcowHeaderExtension:
self.length = length
self.data = data
+ def dump(self):
+ 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'{"magic":<25} {self.magic:#x}')
+ print(f'{"length":<25} {self.length}')
+ print(f'{"data":<25} {data}')
+
@classmethod
def create(cls, magic, data):
return QcowHeaderExtension(magic, len(data), data)
@@ -186,15 +197,6 @@ class QcowHeader(Qcow2Struct):
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 = f"'{ data.decode('ascii') }'"
- else:
- data = '<binary>'
-
print('Header extension:')
- print(f'{"magic":<25} {ex.magic:#x}')
- print(f'{"length":<25} {ex.length}')
- print(f'{"data":<25} {data}')
+ ex.dump()
print()
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 15:30 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Only two fields we can parse by generic code, but that is better than
nothing. Keep further refactoring of variable-length fields for another
day.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 51 ++++++++++++++++++++----------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index ffc7c35b18..4683b1e436 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -73,16 +73,39 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
print('{:<25} {}'.format(f[2], value_str))
-class QcowHeaderExtension:
+class QcowHeaderExtension(Qcow2Struct):
- def __init__(self, magic, length, data):
- if length % 8 != 0:
- padding = 8 - (length % 8)
- data += b'\0' * padding
+ fields = (
+ ('u32', '{:#x}', 'magic'),
+ ('u32', '{}', 'length')
+ # length bytes of data follows
+ # then padding to next multiply of 8
+ )
- self.magic = magic
- self.length = length
- self.data = data
+ def __init__(self, magic=None, length=None, data=None, fd=None):
+ """
+ Support both loading from fd and creation from user data.
+
+ This should be somehow refactored and functionality should be moved to
+ superclass (to allow creation of any qcow2 struct), but then, fields
+ of variable length (data here) should be supported in base class
+ somehow. So, it's a TODO. We'll see how to properly refactor this when
+ we have more qcow2 structures.
+ """
+ if fd is None:
+ assert all(v is not None for v in (magic, length, data))
+ self.magic = magic
+ self.length = length
+ if length % 8 != 0:
+ padding = 8 - (length % 8)
+ data += b'\0' * padding
+ self.data = data
+ 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
def dump(self):
data = self.data[:self.length]
@@ -91,8 +114,7 @@ class QcowHeaderExtension:
else:
data = '<binary>'
- print(f'{"magic":<25} {self.magic:#x}')
- print(f'{"length":<25} {self.length}')
+ super().dump()
print(f'{"data":<25} {data}')
@classmethod
@@ -158,14 +180,11 @@ class QcowHeader(Qcow2Struct):
end = self.cluster_size
while fd.tell() < end:
- (magic, length) = struct.unpack('>II', fd.read(8))
- if magic == 0:
+ ext = QcowHeaderExtension(fd=fd)
+ if ext.magic == 0:
break
else:
- padded = (length + 7) & ~7
- data = fd.read(padded)
- self.extensions.append(QcowHeaderExtension(magic, length,
- data))
+ self.extensions.append(ext)
def update_extensions(self, fd):
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 15:42 ` Andrey Shinkevich
2020-06-05 16:26 ` Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Suggested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/031.out | 22 +++++++++++-----------
tests/qemu-iotests/036.out | 4 ++--
tests/qemu-iotests/061.out | 14 +++++++-------
tests/qemu-iotests/qcow2_format.py | 17 ++++++++++++++++-
4 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
index 5a4beda6a2..e0225f4247 100644
--- a/tests/qemu-iotests/031.out
+++ b/tests/qemu-iotests/031.out
@@ -25,7 +25,7 @@ refcount_order 4
header_length 72
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -53,7 +53,7 @@ refcount_order 4
header_length 72
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -81,12 +81,12 @@ refcount_order 4
header_length 72
Header extension:
-magic 0xe2792aca
+magic 3799591626 (Backing format)
length 11
data 'host_device'
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -116,12 +116,12 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -149,12 +149,12 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -182,17 +182,17 @@ refcount_order 4
header_length 112
Header extension:
-magic 0xe2792aca
+magic 3799591626 (Backing format)
length 11
data 'host_device'
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index e409acf60e..6cf892123a 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -25,7 +25,7 @@ incompatible_features []
compatible_features []
autoclear_features [63]
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -37,7 +37,7 @@ incompatible_features []
compatible_features []
autoclear_features []
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index a51ad1b5ba..7afd2be9d9 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -25,7 +25,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -83,7 +83,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -139,7 +139,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -194,7 +194,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -263,7 +263,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -325,7 +325,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -354,7 +354,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 4683b1e436..caa190f7b1 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -21,6 +21,12 @@ class Flags64(Qcow2Field):
return str(bits)
+class Enum(Qcow2Field):
+
+ def __str__(self):
+ return f'{self.value} ({self.mapping.get(self.value, "<unknown>")})'
+
+
class Qcow2StructMeta(type):
# Mapping from c types to python struct format
@@ -75,8 +81,17 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
class QcowHeaderExtension(Qcow2Struct):
+ class Magic(Enum):
+ mapping = {
+ 0xE2792ACA: 'Backing format',
+ 0x6803f857: 'Feature table',
+ 0x0537be77: 'Crypto header',
+ 0x23852875: 'Bitmaps',
+ 0x44415441: 'Data file'
+ }
+
fields = (
- ('u32', '{:#x}', 'magic'),
+ ('u32', Magic, 'magic'),
('u32', '{}', 'length')
# length bytes of data follows
# then padding to next multiply of 8
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics Vladimir Sementsov-Ogievskiy
@ 2020-06-04 17:41 ` Vladimir Sementsov-Ogievskiy
2020-06-05 16:06 ` Andrey Shinkevich
2020-06-04 19:14 ` [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata no-reply
2020-06-05 20:50 ` Eric Blake
13 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-04 17:41 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, andrey.shinkevich, den
Add class for bitmap extension and dump its fields. Further work is to
dump bitmap directory.
Test new functionality inside 291 iotest.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/291 | 4 +++
tests/qemu-iotests/291.out | 33 +++++++++++++++++++++++
tests/qemu-iotests/qcow2_format.py | 42 +++++++++++++++++++++++-------
3 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
index 3ca83b9cd1..e0cffc7cb1 100755
--- a/tests/qemu-iotests/291
+++ b/tests/qemu-iotests/291
@@ -62,6 +62,8 @@ $QEMU_IO -c 'w 1M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
$QEMU_IMG bitmap --disable -f $IMGFMT "$TEST_IMG" b1
$QEMU_IMG bitmap --enable -f $IMGFMT "$TEST_IMG" b2
$QEMU_IO -c 'w 2M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
+echo "Check resulting qcow2 header extensions:"
+$PYTHON qcow2.py "$TEST_IMG" dump-header-exts
echo
echo "=== Bitmap preservation not possible to non-qcow2 ==="
@@ -88,6 +90,8 @@ $QEMU_IMG bitmap --merge tmp -f $IMGFMT "$TEST_IMG" b0
$QEMU_IMG bitmap --remove --image-opts \
driver=$IMGFMT,file.driver=file,file.filename="$TEST_IMG" tmp
$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+echo "Check resulting qcow2 header extensions:"
+$PYTHON qcow2.py "$TEST_IMG" dump-header-exts
echo
echo "=== Check bitmap contents ==="
diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
index 8c62017567..1d4f9cd96d 100644
--- a/tests/qemu-iotests/291.out
+++ b/tests/qemu-iotests/291.out
@@ -14,6 +14,25 @@ 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)
+Check resulting qcow2 header extensions:
+Header extension:
+magic 3799591626 (Backing format)
+length 5
+data 'qcow2'
+
+Header extension:
+magic 1745090647 (Feature table)
+length 336
+data <binary>
+
+Header extension:
+magic 595929205 (Bitmaps)
+length 24
+nb_bitmaps 2
+reserved32 0
+bitmap_directory_size 0x40
+bitmap_directory_offset 0x510000
+
=== Bitmap preservation not possible to non-qcow2 ===
@@ -65,6 +84,20 @@ Format specific information:
granularity: 65536
refcount bits: 16
corrupt: false
+Check resulting qcow2 header extensions:
+Header extension:
+magic 1745090647 (Feature table)
+length 336
+data <binary>
+
+Header extension:
+magic 595929205 (Bitmaps)
+length 24
+nb_bitmaps 3
+reserved32 0
+bitmap_directory_size 0x60
+bitmap_directory_offset 0x520000
+
=== Check bitmap contents ===
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index caa190f7b1..c2c636b442 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -79,6 +79,19 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
print('{:<25} {}'.format(f[2], value_str))
+class Qcow2BitmapExt(Qcow2Struct):
+
+ fields = (
+ ('u32', '{}', 'nb_bitmaps'),
+ ('u32', '{}', 'reserved32'),
+ ('u64', '{:#x}', 'bitmap_directory_size'),
+ ('u64', '{:#x}', 'bitmap_directory_offset')
+ )
+
+
+QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
+
+
class QcowHeaderExtension(Qcow2Struct):
class Magic(Enum):
@@ -86,7 +99,7 @@ class QcowHeaderExtension(Qcow2Struct):
0xE2792ACA: 'Backing format',
0x6803f857: 'Feature table',
0x0537be77: 'Crypto header',
- 0x23852875: 'Bitmaps',
+ QCOW2_EXT_MAGIC_BITMAPS: 'Bitmaps',
0x44415441: 'Data file'
}
@@ -104,8 +117,11 @@ class QcowHeaderExtension(Qcow2Struct):
This should be somehow refactored and functionality should be moved to
superclass (to allow creation of any qcow2 struct), but then, fields
of variable length (data here) should be supported in base class
- somehow. So, it's a TODO. We'll see how to properly refactor this when
- we have more qcow2 structures.
+ somehow. Note also, that we probably want to parse different
+ extensions. Should they be subclasses of this class, or how to do it
+ better? Should it be something like QAPI union with discriminator field
+ (magic here). So, it's a TODO. We'll see how to properly refactor this
+ when we have more qcow2 structures.
"""
if fd is None:
assert all(v is not None for v in (magic, length, data))
@@ -122,15 +138,23 @@ class QcowHeaderExtension(Qcow2Struct):
self.data = fd.read(padded)
assert self.data is not None
- def dump(self):
- data = self.data[:self.length]
- if all(c in string.printable.encode('ascii') for c in data):
- data = f"'{ data.decode('ascii') }'"
+ if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
+ self.obj = Qcow2BitmapExt(data=self.data)
else:
- data = '<binary>'
+ self.obj = None
+ def dump(self):
super().dump()
- print(f'{"data":<25} {data}')
+
+ 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}')
+ else:
+ self.obj.dump()
@classmethod
def create(cls, magic, data):
--
2.21.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2020-06-04 17:41 ` [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension Vladimir Sementsov-Ogievskiy
@ 2020-06-04 19:14 ` no-reply
2020-06-05 10:02 ` Vladimir Sementsov-Ogievskiy
2020-06-05 20:50 ` Eric Blake
13 siblings, 1 reply; 39+ messages in thread
From: no-reply @ 2020-06-04 19:14 UTC (permalink / raw)
To: vsementsov
Cc: kwolf, vsementsov, qemu-block, qemu-devel, mreitz, den,
andrey.shinkevich
Patchew URL: https://patchew.org/QEMU/20200604174135.11042-1-vsementsov@virtuozzo.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20200604174135.11042-1-vsementsov@virtuozzo.com
Subject: [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
ff3d0e0 qcow2_format.py: dump bitmaps header extension
1ca4b4b qcow2: QcowHeaderExtension print names for extension magics
4894f85 qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct
5d2d3ea qcow2_format.py: QcowHeaderExtension: add dump method
7848f52 qcow2_format.py: add field-formatting class
4f0473f qcow2_format.py: separate generic functionality of structure classes
78964d2 qcow2_format.py: use strings to specify c-type of struct fields
492b753 qcow2_format.py: use modern string formatting
7fef737 qcow2_format.py: use tuples instead of lists for fields
8fae4ab qcow2_format.py: drop new line printing at end of dump()
c0b4e4b qcow2.py: move qcow2 format classes to separate module
32668ba qcow2.py: python style fixes
=== OUTPUT BEGIN ===
1/12 Checking commit 32668ba94cf1 (qcow2.py: python style fixes)
ERROR: line over 90 characters
#219: FILE: tests/qemu-iotests/qcow2.py:256:
+ ['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'],
WARNING: line over 80 characters
#220: FILE: tests/qemu-iotests/qcow2.py:257:
+ ['dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions'],
WARNING: line over 80 characters
#221: FILE: tests/qemu-iotests/qcow2.py:258:
+ ['set-header', cmd_set_header, 2, 'Set a field in the header'],
WARNING: line over 80 characters
#222: FILE: tests/qemu-iotests/qcow2.py:259:
+ ['add-header-ext', cmd_add_header_ext, 2, 'Add a header extension'],
ERROR: line over 90 characters
#223: FILE: tests/qemu-iotests/qcow2.py:260:
+ ['add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin'],
WARNING: line over 80 characters
#224: FILE: tests/qemu-iotests/qcow2.py:261:
+ ['del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension'],
total: 2 errors, 4 warnings, 217 lines checked
Patch 1/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/12 Checking commit c0b4e4bcc692 (qcow2.py: move qcow2 format classes to separate module)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#186:
new file mode 100644
total: 0 errors, 1 warnings, 324 lines checked
Patch 2/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/12 Checking commit 8fae4ab6b058 (qcow2_format.py: drop new line printing at end of dump())
4/12 Checking commit 7fef73761e63 (qcow2_format.py: use tuples instead of lists for fields)
5/12 Checking commit 492b753d310e (qcow2_format.py: use modern string formatting)
6/12 Checking commit 78964d206cab (qcow2_format.py: use strings to specify c-type of struct fields)
7/12 Checking commit 4f0473f3cf7f (qcow2_format.py: separate generic functionality of structure classes)
8/12 Checking commit 7848f5283863 (qcow2_format.py: add field-formatting class)
9/12 Checking commit 5d2d3ea44d93 (qcow2_format.py: QcowHeaderExtension: add dump method)
10/12 Checking commit 4894f859e63d (qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct)
11/12 Checking commit 1ca4b4b795d8 (qcow2: QcowHeaderExtension print names for extension magics)
12/12 Checking commit ff3d0e0f471a (qcow2_format.py: dump bitmaps header extension)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200604174135.11042-1-vsementsov@virtuozzo.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
2020-06-04 19:14 ` [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata no-reply
@ 2020-06-05 10:02 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-05 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, andrey.shinkevich, den, qemu-block, mreitz
04.06.2020 22:14, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200604174135.11042-1-vsementsov@virtuozzo.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Message-id: 20200604174135.11042-1-vsementsov@virtuozzo.com
> Subject: [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
> Type: series
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Switched to a new branch 'test'
> ff3d0e0 qcow2_format.py: dump bitmaps header extension
> 1ca4b4b qcow2: QcowHeaderExtension print names for extension magics
> 4894f85 qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct
> 5d2d3ea qcow2_format.py: QcowHeaderExtension: add dump method
> 7848f52 qcow2_format.py: add field-formatting class
> 4f0473f qcow2_format.py: separate generic functionality of structure classes
> 78964d2 qcow2_format.py: use strings to specify c-type of struct fields
> 492b753 qcow2_format.py: use modern string formatting
> 7fef737 qcow2_format.py: use tuples instead of lists for fields
> 8fae4ab qcow2_format.py: drop new line printing at end of dump()
> c0b4e4b qcow2.py: move qcow2 format classes to separate module
> 32668ba qcow2.py: python style fixes
>
> === OUTPUT BEGIN ===
> 1/12 Checking commit 32668ba94cf1 (qcow2.py: python style fixes)
> ERROR: line over 90 characters
> #219: FILE: tests/qemu-iotests/qcow2.py:256:
> + ['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'],
>
> WARNING: line over 80 characters
> #220: FILE: tests/qemu-iotests/qcow2.py:257:
> + ['dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions'],
>
> WARNING: line over 80 characters
> #221: FILE: tests/qemu-iotests/qcow2.py:258:
> + ['set-header', cmd_set_header, 2, 'Set a field in the header'],
>
> WARNING: line over 80 characters
> #222: FILE: tests/qemu-iotests/qcow2.py:259:
> + ['add-header-ext', cmd_add_header_ext, 2, 'Add a header extension'],
>
> ERROR: line over 90 characters
> #223: FILE: tests/qemu-iotests/qcow2.py:260:
> + ['add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin'],
>
> WARNING: line over 80 characters
> #224: FILE: tests/qemu-iotests/qcow2.py:261:
> + ['del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension'],
>
> total: 2 errors, 4 warnings, 217 lines checked
>
> Patch 1/12 has style problems, please review. If any of these errors
Preexisting. And it will look worse if wrap lines.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 01/12] qcow2.py: python style fixes
2020-06-04 17:41 ` [PATCH v4 01/12] qcow2.py: python style fixes Vladimir Sementsov-Ogievskiy
@ 2020-06-05 10:23 ` Andrey Shinkevich
2020-06-05 19:43 ` Eric Blake
1 sibling, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 10:23 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 01/12] qcow2.py: python style fixes
Fix flake8 complains. Leave the only chunk of lines over 79 characters:
initialization of cmds variable. Leave it for another day, when it
should be refactored to utilize argparse instead of hand-written
parsing.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 92 +++++++++++++++++++++----------------
1 file changed, 53 insertions(+), 39 deletions(-)
[-- Attachment #2: Type: text/html, Size: 2009 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
2020-06-04 17:41 ` [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module Vladimir Sementsov-Ogievskiy
@ 2020-06-05 10:51 ` Andrey Shinkevich
2020-06-05 20:14 ` Eric Blake
1 sibling, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 10:51 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
We are going to enhance qcow2 format parsing by adding more structure
classes. Let's split format parsing from utility code.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 161 +----------------------------
tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
2 files changed, 161 insertions(+), 157 deletions(-)
create mode 100644 tests/qemu-iotests/qcow2_format.py
[-- Attachment #2: Type: text/html, Size: 2153 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump()
2020-06-04 17:41 ` [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump() Vladimir Sementsov-Ogievskiy
@ 2020-06-05 10:54 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 10:54 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump()
This will simplify further conversion. To compensate, print this empty
line directly in cmd_dump_header().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2.py | 1 +
tests/qemu-iotests/qcow2_format.py | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index d9c41668fd..79db81b040 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -10,6 +10,7 @@ from qcow2_format import (
def cmd_dump_header(fd):
h = QcowHeader(fd)
h.dump()
+ print()
h.dump_extensions()
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index c7270a0a6e..99e5248e73 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -139,7 +139,6 @@ class QcowHeader:
value_str = f[1] % value
print("%-25s" % f[2], value_str)
- print("")
def dump_extensions(self):
for ex in self.extensions:
--
2.21.0
[-- Attachment #2: Type: text/html, Size: 3185 bytes --]
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
2020-06-04 17:41 ` [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields Vladimir Sementsov-Ogievskiy
@ 2020-06-05 11:20 ` Andrey Shinkevich
2020-06-05 20:16 ` Eric Blake
1 sibling, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 11:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
No need in lists: it's a constant variable.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
1 file changed, 20 insertions(+), 20 deletions(-)
[-- Attachment #2: Type: text/html, Size: 1828 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 05/12] qcow2_format.py: use modern string formatting
2020-06-04 17:41 ` [PATCH v4 05/12] qcow2_format.py: use modern string formatting Vladimir Sementsov-Ogievskiy
@ 2020-06-05 11:27 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 11:27 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 05/12] qcow2_format.py: use modern string formatting
Use .format and f-strings instead of old %style. Also, the file uses
both '' and "" quotes, for consistency let's use '', except for cases
when we need '' inside the string (use "" to avoid extra escaping).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 54 +++++++++++++++---------------
1 file changed, 27 insertions(+), 27 deletions(-)
[-- Attachment #2: Type: text/html, Size: 2011 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields
2020-06-04 17:41 ` [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields Vladimir Sementsov-Ogievskiy
@ 2020-06-05 11:36 ` Andrey Shinkevich
2020-06-05 20:19 ` Eric Blake
1 sibling, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 11:36 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields
We are going to move field-parsing to super-class, this will be simpler
with simple string specifiers instead of variables.
For some reason python doesn't allow to define ctypes in class too, as
well as fields: it's not available than in 'for' operator. Don't worry:
ctypes will be moved to metaclass soon.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 50 +++++++++++++++++-------------
1 file changed, 28 insertions(+), 22 deletions(-)
[-- Attachment #2: Type: text/html, Size: 2133 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes
2020-06-04 17:41 ` [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes Vladimir Sementsov-Ogievskiy
@ 2020-06-05 12:29 ` Andrey Shinkevich
2020-06-05 12:37 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 12:29 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]
Two more int types in the comment?
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes
We are going to introduce more Qcow2 structure types, defined like
QcowHeader. Move generic functionality into base class to be reused for
further structure classes.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 93 +++++++++++++++++++-----------
1 file changed, 58 insertions(+), 35 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 1fd9473b7f..d71f578377 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -2,6 +2,62 @@ import struct
+
+class Qcow2Struct(metaclass=Qcow2StructMeta):
+
+ """Qcow2Struct: base class for qcow2 data structures
+
+ Successors should define fields class variable, which is: list of tuples,
+ each of three elements:
+ - c-type (one of 'u32', 'u64')
u8, u16 ?
[-- Attachment #2: Type: text/html, Size: 3369 bytes --]
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes
2020-06-05 12:29 ` Andrey Shinkevich
@ 2020-06-05 12:37 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-05 12:37 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-block; +Cc: kwolf, Denis Lunev, qemu-devel, mreitz
05.06.2020 15:29, Andrey Shinkevich wrote:
> Two more int types in the comment?
>
> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> *From:* Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> *Sent:* Thursday, June 4, 2020 8:41 PM
> *To:* qemu-block@nongnu.org <qemu-block@nongnu.org>
> *Cc:* qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> *Subject:* [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes
> We are going to introduce more Qcow2 structure types, defined like
> QcowHeader. Move generic functionality into base class to be reused for
> further structure classes.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2_format.py | 93 +++++++++++++++++++-----------
> 1 file changed, 58 insertions(+), 35 deletions(-)
>
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index 1fd9473b7f..d71f578377 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -2,6 +2,62 @@ import struct
>
> +
> +class Qcow2Struct(metaclass=Qcow2StructMeta):
> +
> + """Qcow2Struct: base class for qcow2 data structures
> +
> + Successors should define fields class variable, which is: list of tuples,
> + each of three elements:
> + - c-type (one of 'u32', 'u64')
>
> u8, u16 ?
yes
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 08/12] qcow2_format.py: add field-formatting class
2020-06-04 17:41 ` [PATCH v4 08/12] qcow2_format.py: add field-formatting class Vladimir Sementsov-Ogievskiy
@ 2020-06-05 13:27 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 13:27 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 979 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 08/12] qcow2_format.py: add field-formatting class
Allow formatter class in structure definition instead of hacking with
'mask'. This will simplify further introduction of new formatters.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 35 +++++++++++++++++++++---------
1 file changed, 25 insertions(+), 10 deletions(-)
[-- Attachment #2: Type: text/html, Size: 1936 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method
2020-06-04 17:41 ` [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method Vladimir Sementsov-Ogievskiy
@ 2020-06-05 14:43 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 14:43 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 928 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method
Obviously, for-loop body in dump_extensions should be the dump method
of extension.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
[-- Attachment #2: Type: text/html, Size: 1849 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct
2020-06-04 17:41 ` [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct Vladimir Sementsov-Ogievskiy
@ 2020-06-05 15:30 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 15:30 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]
I'd add a comment that correct current file offset is the responsibility of a caller.
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct
Only two fields we can parse by generic code, but that is better than
nothing. Keep further refactoring of variable-length fields for another
day.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 51 ++++++++++++++++++++----------
1 file changed, 35 insertions(+), 16 deletions(-)
[-- Attachment #2: Type: text/html, Size: 2098 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics
2020-06-04 17:41 ` [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics Vladimir Sementsov-Ogievskiy
@ 2020-06-05 15:42 ` Andrey Shinkevich
2020-06-05 16:26 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 15:42 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 7891 bytes --]
s/0xE2792ACA/0xe2792aca/
(as suggested by Eric)
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics
Suggested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/031.out | 22 +++++++++++-----------
tests/qemu-iotests/036.out | 4 ++--
tests/qemu-iotests/061.out | 14 +++++++-------
tests/qemu-iotests/qcow2_format.py | 17 ++++++++++++++++-
4 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
index 5a4beda6a2..e0225f4247 100644
--- a/tests/qemu-iotests/031.out
+++ b/tests/qemu-iotests/031.out
@@ -25,7 +25,7 @@ refcount_order 4
header_length 72
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -53,7 +53,7 @@ refcount_order 4
header_length 72
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -81,12 +81,12 @@ refcount_order 4
header_length 72
Header extension:
-magic 0xe2792aca
+magic 3799591626 (Backing format)
length 11
data 'host_device'
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -116,12 +116,12 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -149,12 +149,12 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
@@ -182,17 +182,17 @@ refcount_order 4
header_length 112
Header extension:
-magic 0xe2792aca
+magic 3799591626 (Backing format)
length 11
data 'host_device'
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
Header extension:
-magic 0x12345678
+magic 305419896 (<unknown>)
length 31
data 'This is a test header extension'
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index e409acf60e..6cf892123a 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -25,7 +25,7 @@ incompatible_features []
compatible_features []
autoclear_features [63]
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -37,7 +37,7 @@ incompatible_features []
compatible_features []
autoclear_features []
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index a51ad1b5ba..7afd2be9d9 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -25,7 +25,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -83,7 +83,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -139,7 +139,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -194,7 +194,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -263,7 +263,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -325,7 +325,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
@@ -354,7 +354,7 @@ refcount_order 4
header_length 112
Header extension:
-magic 0x6803f857
+magic 1745090647 (Feature table)
length 336
data <binary>
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 4683b1e436..caa190f7b1 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -21,6 +21,12 @@ class Flags64(Qcow2Field):
return str(bits)
+class Enum(Qcow2Field):
+
+ def __str__(self):
+ return f'{self.value} ({self.mapping.get(self.value, "<unknown>")})'
+
+
class Qcow2StructMeta(type):
# Mapping from c types to python struct format
@@ -75,8 +81,17 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
class QcowHeaderExtension(Qcow2Struct):
+ class Magic(Enum):
+ mapping = {
+ 0xE2792ACA: 'Backing format',
+ 0x6803f857: 'Feature table',
+ 0x0537be77: 'Crypto header',
+ 0x23852875: 'Bitmaps',
+ 0x44415441: 'Data file'
+ }
+
fields = (
- ('u32', '{:#x}', 'magic'),
+ ('u32', Magic, 'magic'),
('u32', '{}', 'length')
# length bytes of data follows
# then padding to next multiply of 8
--
2.21.0
[-- Attachment #2: Type: text/html, Size: 22038 bytes --]
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension
2020-06-04 17:41 ` [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension Vladimir Sementsov-Ogievskiy
@ 2020-06-05 16:06 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 16:06 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension
Add class for bitmap extension and dump its fields. Further work is to
dump bitmap directory.
Test new functionality inside 291 iotest.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/291 | 4 +++
tests/qemu-iotests/291.out | 33 +++++++++++++++++++++++
tests/qemu-iotests/qcow2_format.py | 42 +++++++++++++++++++++++-------
3 files changed, 70 insertions(+), 9 deletions(-)
[-- Attachment #2: Type: text/html, Size: 2296 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics
2020-06-04 17:41 ` [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics Vladimir Sementsov-Ogievskiy
2020-06-05 15:42 ` Andrey Shinkevich
@ 2020-06-05 16:26 ` Vladimir Sementsov-Ogievskiy
2020-06-05 16:41 ` Andrey Shinkevich
1 sibling, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-05 16:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz, andrey.shinkevich, den
04.06.2020 20:41, Vladimir Sementsov-Ogievskiy wrote:
> Suggested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/031.out | 22 +++++++++++-----------
> tests/qemu-iotests/036.out | 4 ++--
> tests/qemu-iotests/061.out | 14 +++++++-------
> tests/qemu-iotests/qcow2_format.py | 17 ++++++++++++++++-
> 4 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
> index 5a4beda6a2..e0225f4247 100644
> --- a/tests/qemu-iotests/031.out
> +++ b/tests/qemu-iotests/031.out
> @@ -25,7 +25,7 @@ refcount_order 4
> header_length 72
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
Oops, I've broken hex number. I think it should be kept hex, even with name.
> length 31
> data 'This is a test header extension'
>
> @@ -53,7 +53,7 @@ refcount_order 4
> header_length 72
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
> length 31
> data 'This is a test header extension'
>
> @@ -81,12 +81,12 @@ refcount_order 4
> header_length 72
>
> Header extension:
> -magic 0xe2792aca
> +magic 3799591626 (Backing format)
> length 11
> data 'host_device'
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
> length 31
> data 'This is a test header extension'
>
> @@ -116,12 +116,12 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
> length 31
> data 'This is a test header extension'
>
> @@ -149,12 +149,12 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
> length 31
> data 'This is a test header extension'
>
> @@ -182,17 +182,17 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0xe2792aca
> +magic 3799591626 (Backing format)
> length 11
> data 'host_device'
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
> length 31
> data 'This is a test header extension'
>
> diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
> index e409acf60e..6cf892123a 100644
> --- a/tests/qemu-iotests/036.out
> +++ b/tests/qemu-iotests/036.out
> @@ -25,7 +25,7 @@ incompatible_features []
> compatible_features []
> autoclear_features [63]
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -37,7 +37,7 @@ incompatible_features []
> compatible_features []
> autoclear_features []
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
> index a51ad1b5ba..7afd2be9d9 100644
> --- a/tests/qemu-iotests/061.out
> +++ b/tests/qemu-iotests/061.out
> @@ -25,7 +25,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -83,7 +83,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -139,7 +139,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -194,7 +194,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -263,7 +263,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -325,7 +325,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> @@ -354,7 +354,7 @@ refcount_order 4
> header_length 112
>
> Header extension:
> -magic 0x6803f857
> +magic 1745090647 (Feature table)
> length 336
> data <binary>
>
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index 4683b1e436..caa190f7b1 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -21,6 +21,12 @@ class Flags64(Qcow2Field):
> return str(bits)
>
>
> +class Enum(Qcow2Field):
> +
> + def __str__(self):
> + return f'{self.value} ({self.mapping.get(self.value, "<unknown>")})'
> +
> +
> class Qcow2StructMeta(type):
>
> # Mapping from c types to python struct format
> @@ -75,8 +81,17 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
>
> class QcowHeaderExtension(Qcow2Struct):
>
> + class Magic(Enum):
> + mapping = {
> + 0xE2792ACA: 'Backing format',
> + 0x6803f857: 'Feature table',
> + 0x0537be77: 'Crypto header',
> + 0x23852875: 'Bitmaps',
> + 0x44415441: 'Data file'
> + }
> +
> fields = (
> - ('u32', '{:#x}', 'magic'),
> + ('u32', Magic, 'magic'),
> ('u32', '{}', 'length')
> # length bytes of data follows
> # then padding to next multiply of 8
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics
2020-06-05 16:26 ` Vladimir Sementsov-Ogievskiy
@ 2020-06-05 16:41 ` Andrey Shinkevich
0 siblings, 0 replies; 39+ messages in thread
From: Andrey Shinkevich @ 2020-06-05 16:41 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, Denis Lunev, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]
Yes, but hex(int) returns str.
Andrey
________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Friday, June 5, 2020 7:26 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: Re: [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics
04.06.2020 20:41, Vladimir Sementsov-Ogievskiy wrote:
> Suggested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/031.out | 22 +++++++++++-----------
> tests/qemu-iotests/036.out | 4 ++--
> tests/qemu-iotests/061.out | 14 +++++++-------
> tests/qemu-iotests/qcow2_format.py | 17 ++++++++++++++++-
> 4 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
> index 5a4beda6a2..e0225f4247 100644
> --- a/tests/qemu-iotests/031.out
> +++ b/tests/qemu-iotests/031.out
> @@ -25,7 +25,7 @@ refcount_order 4
> header_length 72
>
> Header extension:
> -magic 0x12345678
> +magic 305419896 (<unknown>)
Oops, I've broken hex number. I think it should be kept hex, even with name.
[-- Attachment #2: Type: text/html, Size: 2979 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 01/12] qcow2.py: python style fixes
2020-06-04 17:41 ` [PATCH v4 01/12] qcow2.py: python style fixes Vladimir Sementsov-Ogievskiy
2020-06-05 10:23 ` Andrey Shinkevich
@ 2020-06-05 19:43 ` Eric Blake
2020-06-06 7:00 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 39+ messages in thread
From: Eric Blake @ 2020-06-05 19:43 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> Fix flake8 complains. Leave the only chunk of lines over 79 characters:
complaints
> initialization of cmds variable. Leave it for another day, when it
> should be refactored to utilize argparse instead of hand-written
> parsing.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 92 +++++++++++++++++++++----------------
> 1 file changed, 53 insertions(+), 39 deletions(-)
>
> cmds = [
> - [ 'dump-header', cmd_dump_header, 0, 'Dump image header and header extensions' ],
> - [ 'dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions' ],
> - [ 'set-header', cmd_set_header, 2, 'Set a field in the header'],
> - [ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header extension' ],
> - [ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin' ],
> - [ 'del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension' ],
> - [ 'set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'],
> + ['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'],
I know you mentioned argparse as a later refactoring, but is it worth
reflowing the table in the meantime?
['dump-header', cmd_dump_header, 0,
'Dump image header and header extensions'],
[...
'...'],
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
2020-06-04 17:41 ` [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module Vladimir Sementsov-Ogievskiy
2020-06-05 10:51 ` Andrey Shinkevich
@ 2020-06-05 20:14 ` Eric Blake
2020-06-06 7:03 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 39+ messages in thread
From: Eric Blake @ 2020-06-05 20:14 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to enhance qcow2 format parsing by adding more structure
> classes. Let's split format parsing from utility code.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2.py | 161 +----------------------------
> tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
> 2 files changed, 161 insertions(+), 157 deletions(-)
> create mode 100644 tests/qemu-iotests/qcow2_format.py
>
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 539f5a186b..d9c41668fd 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -1,163 +1,10 @@
> #!/usr/bin/env python3
> -
> import sys
Pre-existing: no copyright blurb on the old file...
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -0,0 +1,157 @@
> +import struct
> +import string
> +
It would be nice to fix that, and have one on the new file as well.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
2020-06-04 17:41 ` [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields Vladimir Sementsov-Ogievskiy
2020-06-05 11:20 ` Andrey Shinkevich
@ 2020-06-05 20:16 ` Eric Blake
2020-06-06 7:07 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 39+ messages in thread
From: Eric Blake @ 2020-06-05 20:16 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> No need in lists: it's a constant variable.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
>
> # Version 3 header fields
> - [uint64_t, 'mask', 'incompatible_features'],
> - [uint64_t, 'mask', 'compatible_features'],
> - [uint64_t, 'mask', 'autoclear_features'],
> - [uint32_t, '%d', 'refcount_order'],
> - [uint32_t, '%d', 'header_length'],
> - ]
> + (uint64_t, 'mask', 'incompatible_features'),
> + (uint64_t, 'mask', 'compatible_features'),
> + (uint64_t, 'mask', 'autoclear_features'),
> + (uint32_t, '%d', 'refcount_order'),
> + (uint32_t, '%d', 'header_length'),
> + )
Not for this patch, but noticing it since we're here: should this be
taught to list the optional compression_type field that we recently
added after header_length?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields
2020-06-04 17:41 ` [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields Vladimir Sementsov-Ogievskiy
2020-06-05 11:36 ` Andrey Shinkevich
@ 2020-06-05 20:19 ` Eric Blake
1 sibling, 0 replies; 39+ messages in thread
From: Eric Blake @ 2020-06-05 20:19 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to move field-parsing to super-class, this will be simpler
> with simple string specifiers instead of variables.
>
> For some reason python doesn't allow to define ctypes in class too, as
> well as fields: it's not available than in 'for' operator. Don't worry:
> ctypes will be moved to metaclass soon.
Grammar suggestion:
For some reason, python doesn't allow the definition of ctypes in the
class alongside fields: it would not be available then for use by the
'for' operator.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2_format.py | 50 +++++++++++++++++-------------
> 1 file changed, 28 insertions(+), 22 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
` (12 preceding siblings ...)
2020-06-04 19:14 ` [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata no-reply
@ 2020-06-05 20:50 ` Eric Blake
13 siblings, 0 replies; 39+ messages in thread
From: Eric Blake @ 2020-06-05 20:50 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> Here is my suggestion to substitute only first three patches :) of
> Andrey's [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata
>
> so, I called it v4 for convenience.
>
> What is here:
> 1. First, update code style
> 2. Next, try to refactor in a manner which will make adding new data
> structures simple (look at Qcow2BitmapExt class in last patch)
>
> I think, next step is to add type hints. Then add more structures.
> And, anyway, at some point we should move it into python/ directory (at
> least qcow2_format.py lib)
My python reviewing skills are weak, but in general this series makes
sense. I had enough comments that you're probably better off spinning a
v5, but it looks like it is probably close enough that I can include
this in my next bitmaps pull request, and we can work on rebasing the
remainder of Andrey's patches on top of it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 01/12] qcow2.py: python style fixes
2020-06-05 19:43 ` Eric Blake
@ 2020-06-06 7:00 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-06 7:00 UTC (permalink / raw)
To: Eric Blake, qemu-block; +Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
05.06.2020 22:43, Eric Blake wrote:
> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>> Fix flake8 complains. Leave the only chunk of lines over 79 characters:
>
> complaints
>
>> initialization of cmds variable. Leave it for another day, when it
>> should be refactored to utilize argparse instead of hand-written
>> parsing.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> tests/qemu-iotests/qcow2.py | 92 +++++++++++++++++++++----------------
>> 1 file changed, 53 insertions(+), 39 deletions(-)
>>
>
>> cmds = [
>> - [ 'dump-header', cmd_dump_header, 0, 'Dump image header and header extensions' ],
>> - [ 'dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions' ],
>> - [ 'set-header', cmd_set_header, 2, 'Set a field in the header'],
>> - [ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header extension' ],
>> - [ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin' ],
>> - [ 'del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension' ],
>> - [ 'set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'],
>> + ['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'],
>
> I know you mentioned argparse as a later refactoring, but is it worth reflowing the table in the meantime?
>
> ['dump-header', cmd_dump_header, 0,
> 'Dump image header and header extensions'],
> [...
> '...'],
>
Yes, now I think it worth doing, patchew emails never makes series more beautiful.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
2020-06-05 20:14 ` Eric Blake
@ 2020-06-06 7:03 ` Vladimir Sementsov-Ogievskiy
2020-06-08 15:20 ` Eric Blake
0 siblings, 1 reply; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-06 7:03 UTC (permalink / raw)
To: Eric Blake, qemu-block; +Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
05.06.2020 23:14, Eric Blake wrote:
> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>> We are going to enhance qcow2 format parsing by adding more structure
>> classes. Let's split format parsing from utility code.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> tests/qemu-iotests/qcow2.py | 161 +----------------------------
>> tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
>> 2 files changed, 161 insertions(+), 157 deletions(-)
>> create mode 100644 tests/qemu-iotests/qcow2_format.py
>>
>> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
>> index 539f5a186b..d9c41668fd 100755
>> --- a/tests/qemu-iotests/qcow2.py
>> +++ b/tests/qemu-iotests/qcow2.py
>> @@ -1,163 +1,10 @@
>> #!/usr/bin/env python3
>> -
>> import sys
>
> Pre-existing: no copyright blurb on the old file...
>
>> +++ b/tests/qemu-iotests/qcow2_format.py
>> @@ -0,0 +1,157 @@
>> +import struct
>> +import string
>> +
>
> It would be nice to fix that, and have one on the new file as well.
>
Yes, I thought about this.. But what to add? Seems OK to add Virtuozzo copyright and common "This program is free software; you can redist... GPL ..." header to qcow2_format.py, as we are going to rewrite and improve it a lot. But what to add to qcow2.py, to the part which is kept mostly unchanged?
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
2020-06-05 20:16 ` Eric Blake
@ 2020-06-06 7:07 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 39+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-06-06 7:07 UTC (permalink / raw)
To: Eric Blake, qemu-block; +Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
05.06.2020 23:16, Eric Blake wrote:
> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>> No need in lists: it's a constant variable.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
>> 1 file changed, 20 insertions(+), 20 deletions(-)
>>
>
>> # Version 3 header fields
>> - [uint64_t, 'mask', 'incompatible_features'],
>> - [uint64_t, 'mask', 'compatible_features'],
>> - [uint64_t, 'mask', 'autoclear_features'],
>> - [uint32_t, '%d', 'refcount_order'],
>> - [uint32_t, '%d', 'header_length'],
>> - ]
>> + (uint64_t, 'mask', 'incompatible_features'),
>> + (uint64_t, 'mask', 'compatible_features'),
>> + (uint64_t, 'mask', 'autoclear_features'),
>> + (uint32_t, '%d', 'refcount_order'),
>> + (uint32_t, '%d', 'header_length'),
>> + )
>
> Not for this patch, but noticing it since we're here: should this be taught to list the optional compression_type field that we recently added after header_length?
>
Good note. And we can use it in corresponding iotest about compression_type. We just should carefully check, how header_length field is handled and fix if needed. I think, Andrey or I will make an additional patch.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
2020-06-06 7:03 ` Vladimir Sementsov-Ogievskiy
@ 2020-06-08 15:20 ` Eric Blake
0 siblings, 0 replies; 39+ messages in thread
From: Eric Blake @ 2020-06-08 15:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, den, andrey.shinkevich, qemu-devel, mreitz
On 6/6/20 2:03 AM, Vladimir Sementsov-Ogievskiy wrote:
> 05.06.2020 23:14, Eric Blake wrote:
>> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> We are going to enhance qcow2 format parsing by adding more structure
>>> classes. Let's split format parsing from utility code.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> +++ b/tests/qemu-iotests/qcow2.py
>>> @@ -1,163 +1,10 @@
>>> #!/usr/bin/env python3
>>> -
>>> import sys
>>
>> Pre-existing: no copyright blurb on the old file...
>>
>>> +++ b/tests/qemu-iotests/qcow2_format.py
>>> @@ -0,0 +1,157 @@
>>> +import struct
>>> +import string
>>> +
>>
>> It would be nice to fix that, and have one on the new file as well.
>>
>
>
> Yes, I thought about this.. But what to add? Seems OK to add Virtuozzo
> copyright and common "This program is free software; you can redist...
> GPL ..." header to qcow2_format.py, as we are going to rewrite and
> improve it a lot. But what to add to qcow2.py, to the part which is kept
> mostly unchanged?
Late answer, since I see you've already posted v5, but any file without
an explicit header is implicitly GPLv2+ by virtue of the top-level
LICENSE file.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2020-06-08 15:22 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 17:41 [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 01/12] qcow2.py: python style fixes Vladimir Sementsov-Ogievskiy
2020-06-05 10:23 ` Andrey Shinkevich
2020-06-05 19:43 ` Eric Blake
2020-06-06 7:00 ` Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module Vladimir Sementsov-Ogievskiy
2020-06-05 10:51 ` Andrey Shinkevich
2020-06-05 20:14 ` Eric Blake
2020-06-06 7:03 ` Vladimir Sementsov-Ogievskiy
2020-06-08 15:20 ` Eric Blake
2020-06-04 17:41 ` [PATCH v4 03/12] qcow2_format.py: drop new line printing at end of dump() Vladimir Sementsov-Ogievskiy
2020-06-05 10:54 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields Vladimir Sementsov-Ogievskiy
2020-06-05 11:20 ` Andrey Shinkevich
2020-06-05 20:16 ` Eric Blake
2020-06-06 7:07 ` Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 05/12] qcow2_format.py: use modern string formatting Vladimir Sementsov-Ogievskiy
2020-06-05 11:27 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields Vladimir Sementsov-Ogievskiy
2020-06-05 11:36 ` Andrey Shinkevich
2020-06-05 20:19 ` Eric Blake
2020-06-04 17:41 ` [PATCH v4 07/12] qcow2_format.py: separate generic functionality of structure classes Vladimir Sementsov-Ogievskiy
2020-06-05 12:29 ` Andrey Shinkevich
2020-06-05 12:37 ` Vladimir Sementsov-Ogievskiy
2020-06-04 17:41 ` [PATCH v4 08/12] qcow2_format.py: add field-formatting class Vladimir Sementsov-Ogievskiy
2020-06-05 13:27 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 09/12] qcow2_format.py: QcowHeaderExtension: add dump method Vladimir Sementsov-Ogievskiy
2020-06-05 14:43 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 10/12] qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct Vladimir Sementsov-Ogievskiy
2020-06-05 15:30 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 11/12] qcow2: QcowHeaderExtension print names for extension magics Vladimir Sementsov-Ogievskiy
2020-06-05 15:42 ` Andrey Shinkevich
2020-06-05 16:26 ` Vladimir Sementsov-Ogievskiy
2020-06-05 16:41 ` Andrey Shinkevich
2020-06-04 17:41 ` [PATCH v4 12/12] qcow2_format.py: dump bitmaps header extension Vladimir Sementsov-Ogievskiy
2020-06-05 16:06 ` Andrey Shinkevich
2020-06-04 19:14 ` [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata no-reply
2020-06-05 10:02 ` Vladimir Sementsov-Ogievskiy
2020-06-05 20:50 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.