14.07.2020 00:36, Andrey Shinkevich wrote:
> There are two ways to initialize a class derived from Qcow2Struct:
> 1. Pass a block of binary data to the constructor.
> 2. Pass the file descriptor to allow reading the file from constructor.
> Let's change the Qcow2BitmapExt initialization method from 1 to 2 to
> support a scattered reading in the initialization chain.
> The implementation comes with the patch that follows.
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2_format.py | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index 2f3681b..cbaffc4 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -113,6 +113,11 @@ class Qcow2BitmapExt(Qcow2Struct):
> ('u64', '{:#x}', 'bitmap_directory_offset')
> )
>
> + def __init__(self, fd):
> + super().__init__(fd=fd)
> + pad = (struct.calcsize(self.fmt) + 7) & ~7
It's a size of structure rounded up to 8-bytes boundary. But after super init, we should alread be at the end of the structure and need to add only padding, not the whole structure rounded up. I think, correct code should be:
tail = struct.calcsize(self.fmt) % 8
if tail:
fd.seek(8 - tail, 1)
> + if pad:
> + fd.seek(pad, 1)
>
> QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
>
> @@ -161,21 +166,21 @@ class QcowHeaderExtension(Qcow2Struct):
> else:
Hmm, you parse data now only for this "else:" branch. Prior to this patch, it was parsed for "if fd is None:" branch as well, after the whole "if" statement. This is wrong.
> 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
> -
> - data_str = self.data[:self.length]
> - if all(c in string.printable.encode('ascii') for c in data_str):
> - data_str = f"'{ data_str.decode('ascii') }'"
> - else:
> - data_str = '<binary>'
> - self.data_str = data_str
> + if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
> + self.obj = Qcow2BitmapExt(fd=fd)
> + else:
> + padded = (self.length + 7) & ~7
> + self.data = fd.read(padded)
> + assert self.data is not None
> + self.obj = None
> + data_str = self.data[:self.length]
> + if all(c in string.printable.encode(
> + 'ascii') for c in data_str):
> + data_str = f"'{ data_str.decode('ascii') }'"
> + else:
> + data_str = '<binary>'
> + self.data_str = data_str
>
> - if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
> - self.obj = Qcow2BitmapExt(data=self.data)
> - else:
> - self.obj = None
>
> def dump(self):
> super().dump()
>
--
Best regards,
Vladimir