diff mbox series

[v4,02/12] qcow2.py: move qcow2 format classes to separate module

Message ID 20200604174135.11042-3-vsementsov@virtuozzo.com
State New
Headers show
Series iotests: Dump QCOW2 dirty bitmaps metadata | expand

Commit Message

Vladimir Sementsov-Ogievskiy June 4, 2020, 5:41 p.m. UTC 
We are going to enhance qcow2 format parsing by adding more structure
classes. Let's split format parsing from utility code.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2.py        | 161 +----------------------------
 tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
 2 files changed, 161 insertions(+), 157 deletions(-)
 create mode 100644 tests/qemu-iotests/qcow2_format.py

Comments

Andrey Shinkevich June 5, 2020, 10:51 a.m. UTC | #1 
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Eric Blake June 5, 2020, 8:14 p.m. UTC | #2 
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to enhance qcow2 format parsing by adding more structure
> classes. Let's split format parsing from utility code.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2.py        | 161 +----------------------------
>   tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
>   2 files changed, 161 insertions(+), 157 deletions(-)
>   create mode 100644 tests/qemu-iotests/qcow2_format.py
> 
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index 539f5a186b..d9c41668fd 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -1,163 +1,10 @@
>   #!/usr/bin/env python3
> -
>   import sys

Pre-existing: no copyright blurb on the old file...

> +++ b/tests/qemu-iotests/qcow2_format.py
> @@ -0,0 +1,157 @@
> +import struct
> +import string
> +

It would be nice to fix that, and have one on the new file as well.
Vladimir Sementsov-Ogievskiy June 6, 2020, 7:03 a.m. UTC | #3 
05.06.2020 23:14, Eric Blake wrote:
> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>> We are going to enhance qcow2 format parsing by adding more structure
>> classes. Let's split format parsing from utility code.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/qcow2.py        | 161 +----------------------------
>>   tests/qemu-iotests/qcow2_format.py | 157 ++++++++++++++++++++++++++++
>>   2 files changed, 161 insertions(+), 157 deletions(-)
>>   create mode 100644 tests/qemu-iotests/qcow2_format.py
>>
>> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
>> index 539f5a186b..d9c41668fd 100755
>> --- a/tests/qemu-iotests/qcow2.py
>> +++ b/tests/qemu-iotests/qcow2.py
>> @@ -1,163 +1,10 @@
>>   #!/usr/bin/env python3
>> -
>>   import sys
> 
> Pre-existing: no copyright blurb on the old file...
> 
>> +++ b/tests/qemu-iotests/qcow2_format.py
>> @@ -0,0 +1,157 @@
>> +import struct
>> +import string
>> +
> 
> It would be nice to fix that, and have one on the new file as well.
> 


Yes, I thought about this.. But what to add? Seems OK to add Virtuozzo copyright and common "This program is free software; you can redist... GPL ..." header to qcow2_format.py, as we are going to rewrite and improve it a lot. But what to add to qcow2.py, to the part which is kept mostly unchanged?
Eric Blake June 8, 2020, 3:20 p.m. UTC | #4 
On 6/6/20 2:03 AM, Vladimir Sementsov-Ogievskiy wrote:
> 05.06.2020 23:14, Eric Blake wrote:
>> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> We are going to enhance qcow2 format parsing by adding more structure
>>> classes. Let's split format parsing from utility code.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---

>>> +++ b/tests/qemu-iotests/qcow2.py
>>> @@ -1,163 +1,10 @@
>>>   #!/usr/bin/env python3
>>> -
>>>   import sys
>>
>> Pre-existing: no copyright blurb on the old file...
>>
>>> +++ b/tests/qemu-iotests/qcow2_format.py
>>> @@ -0,0 +1,157 @@
>>> +import struct
>>> +import string
>>> +
>>
>> It would be nice to fix that, and have one on the new file as well.
>>
> 
> 
> Yes, I thought about this.. But what to add? Seems OK to add Virtuozzo 
> copyright and common "This program is free software; you can redist... 
> GPL ..." header to qcow2_format.py, as we are going to rewrite and 
> improve it a lot. But what to add to qcow2.py, to the part which is kept 
> mostly unchanged?

Late answer, since I see you've already posted v5, but any file without 
an explicit header is implicitly GPLv2+ by virtue of the top-level 
LICENSE file.
diff mbox series

Patch

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 539f5a186b..d9c41668fd 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -1,163 +1,10 @@ 
 #!/usr/bin/env python3
-
 import sys
-import struct
-import string
-
-
-class QcowHeaderExtension:
-
-    def __init__(self, magic, length, data):
-        if length % 8 != 0:
-            padding = 8 - (length % 8)
-            data += b"\0" * padding
-
-        self.magic = magic
-        self.length = length
-        self.data = data
-
-    @classmethod
-    def create(cls, magic, data):
-        return QcowHeaderExtension(magic, len(data), data)
-
-
-class QcowHeader:
-
-    uint32_t = 'I'
-    uint64_t = 'Q'
-
-    fields = [
-        # Version 2 header fields
-        [uint32_t, '%#x',  'magic'],
-        [uint32_t, '%d',   'version'],
-        [uint64_t, '%#x',  'backing_file_offset'],
-        [uint32_t, '%#x',  'backing_file_size'],
-        [uint32_t, '%d',   'cluster_bits'],
-        [uint64_t, '%d',   'size'],
-        [uint32_t, '%d',   'crypt_method'],
-        [uint32_t, '%d',   'l1_size'],
-        [uint64_t, '%#x',  'l1_table_offset'],
-        [uint64_t, '%#x',  'refcount_table_offset'],
-        [uint32_t, '%d',   'refcount_table_clusters'],
-        [uint32_t, '%d',   'nb_snapshots'],
-        [uint64_t, '%#x',  'snapshot_offset'],
-
-        # Version 3 header fields
-        [uint64_t, 'mask', 'incompatible_features'],
-        [uint64_t, 'mask', 'compatible_features'],
-        [uint64_t, 'mask', 'autoclear_features'],
-        [uint32_t, '%d',   'refcount_order'],
-        [uint32_t, '%d',   'header_length'],
-    ]
-
-    fmt = '>' + ''.join(field[0] for field in fields)
-
-    def __init__(self, fd):
-
-        buf_size = struct.calcsize(QcowHeader.fmt)
-
-        fd.seek(0)
-        buf = fd.read(buf_size)
-
-        header = struct.unpack(QcowHeader.fmt, buf)
-        self.__dict__ = dict((field[2], header[i])
-                             for i, field in enumerate(QcowHeader.fields))
-
-        self.set_defaults()
-        self.cluster_size = 1 << self.cluster_bits
-
-        fd.seek(self.header_length)
-        self.load_extensions(fd)
-
-        if self.backing_file_offset:
-            fd.seek(self.backing_file_offset)
-            self.backing_file = fd.read(self.backing_file_size)
-        else:
-            self.backing_file = None
-
-    def set_defaults(self):
-        if self.version == 2:
-            self.incompatible_features = 0
-            self.compatible_features = 0
-            self.autoclear_features = 0
-            self.refcount_order = 4
-            self.header_length = 72
-
-    def load_extensions(self, fd):
-        self.extensions = []
-
-        if self.backing_file_offset != 0:
-            end = min(self.cluster_size, self.backing_file_offset)
-        else:
-            end = self.cluster_size
-
-        while fd.tell() < end:
-            (magic, length) = struct.unpack('>II', fd.read(8))
-            if magic == 0:
-                break
-            else:
-                padded = (length + 7) & ~7
-                data = fd.read(padded)
-                self.extensions.append(QcowHeaderExtension(magic, length,
-                                                           data))
-
-    def update_extensions(self, fd):
-
-        fd.seek(self.header_length)
-        extensions = self.extensions
-        extensions.append(QcowHeaderExtension(0, 0, b""))
-        for ex in extensions:
-            buf = struct.pack('>II', ex.magic, ex.length)
-            fd.write(buf)
-            fd.write(ex.data)
-
-        if self.backing_file is not None:
-            self.backing_file_offset = fd.tell()
-            fd.write(self.backing_file)
-
-        if fd.tell() > self.cluster_size:
-            raise Exception("I think I just broke the image...")
-
-    def update(self, fd):
-        header_bytes = self.header_length
-
-        self.update_extensions(fd)
-
-        fd.seek(0)
-        header = tuple(self.__dict__[f] for t, p, f in QcowHeader.fields)
-        buf = struct.pack(QcowHeader.fmt, *header)
-        buf = buf[0:header_bytes-1]
-        fd.write(buf)
-
-    def dump(self):
-        for f in QcowHeader.fields:
-            value = self.__dict__[f[2]]
-            if f[1] == 'mask':
-                bits = []
-                for bit in range(64):
-                    if value & (1 << bit):
-                        bits.append(bit)
-                value_str = str(bits)
-            else:
-                value_str = f[1] % value
-
-            print("%-25s" % f[2], value_str)
-        print("")
-
-    def dump_extensions(self):
-        for ex in self.extensions:
-
-            data = ex.data[:ex.length]
-            if all(c in string.printable.encode('ascii') for c in data):
-                data = "'%s'" % data.decode('ascii')
-            else:
-                data = "<binary>"
 
-            print("Header extension:")
-            print("%-25s %#x" % ("magic", ex.magic))
-            print("%-25s %d" % ("length", ex.length))
-            print("%-25s %s" % ("data", data))
-            print("")
+from qcow2_format import (
+    QcowHeader,
+    QcowHeaderExtension
+)
 
 
 def cmd_dump_header(fd):
diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
new file mode 100644
index 0000000000..c7270a0a6e
--- /dev/null
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -0,0 +1,157 @@ 
+import struct
+import string
+
+
+class QcowHeaderExtension:
+
+    def __init__(self, magic, length, data):
+        if length % 8 != 0:
+            padding = 8 - (length % 8)
+            data += b"\0" * padding
+
+        self.magic = magic
+        self.length = length
+        self.data = data
+
+    @classmethod
+    def create(cls, magic, data):
+        return QcowHeaderExtension(magic, len(data), data)
+
+
+class QcowHeader:
+
+    uint32_t = 'I'
+    uint64_t = 'Q'
+
+    fields = [
+        # Version 2 header fields
+        [uint32_t, '%#x',  'magic'],
+        [uint32_t, '%d',   'version'],
+        [uint64_t, '%#x',  'backing_file_offset'],
+        [uint32_t, '%#x',  'backing_file_size'],
+        [uint32_t, '%d',   'cluster_bits'],
+        [uint64_t, '%d',   'size'],
+        [uint32_t, '%d',   'crypt_method'],
+        [uint32_t, '%d',   'l1_size'],
+        [uint64_t, '%#x',  'l1_table_offset'],
+        [uint64_t, '%#x',  'refcount_table_offset'],
+        [uint32_t, '%d',   'refcount_table_clusters'],
+        [uint32_t, '%d',   'nb_snapshots'],
+        [uint64_t, '%#x',  'snapshot_offset'],
+
+        # Version 3 header fields
+        [uint64_t, 'mask', 'incompatible_features'],
+        [uint64_t, 'mask', 'compatible_features'],
+        [uint64_t, 'mask', 'autoclear_features'],
+        [uint32_t, '%d',   'refcount_order'],
+        [uint32_t, '%d',   'header_length'],
+    ]
+
+    fmt = '>' + ''.join(field[0] for field in fields)
+
+    def __init__(self, fd):
+
+        buf_size = struct.calcsize(QcowHeader.fmt)
+
+        fd.seek(0)
+        buf = fd.read(buf_size)
+
+        header = struct.unpack(QcowHeader.fmt, buf)
+        self.__dict__ = dict((field[2], header[i])
+                             for i, field in enumerate(QcowHeader.fields))
+
+        self.set_defaults()
+        self.cluster_size = 1 << self.cluster_bits
+
+        fd.seek(self.header_length)
+        self.load_extensions(fd)
+
+        if self.backing_file_offset:
+            fd.seek(self.backing_file_offset)
+            self.backing_file = fd.read(self.backing_file_size)
+        else:
+            self.backing_file = None
+
+    def set_defaults(self):
+        if self.version == 2:
+            self.incompatible_features = 0
+            self.compatible_features = 0
+            self.autoclear_features = 0
+            self.refcount_order = 4
+            self.header_length = 72
+
+    def load_extensions(self, fd):
+        self.extensions = []
+
+        if self.backing_file_offset != 0:
+            end = min(self.cluster_size, self.backing_file_offset)
+        else:
+            end = self.cluster_size
+
+        while fd.tell() < end:
+            (magic, length) = struct.unpack('>II', fd.read(8))
+            if magic == 0:
+                break
+            else:
+                padded = (length + 7) & ~7
+                data = fd.read(padded)
+                self.extensions.append(QcowHeaderExtension(magic, length,
+                                                           data))
+
+    def update_extensions(self, fd):
+
+        fd.seek(self.header_length)
+        extensions = self.extensions
+        extensions.append(QcowHeaderExtension(0, 0, b""))
+        for ex in extensions:
+            buf = struct.pack('>II', ex.magic, ex.length)
+            fd.write(buf)
+            fd.write(ex.data)
+
+        if self.backing_file is not None:
+            self.backing_file_offset = fd.tell()
+            fd.write(self.backing_file)
+
+        if fd.tell() > self.cluster_size:
+            raise Exception("I think I just broke the image...")
+
+    def update(self, fd):
+        header_bytes = self.header_length
+
+        self.update_extensions(fd)
+
+        fd.seek(0)
+        header = tuple(self.__dict__[f] for t, p, f in QcowHeader.fields)
+        buf = struct.pack(QcowHeader.fmt, *header)
+        buf = buf[0:header_bytes-1]
+        fd.write(buf)
+
+    def dump(self):
+        for f in QcowHeader.fields:
+            value = self.__dict__[f[2]]
+            if f[1] == 'mask':
+                bits = []
+                for bit in range(64):
+                    if value & (1 << bit):
+                        bits.append(bit)
+                value_str = str(bits)
+            else:
+                value_str = f[1] % value
+
+            print("%-25s" % f[2], value_str)
+        print("")
+
+    def dump_extensions(self):
+        for ex in self.extensions:
+
+            data = ex.data[:ex.length]
+            if all(c in string.printable.encode('ascii') for c in data):
+                data = "'%s'" % data.decode('ascii')
+            else:
+                data = "<binary>"
+
+            print("Header extension:")
+            print("%-25s %#x" % ("magic", ex.magic))
+            print("%-25s %d" % ("length", ex.length))
+            print("%-25s %s" % ("data", data))
+            print("")