01.06.2020 16:48, Andrey Shinkevich wrote:
Read and dump entries from the bitmap directory of QCOW2 image with the
script qcow2.py.
Header extension: Bitmaps
...
Bitmap name bitmap-1
flag auto
bitmap_table_offset 0xf
bitmap_table_size 8
flag_bits 2
type 1
granularity_bits 16
name_size 8
extra_data_size 0
Suggested-by: Kevin Wolf
Signed-off-by: Andrey Shinkevich
---
tests/qemu-iotests/qcow2.py | 104 +++-
1 file changed, 103 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 8286115..e4453f6 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -5,6 +5,88 @@ import struct
import string
+class Qcow2BitmapDirEntry:
+
+name = ''
+
+uint8_t = 'B'
+uint16_t = 'H'
+uint32_t = 'I'
+uint64_t = 'Q'
+
+fields = [
+[uint64_t, '%#x', 'bitmap_table_offset'],
+[uint32_t, '%d', 'bitmap_table_size'],
+[uint32_t, '%d', 'flag_bits'],
I'd keep exact field name from spec: "flags"
+[uint8_t, '%d', 'type'],
+[uint8_t, '%d', 'granularity_bits'],
+[uint16_t, '%d', 'name_size'],
+[uint32_t, '%d', 'extra_data_size']
+]
+
+fmt = '>' + ''.join(field[0] for field in fields)
+
+def __init__(self, data):
+
You are not consistent about adding this empty line. I'd avoid it.
+entry = struct.unpack(Qcow2BitmapDirEntry.fmt, data)
+self.__dict__ = dict((field[2], entry[i])
+ for i, field in enumerate(
+ Qcow2BitmapDirEntry.fields))
+
+self.bitmap_table_size = self.bitmap_table_size \
+* struct.calcsize(self.uint64_t)
I don't like this: you keep name from specification, but change its
meaning.
+
+self.bitmap_flags = []
+BME_FLAG_IN_USE = 1
+BME_FLAG_AUTO = 1 << 1
I'd define all constants copied from C code in global space, to be simply
available from the module.
+if (self.flag_bits & BME_FLAG_IN_USE) != 0:
in python zero is false enough :) (you may omit comparison to 0)
+self.bitmap_flags.append("in-use")
+if (self.flag_bits & BME_FLAG_AUTO) != 0:
+self.bitmap_flags.append("auto")
+
+def bitmap_dir_entry_raw_size(self):
+return struct.calcsize(self.fmt) + self.name_size + \
+self.extra_data_size
+
+def dump_bitmap_dir_entry(self):
+print("%-25s" % 'Bitmap name', self.name)
+
+for fl in self.bitmap_flags:
+print("%-25s" % 'flag', fl)
+
+for f in Qcow2BitmapDirEntry.fields:
+value = self.__dict__[f[2]]
+value_str = f[1] % value
+print("%-25s" % f[2], value_str)
+
+
+class Qcow2BitmapDirectory:
+
+def __init__(self, bm_header_ext):
+self.nb_bitmaps = bm_header_ext.nb_bitmaps
+self.bitmap_directory_offset = bm_header_ext.bitmap_directory_offset
+self.bitmap_directory_size = bm_header_ext.bitmap_directory_size
Honestly, I don't like duplicating attributes between different objects.
+
+def read_bitmap_directory(self, fd):
Why not do it in constructor? The only use of the class
+self.bitmaps = []
+fd.seek(self.bitmap_directory_offset)
+buf_size = struct.calcsize(Qcow2BitmapDirEntry.fmt)
+
+for n in range(self.nb_bitmaps):
+buf = fd.read(buf_size)
+dir_entry = Qcow2BitmapDirEntry(buf)
+fd.seek(dir_entry.extra_data_size, 1)
+bitmap_name = fd.read(dir_entry.name_size)
+dir_entry.name = bitmap_name.decode('ascii')
+self.bitmaps.append(dir_entry)
+entry_raw_size = dir_entry.bitmap_dir_entry_raw_size()
+shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
+fd.seek(shift, 1)
+
+def get_bitmaps(self):
+return self.bitmaps
Why we need getter for this field?
+
+
class Qcow2BitmapExt:
uint32_t = 'I'
@@ -33,8 +115,21 @@ class Qcow2BitmapExt:
print("%-25s" % f[2], value_str)
print("")
+def read_bitmap_directory(self, fd):
+bm_directory = Qcow2BitmapDirectory(self)
+bm_directory.read_bitmap_directory(fd)
+self.bitmaps = bm_directory.get_bitmaps()
+
+def load(self, fd):
+self.read_bitmap_directory(fd)
+
+def dump_bitmap_directory(self):
+for bm in self.bitmaps:
+bm.dump_bitmap_dir_entry()
+
def dump_ext(self):
self.dump_bitmap_ext()
+self.dump_bitmap_directory()
class QcowHeaderExtension:
@@ -79,6 +174,10 @@ class QcowHeaderExtension:
self.QCOW2_EXT_MAGIC_DATA_FILE: 'Data file',
}.get(magic, 'Unknown')
+def load(self, fd):
+if self.obj is not None:
+
