[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8 03/10] qcow2_format.py: change Qcow2BitmapExt initializati

From: Andrey Shinkevich
Subject: Re: [PATCH v8 03/10] qcow2_format.py: change Qcow2BitmapExt initialization method
Date: Mon, 13 Jul 2020 07:49:35 +0300
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.9.0

On 11.07.2020 19:34, Vladimir Sementsov-Ogievskiy wrote:
03.07.2020 16:13, 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 | 14 ++++++++++++--
  1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 2f3681b..1435e34 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -63,7 +63,8 @@ class Qcow2StructMeta(type):
    class Qcow2Struct(metaclass=Qcow2StructMeta):
  -    """Qcow2Struct: base class for qcow2 data structures
+    """
+    Qcow2Struct: base class for qcow2 data structures

Unrelated chunk. And why?

To conform to the common style for comments in the file as it is at

class QcowHeaderExtension::__init__()

        Successors should define fields class variable, which is: list of tuples,
      each of three elements:
@@ -113,6 +114,9 @@ class Qcow2BitmapExt(Qcow2Struct):
          ('u64', '{:#x}', 'bitmap_directory_offset')
  +    def __init__(self, fd):
+        super().__init__(fd=fd)

this does nothing. We inherit the __init__ of super class, no need to define it just to call same __init__.

    QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
  @@ -173,7 +177,13 @@ class QcowHeaderExtension(Qcow2Struct):
          self.data_str = data_str
            if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-            self.obj = Qcow2BitmapExt(data=self.data)
+            assert fd is not None
+            position = fd.tell()
+            # Step back to reread data

This definitely shows that we are doing something wrong

For Qcow2BitmapExt, we need both fd and data and they are mutualy exclusive

in the constructor of the class Qcow2Struct. Rereading the bitmap extension

is a solution without changing the Qcow2Struct. Any other suggestion?


+            padded = (self.length + 7) & ~7
+            fd.seek(-padded, 1)
+            self.obj = Qcow2BitmapExt(fd=fd)
+            fd.seek(position)
              self.obj = None

reply via email to

[Prev in Thread] Current Thread [Next in Thread]