1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] iotests/236: fix transaction kwarg order

John Snow posted 1 patch 5 years, 2 months ago
Test docker-mingw@fedora passed
Test asan passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190131022648.14264-1-jsnow@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
tests/qemu-iotests/236.out    | 56 +++++++++++++++++------------------
tests/qemu-iotests/iotests.py | 21 ++++++-------
2 files changed, 39 insertions(+), 38 deletions(-)
[Qemu-devel] [PATCH] iotests/236: fix transaction kwarg order
Posted by John Snow 5 years, 2 months ago 
It's not enough to order the kwargs for consistent QMP log output,
we must also sort any sub-dictionaries in lists that appear as values.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/236.out    | 56 +++++++++++++++++------------------
 tests/qemu-iotests/iotests.py | 21 ++++++-------
 2 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/tests/qemu-iotests/236.out b/tests/qemu-iotests/236.out
index 1dad24db0d..bb2d71ea5e 100644
--- a/tests/qemu-iotests/236.out
+++ b/tests/qemu-iotests/236.out
@@ -45,23 +45,23 @@ write -P0xcd 0x3ff0000 64k
     "actions": [
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapB"
+          "name": "bitmapB",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-disable"
       },
       {
         "data": {
-          "node": "drive0",
+          "granularity": 65536,
           "name": "bitmapC",
-          "granularity": 65536
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-add"
       },
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapA"
+          "name": "bitmapA",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-clear"
       },
@@ -105,30 +105,30 @@ write -P0xcd 0x3ff0000 64k
     "actions": [
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapB"
+          "name": "bitmapB",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-disable"
       },
       {
         "data": {
-          "node": "drive0",
+          "granularity": 65536,
           "name": "bitmapC",
-          "granularity": 65536
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-add"
       },
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapC"
+          "name": "bitmapC",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-disable"
       },
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapC"
+          "name": "bitmapC",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-enable"
       }
@@ -158,15 +158,15 @@ write -P0xea 0x3fe0000 64k
     "actions": [
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapA"
+          "name": "bitmapA",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-disable"
       },
       {
         "data": {
-          "node": "drive0",
-          "name": "bitmapC"
+          "name": "bitmapC",
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-disable"
       }
@@ -209,21 +209,21 @@ write -P0xea 0x3fe0000 64k
     "actions": [
       {
         "data": {
-          "node": "drive0",
           "disabled": true,
+          "granularity": 65536,
           "name": "bitmapD",
-          "granularity": 65536
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-add"
       },
       {
         "data": {
-          "node": "drive0",
-          "target": "bitmapD",
           "bitmaps": [
             "bitmapB",
             "bitmapC"
-          ]
+          ],
+          "node": "drive0",
+          "target": "bitmapD"
         },
         "type": "block-dirty-bitmap-merge"
       },
@@ -273,21 +273,21 @@ write -P0xea 0x3fe0000 64k
     "actions": [
       {
         "data": {
-          "node": "drive0",
           "disabled": true,
+          "granularity": 65536,
           "name": "bitmapD",
-          "granularity": 65536
+          "node": "drive0"
         },
         "type": "block-dirty-bitmap-add"
       },
       {
         "data": {
-          "node": "drive0",
-          "target": "bitmapD",
           "bitmaps": [
             "bitmapB",
             "bitmapC"
-          ]
+          ],
+          "node": "drive0",
+          "target": "bitmapD"
         },
         "type": "block-dirty-bitmap-merge"
       }
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index cbedfaf1df..76ad52b358 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -76,15 +76,16 @@ def qemu_img(*args):
         sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))
     return exitcode
 
-def ordered_kwargs(kwargs):
-    # kwargs prior to 3.6 are not ordered, so:
-    od = OrderedDict()
-    for k, v in sorted(kwargs.items()):
-        if isinstance(v, dict):
-            od[k] = ordered_kwargs(v)
-        else:
-            od[k] = v
-    return od
+def ordered_qmp(qmsg):
+    # Dictionaries are not ordered prior to 3.6, therefore:
+    if isinstance(qmsg, list):
+        return [ordered_qmp(atom) for atom in qmsg]
+    if isinstance(qmsg, dict):
+        od = OrderedDict()
+        for k, v in sorted(qmsg.items()):
+            od[k] = ordered_qmp(v)
+        return od
+    return qmsg
 
 def qemu_img_create(*args):
     args = list(args)
@@ -491,7 +492,7 @@ class VM(qtest.QEMUQtestMachine):
     def qmp_log(self, cmd, filters=[], indent=None, **kwargs):
         full_cmd = OrderedDict((
             ("execute", cmd),
-            ("arguments", ordered_kwargs(kwargs))
+            ("arguments", ordered_qmp(kwargs))
         ))
         log(full_cmd, filters, indent=indent)
         result = self.qmp(cmd, **kwargs)
-- 
2.17.2
Re: [Qemu-devel] [PATCH] iotests/236: fix transaction kwarg order
Posted by Eric Blake 5 years, 2 months ago 
On 1/30/19 8:26 PM, John Snow wrote:
> It's not enough to order the kwargs for consistent QMP log output,
> we must also sort any sub-dictionaries in lists that appear as values.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/236.out    | 56 +++++++++++++++++------------------
>  tests/qemu-iotests/iotests.py | 21 ++++++-------
>  2 files changed, 39 insertions(+), 38 deletions(-)
> 

> +++ b/tests/qemu-iotests/iotests.py
> @@ -76,15 +76,16 @@ def qemu_img(*args):
>          sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))
>      return exitcode
>  
> -def ordered_kwargs(kwargs):
> -    # kwargs prior to 3.6 are not ordered, so:
> -    od = OrderedDict()
> -    for k, v in sorted(kwargs.items()):
> -        if isinstance(v, dict):
> -            od[k] = ordered_kwargs(v)
> -        else:
> -            od[k] = v
> -    return od
> +def ordered_qmp(qmsg):
> +    # Dictionaries are not ordered prior to 3.6, therefore:
> +    if isinstance(qmsg, list):
> +        return [ordered_qmp(atom) for atom in qmsg]
> +    if isinstance(qmsg, dict):
> +        od = OrderedDict()
> +        for k, v in sorted(qmsg.items()):
> +            od[k] = ordered_qmp(v)
> +        return od
> +    return qmsg
>  

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] iotests/236: fix transaction kwarg order
Posted by Kevin Wolf 5 years, 2 months ago 
Am 31.01.2019 um 03:26 hat John Snow geschrieben:
> It's not enough to order the kwargs for consistent QMP log output,
> we must also sort any sub-dictionaries in lists that appear as values.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>

Thanks, applied to the block branch.

Kevin
Re: [Qemu-devel] [PATCH] iotests/236: fix transaction kwarg order
Posted by Vladimir Sementsov-Ogievskiy 5 years, 2 months ago 
31.01.2019 5:26, John Snow wrote:
> It's not enough to order the kwargs for consistent QMP log output,
> we must also sort any sub-dictionaries in lists that appear as values.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/236.out    | 56 +++++++++++++++++------------------
>   tests/qemu-iotests/iotests.py | 21 ++++++-------
>   2 files changed, 39 insertions(+), 38 deletions(-)
> 
> diff --git a/tests/qemu-iotests/236.out b/tests/qemu-iotests/236.out
> index 1dad24db0d..bb2d71ea5e 100644
> --- a/tests/qemu-iotests/236.out
> +++ b/tests/qemu-iotests/236.out
> @@ -45,23 +45,23 @@ write -P0xcd 0x3ff0000 64k
>       "actions": [
>         {
>           "data": {
> -          "node": "drive0",
> -          "name": "bitmapB"

We could notice during review, that things we are trying to order so hard
are just not ordered in result :)))

> +          "name": "bitmapB",
> +          "node": "drive0"
>           },

[..]

> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index cbedfaf1df..76ad52b358 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -76,15 +76,16 @@ def qemu_img(*args):
>           sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))
>       return exitcode
>   
> -def ordered_kwargs(kwargs):
> -    # kwargs prior to 3.6 are not ordered, so:
> -    od = OrderedDict()
> -    for k, v in sorted(kwargs.items()):
> -        if isinstance(v, dict):
> -            od[k] = ordered_kwargs(v)
> -        else:
> -            od[k] = v
> -    return od
> +def ordered_qmp(qmsg):
> +    # Dictionaries are not ordered prior to 3.6, therefore:
> +    if isinstance(qmsg, list):
> +        return [ordered_qmp(atom) for atom in qmsg]
> +    if isinstance(qmsg, dict):
> +        od = OrderedDict()
> +        for k, v in sorted(qmsg.items()):
> +            od[k] = ordered_qmp(v)
> +        return od

or if you want:
pairs = ((k, ordered_qmp(v)) for k, v in sorted(qmsg.items()))
return OrderedDict(pairs)

> +    return qmsg
>   
>   def qemu_img_create(*args):
>       args = list(args)
> @@ -491,7 +492,7 @@ class VM(qtest.QEMUQtestMachine):
>       def qmp_log(self, cmd, filters=[], indent=None, **kwargs):
>           full_cmd = OrderedDict((
>               ("execute", cmd),
> -            ("arguments", ordered_kwargs(kwargs))
> +            ("arguments", ordered_qmp(kwargs))
>           ))
>           log(full_cmd, filters, indent=indent)
>           result = self.qmp(cmd, **kwargs)
> 

anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH] iotests/236: fix transaction kwarg order
Posted by John Snow 5 years, 2 months ago 

On 1/31/19 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:
> 31.01.2019 5:26, John Snow wrote:
>> It's not enough to order the kwargs for consistent QMP log output,
>> we must also sort any sub-dictionaries in lists that appear as values.
>>
>> Reported-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/qemu-iotests/236.out    | 56 +++++++++++++++++------------------
>>   tests/qemu-iotests/iotests.py | 21 ++++++-------
>>   2 files changed, 39 insertions(+), 38 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/236.out b/tests/qemu-iotests/236.out
>> index 1dad24db0d..bb2d71ea5e 100644
>> --- a/tests/qemu-iotests/236.out
>> +++ b/tests/qemu-iotests/236.out
>> @@ -45,23 +45,23 @@ write -P0xcd 0x3ff0000 64k
>>       "actions": [
>>         {
>>           "data": {
>> -          "node": "drive0",
>> -          "name": "bitmapB"
> 
> We could notice during review, that things we are trying to order so hard
> are just not ordered in result :)))
> 

I was in a bit of a rush before the holidays :(

>> +          "name": "bitmapB",
>> +          "node": "drive0"
>>           },
> 
> [..]
> 
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index cbedfaf1df..76ad52b358 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -76,15 +76,16 @@ def qemu_img(*args):
>>           sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))
>>       return exitcode
>>   
>> -def ordered_kwargs(kwargs):
>> -    # kwargs prior to 3.6 are not ordered, so:
>> -    od = OrderedDict()
>> -    for k, v in sorted(kwargs.items()):
>> -        if isinstance(v, dict):
>> -            od[k] = ordered_kwargs(v)
>> -        else:
>> -            od[k] = v
>> -    return od
>> +def ordered_qmp(qmsg):
>> +    # Dictionaries are not ordered prior to 3.6, therefore:
>> +    if isinstance(qmsg, list):
>> +        return [ordered_qmp(atom) for atom in qmsg]
>> +    if isinstance(qmsg, dict):
>> +        od = OrderedDict()
>> +        for k, v in sorted(qmsg.items()):
>> +            od[k] = ordered_qmp(v)
>> +        return od
> 
> or if you want:
> pairs = ((k, ordered_qmp(v)) for k, v in sorted(qmsg.items()))
> return OrderedDict(pairs)
> 

Ah, well, it already went in -- I don't think I'll send a fixup, but I
do like this way more.

Thanks for the review!

>> +    return qmsg
>>   
>>   def qemu_img_create(*args):
>>       args = list(args)
>> @@ -491,7 +492,7 @@ class VM(qtest.QEMUQtestMachine):
>>       def qmp_log(self, cmd, filters=[], indent=None, **kwargs):
>>           full_cmd = OrderedDict((
>>               ("execute", cmd),
>> -            ("arguments", ordered_kwargs(kwargs))
>> +            ("arguments", ordered_qmp(kwargs))
>>           ))
>>           log(full_cmd, filters, indent=indent)
>>           result = self.qmp(cmd, **kwargs)
>>
> 
> anyway,
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.