On 23.05.19 20:03, John Snow wrote:
>
>
> On 5/23/19 1:49 PM, Max Reitz wrote:
>> On 23.05.19 19:06, John Snow wrote:
>>> Instead of event_wait which looks for a single event, add an events_wait
>>> which can look for any number of events simultaneously. However, it
>>> will still only return one at a time, whichever happens first.
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> ---
>>> python/qemu/__init__.py | 69 +++++++++++++++++++++++++++++------------
>>> 1 file changed, 49 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
>>> index 81d9657ec0..98ed8a2e28 100644
>>> --- a/python/qemu/__init__.py
>>> +++ b/python/qemu/__init__.py
>>> @@ -402,42 +402,71 @@ class QEMUMachine(object):
>>> self._qmp.clear_events()
>>> return events
>>>
>>> - def event_wait(self, name, timeout=60.0, match=None):
>>> + @staticmethod
>>> + def event_match(event, match=None):
>>> """
>>> - Wait for specified timeout on named event in QMP; optionally filter
>>> - results by match.
>>> + Check if an event matches optional match criteria.
>>>
>>> - The 'match' is checked to be a recursive subset of the 'event';
>>> skips
>>> - branch processing on match's value None
>>> - {"foo": {"bar": 1}} matches {"foo": None}
>>> - {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
>>> + The match criteria takes the form of a matching subdict. The event
>>> is
>>> + checked to be a superset of the subdict, recursively, with matching
>>> + values whenever those values are not None.
>>> +
>>> + Examples, with the subdict queries on the left:
>>> + - None matches any object.
>>> + - {"foo": None} matches {"foo": {"bar": 1}}
>>> + - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
>>
>> Pre-existing, but the difference between “bar” and “baz” confused me
>> quite a bit.
>>
>> Also, I wonder... {"foo": None} would not match {"foo": 1}, right?
>> Does that make sense? Shouldn’t None be the wildcard here in general?
>> (Also pre-existing of course.)
>>
>> But this patch doesn’t make things worse, so:
>>
>> Reviewed-by: Max Reitz <mre...@redhat.com>
>>
>> (I’d still like your opinion.)
>>
>
> I knew I was inviting trouble by trying to re-document this.
>
> The intention I had when writing the docs, which I think are wrong now,
> was for {"foo": None} to match {"foo": 1}, but I think you're right that
> it won't because '1' isn't a dict, so it tests for equality instead.
>
> So I need to fix this one up a little bit, but I'll take the review as a
> sign that this approach seems workable.
I think the comment is technically completely correct. It’s just that
(1) it’s hard to discern “bar” from “baz”, and (2) if {"foo": None}
intentionally does not match {"foo": 1}, we may want to document that,
because it isn’t intuitively clear from the description. If it’s a bug,
the code should be fixed (and it should still be documented).
Max
signature.asc
Description: OpenPGP digital signature
