Re: [PATCH v3 5/6] iotests: Dump bitmap table entries serialized in QCOW2 image

[Vladimir Sementsov-Ogievskiy]

01.06.2020 16:48, Andrey Shinkevich wrote:
> Add bitmap table info to the QCOW2 metadata dump with qcow2.py.
>
> Bitmap name               bitmap-1
> ...
> itmap table    type            offset          size
>          0       serialized      0xa0000         65536
>          1       all-zeroes      0x0             65536
>          2       all-zeroes      0x0             65536
>          3       all-zeroes      0x0             65536
>          4       all-zeroes      0x0             65536
>          5       all-zeroes      0x0             65536
>          6       all-zeroes      0x0             65536
>          7       all-zeroes      0x0             65536
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2.py | 48 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
>
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index e4453f6..76e0c69 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -5,6 +5,41 @@ import struct
>   import string
>   
>   
> +cluster_size = 0
> +
> +
> +class Qcow2BitmapTableEntry:
> +
> +    BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffffffffffe00
> +    BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
> +    bmte_type = ['all-zeroes', 'all-ones', 'serialized']
> +
> +    def __init__(self, entry):
> +        self.cluster_size = cluster_size
> +        self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
> +        if self.offset != 0:
> +            index = 2
> +        else:
> +            index = entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES
> +        self.type = self.bmte_type[index]

IMHO, it would be clearer without extra list layer:

if self.offset != 0:
  self.type = 'serialized'
elif entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
  self.type = 'all-ones'
else:
  self.type = 'all-zeroes'


> +
> +
> +class Qcow2BitmapTable:
> +
> +    def __init__(self, raw_table):
> +        self.entries = []
> +        for entry in raw_table:
> +            self.entries.append(Qcow2BitmapTableEntry(entry))
> +
> +    def print_bitmap_table(self):
> +        bitmap_table = enumerate(self.entries)
> +        print("Bitmap table\ttype\t\toffset\t\tsize")
> +        for i, entry in bitmap_table:
> +            print("\t%-4d\t%s\t%#x\t\t%d" % (i, entry.type, entry.offset,
> +                                             entry.cluster_size))
> +        print("")
> +
> +
>   class Qcow2BitmapDirEntry:
>   
>       name = ''
> @@ -48,6 +83,12 @@ class Qcow2BitmapDirEntry:
>           return struct.calcsize(self.fmt) + self.name_size + \
>               self.extra_data_size
>   
> +    def read_bitmap_table(self, fd):
> +        fd.seek(self.bitmap_table_offset)
> +        table_size = self.bitmap_table_size * struct.calcsize(self.uint64_t)
> +        table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
> +        self.bitmap_table = Qcow2BitmapTable(table)
> +
>       def dump_bitmap_dir_entry(self):
>           print("%-25s" % 'Bitmap name', self.name)
>   
> @@ -59,6 +100,8 @@ class Qcow2BitmapDirEntry:
>               value_str = f[1] % value
>               print("%-25s" % f[2], value_str)
>   
> +        self.bitmap_table.print_bitmap_table()
> +
>   
>   class Qcow2BitmapDirectory:
>   
> @@ -83,6 +126,9 @@ class Qcow2BitmapDirectory:
>               shift = ((entry_raw_size + 7) & ~7) - entry_raw_size
>               fd.seek(shift, 1)
>   
> +        for bm in self.bitmaps:
> +            bm.read_bitmap_table(fd)
> +
>       def get_bitmaps(self):
>           return self.bitmaps
>   
> @@ -223,6 +269,8 @@ class QcowHeader:
>   
>           self.set_defaults()
>           self.cluster_size = 1 << self.cluster_bits
> +        global cluster_size
> +        cluster_size = self.cluster_size

Oh.. We should avoid such thing. You set global variable here and use it in 
another class. It will definitely break, if we try load two qcow2 files with 
different cluster sizes in one context.

>   
>           fd.seek(self.header_length)
>           self.load_extensions(fd)


--
Best regards,
Vladimir