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

[Eric Blake]

On 12/19/18 12:57 PM, John Snow wrote:
> On 12/19/18 1:52 PM, Eric Blake wrote:
> > 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.
>
> Yeah, it's because dicts are unordered and prior to 3.6 they may shuffle
> around arbitrarily based on internal hashes.
>
> kwargs in particular are unordered- the order we send over the wire may
> or may not reflect the order the test author wrote in their iotest.

Which shouldn't matter to what QMP executes, but MIGHT matter in getting 
reproducible log output.

> Therefore, it's a way to get consistent ordering.
>
> Now, we CAN just rely on sort_keys=True to be done with it, however,
> this puts arguments before execute, and it's less nice to read -- and
> I'd have to change a LOT of test output.

Okay, I'm finally seeing it; the existing code has:

    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
        logmsg = '{"execute": "%s", "arguments": %s}' % \
            (cmd, json.dumps(kwargs, sort_keys=True))

where our log is outputting a message that resembles a dict where 
"execute": is the first key, and the user's input dict is then sorted 
(the top-most output of {} is unsorted, but the nested ones are sorted, 
and possibly in a different order than the user typed them, but at least 
deterministic). But when you change to the user passing a full QMP 
command (rather than just a dict for the arguments of the QMP command), 
using sort_keys=True will sort everything _including_ putting 
"arguments" before "execute" (which is deterministic but changes log 
output); while using sort_keys=False will not affect output in newer 
python where kwargs remains ordered, but risks nondeterministic output 
on older python.

> This way keeps the order you expect to see while maintaining a strict
> order for the arguments. I think that's the nicest compromise until we
> can stipulate 3.6+ in the year 2034 where kwargs *are* strictly ordered.

But kwargs strict ordering is only at the top-level, not recursive, 
right? That is, even if you can rely on:

foo(b=1, a=2)

providing kwargs where 'b' always outputs before 'a', I don't see how 
that would help:

foo(b={'d':3, 'c':4})

not reorder the keys within 'b' without your recursive ordering.

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