Re: [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: kwolf@redhat.com, den@openvz.org, qemu-devel@nongnu.org,
	mreitz@redhat.com
Subject: Re: [PATCH v12 07/11] qcow2_format.py: Dump bitmap table serialized entries
Date: Wed, 5 Aug 2020 18:57:26 +0300	[thread overview]
Message-ID: <ed880db8-be61-1633-390c-a6a708ef19ad@virtuozzo.com> (raw)
In-Reply-To: <1596118512-424960-8-git-send-email-andrey.shinkevich@virtuozzo.com>

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

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

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

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

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

>   
>   QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
> 

-- 
Best regards,
Vladimir