Re: [Qemu-devel] [PATCH v4 1/5] iotests: add qmp recursive sorting function

[Eric Blake]

On 12/18/18 7:52 PM, John Snow wrote:
> Python before 3.6 does not sort kwargs by default.
> If we want to print out pretty-printed QMP objects while
> preserving the "exec" > "arguments" ordering, we need a custom sort.

Naive question - why do we need the sorting? Is it so that the output is 
deterministic?  Surely it can't be because the ordering otherwise makes 
a difference to execution.

Here's my review from a non-Python export viewpoint:

> We can accomplish this by sorting **kwargs into an OrderedDict,
> which does preserve addition order.
> ---
>   tests/qemu-iotests/iotests.py | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)

> @@ -448,8 +460,9 @@ class VM(qtest.QEMUQtestMachine):
>           return result
>   
>       def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
> -        logmsg = '{"execute": "%s", "arguments": %s}' % \
> -            (cmd, json.dumps(kwargs, sort_keys=True))
> +        full_cmd = OrderedDict({"execute": cmd,
> +                                "arguments": ordered_kwargs(kwargs)})
> +        logmsg = json.dumps(full_cmd)

Vladimir knows Python better than me, but once you fix this OrderedDict 
construction up to actually work, the patch looks reasonable to me.

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