Re: [Qemu-block] [Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method

[John Snow]

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 <address@hidden>
>> ---
>>  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 <address@hidden>
> 
> (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.

--js