[PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields

Vladimir Sementsov-Ogievskiy posted 12 patches 5 years, 8 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
Posted by Vladimir Sementsov-Ogievskiy 5 years, 8 months ago
No need in lists: it's a constant variable.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py b/tests/qemu-iotests/qcow2_format.py
index 99e5248e73..5d242c4aa4 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -23,29 +23,29 @@ class QcowHeader:
     uint32_t = 'I'
     uint64_t = 'Q'
 
-    fields = [
+    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'],
+        (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'],
-    ]
+        (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)
 
-- 
2.21.0


Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
Posted by Andrey Shinkevich 5 years, 8 months ago
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

________________________________
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Sent: Thursday, June 4, 2020 8:41 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mreitz@redhat.com <mreitz@redhat.com>; kwolf@redhat.com <kwolf@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields

No need in lists: it's a constant variable.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 20 deletions(-)
Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
Posted by Eric Blake 5 years, 8 months ago
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
> No need in lists: it's a constant variable.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
>   1 file changed, 20 insertions(+), 20 deletions(-)
> 

>   
>           # 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'],
> -    ]
> +        (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'),
> +    )

Not for this patch, but noticing it since we're here: should this be 
taught to list the optional compression_type field that we recently 
added after header_length?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
Posted by Vladimir Sementsov-Ogievskiy 5 years, 8 months ago
05.06.2020 23:16, Eric Blake wrote:
> On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote:
>> No need in lists: it's a constant variable.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/qcow2_format.py | 40 +++++++++++++++---------------
>>   1 file changed, 20 insertions(+), 20 deletions(-)
>>
> 
>>           # 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'],
>> -    ]
>> +        (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'),
>> +    )
> 
> Not for this patch, but noticing it since we're here: should this be taught to list the optional compression_type field that we recently added after header_length?
> 

Good note. And we can use it in corresponding iotest about compression_type. We just should carefully check, how header_length field is handled and fix if needed. I think, Andrey or I will make an additional patch.

-- 
Best regards,
Vladimir