The cluster size of an image is the QcowHeader class member and may be
obtained by dependent extension structures such as Qcow2BitmapExt for
further bitmap table details print.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/qcow2_format.py | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index f0db9f4..d9c8513 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -129,19 +129,21 @@ class Qcow2BitmapExt(Qcow2Struct):
('u64', '{:#x}', 'bitmap_directory_offset')
)
- def __init__(self, fd):
+ def __init__(self, fd, cluster_size):
super().__init__(fd=fd)
pad = (struct.calcsize(self.fmt) + 7) & ~7
if pad:
fd.seek(pad, 1)
position = fd.tell()
+ self.cluster_size = cluster_size
self.read_bitmap_directory(fd)
fd.seek(position)
def read_bitmap_directory(self, fd):
fd.seek(self.bitmap_directory_offset)
self.bitmap_directory = \
- [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+ [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
+ for _ in range(self.nb_bitmaps)]
def dump(self):
super().dump()
@@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
('u32', '{}', 'extra_data_size')
)
- def __init__(self, fd):
+ def __init__(self, fd, cluster_size):
super().__init__(fd=fd)
+ self.cluster_size = cluster_size
# Seek relative to the current position in the file
fd.seek(self.extra_data_size, 1)
bitmap_name = fd.read(self.name_size)
@@ -203,11 +206,13 @@ class QcowHeaderExtension(Qcow2Struct):
# then padding to next multiply of 8
)
- def __init__(self, magic=None, length=None, data=None, fd=None):
+ def __init__(self, magic=None, length=None, data=None, fd=None,
+ cluster_size=None):
"""
Support both loading from fd and creation from user data.
For fd-based creation current position in a file will be used to read
the data.
+ The cluster_size value may be obtained by dependent structures.
This should be somehow refactored and functionality should be moved to
superclass (to allow creation of any qcow2 struct), but then, fields
@@ -230,7 +235,7 @@ class QcowHeaderExtension(Qcow2Struct):
assert all(v is None for v in (magic, length, data))
super().__init__(fd=fd)
if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
- self.obj = Qcow2BitmapExt(fd=fd)
+ self.obj = Qcow2BitmapExt(fd=fd, cluster_size=cluster_size)
else:
padded = (self.length + 7) & ~7
self.data = fd.read(padded)
@@ -244,7 +249,6 @@ class QcowHeaderExtension(Qcow2Struct):
data_str = '<binary>'
self.data_str = data_str
-
def dump(self):
super().dump()
@@ -316,7 +320,7 @@ class QcowHeader(Qcow2Struct):
end = self.cluster_size
while fd.tell() < end:
- ext = QcowHeaderExtension(fd=fd)
+ ext = QcowHeaderExtension(fd=fd, cluster_size=self.cluster_size)
if ext.magic == 0:
break
else:
--
1.8.3.1
14.07.2020 00:36, Andrey Shinkevich wrote:
> The cluster size of an image is the QcowHeader class member and may be
> obtained by dependent extension structures such as Qcow2BitmapExt for
> further bitmap table details print.
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/qcow2_format.py | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
> index f0db9f4..d9c8513 100644
> --- a/tests/qemu-iotests/qcow2_format.py
> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -129,19 +129,21 @@ class Qcow2BitmapExt(Qcow2Struct):
> ('u64', '{:#x}', 'bitmap_directory_offset')
> )
>
> - def __init__(self, fd):
> + def __init__(self, fd, cluster_size):
> super().__init__(fd=fd)
> pad = (struct.calcsize(self.fmt) + 7) & ~7
> if pad:
> fd.seek(pad, 1)
> position = fd.tell()
> + self.cluster_size = cluster_size
> self.read_bitmap_directory(fd)
> fd.seek(position)
>
> def read_bitmap_directory(self, fd):
> fd.seek(self.bitmap_directory_offset)
> self.bitmap_directory = \
> - [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
> + [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
> + for _ in range(self.nb_bitmaps)]
Better to inline the bitmap directory loading code into __init__:
Benefits:
1. Less code. read_bitmap_directory() is very small, used only in __init__ and just not needed as a separate method. __init__ is very small and simple too, so it's not a problem.
2. no need of extra self.cluster_size variable (you can use cluster_size parameter directly)
3. keep all fd.seek logic in one method
but it's not about this patch.
>
> def dump(self):
> super().dump()
> @@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
> ('u32', '{}', 'extra_data_size')
> )
>
> - def __init__(self, fd):
> + def __init__(self, fd, cluster_size):
> super().__init__(fd=fd)
> + self.cluster_size = cluster_size
> # Seek relative to the current position in the file
> fd.seek(self.extra_data_size, 1)
> bitmap_name = fd.read(self.name_size)
> @@ -203,11 +206,13 @@ class QcowHeaderExtension(Qcow2Struct):
> # then padding to next multiply of 8
> )
>
> - def __init__(self, magic=None, length=None, data=None, fd=None):
> + def __init__(self, magic=None, length=None, data=None, fd=None,
> + cluster_size=None):
> """
> Support both loading from fd and creation from user data.
> For fd-based creation current position in a file will be used to read
> the data.
> + The cluster_size value may be obtained by dependent structures.
>
> This should be somehow refactored and functionality should be moved to
> superclass (to allow creation of any qcow2 struct), but then, fields
> @@ -230,7 +235,7 @@ class QcowHeaderExtension(Qcow2Struct):
> assert all(v is None for v in (magic, length, data))
> super().__init__(fd=fd)
> if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
> - self.obj = Qcow2BitmapExt(fd=fd)
> + self.obj = Qcow2BitmapExt(fd=fd, cluster_size=cluster_size)
> else:
> padded = (self.length + 7) & ~7
> self.data = fd.read(padded)
> @@ -244,7 +249,6 @@ class QcowHeaderExtension(Qcow2Struct):
> data_str = '<binary>'
> self.data_str = data_str
>
> -
Unrelated style-fixing chunk, please don't do so.
with it dropped:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> def dump(self):
> super().dump()
>
> @@ -316,7 +320,7 @@ class QcowHeader(Qcow2Struct):
> end = self.cluster_size
>
> while fd.tell() < end:
> - ext = QcowHeaderExtension(fd=fd)
> + ext = QcowHeaderExtension(fd=fd, cluster_size=self.cluster_size)
> if ext.magic == 0:
> break
> else:
>
--
Best regards,
Vladimir
On 16.07.2020 12:26, Vladimir Sementsov-Ogievskiy wrote: > 14.07.2020 00:36, Andrey Shinkevich wrote: >> The cluster size of an image is the QcowHeader class member and may be >> obtained by dependent extension structures such as Qcow2BitmapExt for >> further bitmap table details print. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> tests/qemu-iotests/qcow2_format.py | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/tests/qemu-iotests/qcow2_format.py >> b/tests/qemu-iotests/qcow2_format.py >> ... >> def read_bitmap_directory(self, fd): >> fd.seek(self.bitmap_directory_offset) >> self.bitmap_directory = \ >> - [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)] >> + [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size) >> + for _ in range(self.nb_bitmaps)] > > Better to inline the bitmap directory loading code into __init__: > > Benefits: > 1. Less code. read_bitmap_directory() is very small, used only in > __init__ and just not needed as a separate method. __init__ is very > small and simple too, so it's not a problem. > 2. no need of extra self.cluster_size variable (you can use > cluster_size parameter directly) > 3. keep all fd.seek logic in one method > > but it's not about this patch. > The idea behind the read_bitmap_directory() method was an encapsulation of reading the optional parameter. So, we can be flexible in future. Sure, there are prones and cons for that in the current implementation. Andrey >> def dump(self): >> super().dump() >> @@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct): >> ... >> @@ -244,7 +249,6 @@ class QcowHeaderExtension(Qcow2Struct): >> data_str = '<binary>' >> self.data_str = data_str >> - > > Unrelated style-fixing chunk, please don't do so. > > with it dropped: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >> def dump(self): >> super().dump() >> @@ -316,7 +320,7 @@ class QcowHeader(Qcow2Struct): >> end = self.cluster_size >> while fd.tell() < end: >> - ext = QcowHeaderExtension(fd=fd) >> + ext = QcowHeaderExtension(fd=fd, >> cluster_size=self.cluster_size) >> if ext.magic == 0: >> break >> else: >> > >
© 2016 - 2025 Red Hat, Inc.