[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method |
Date: |
Fri, 24 May 2019 13:57:45 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 5/24/19 8:38 AM, Max Reitz wrote:
> 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 <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.
>
> 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
>
Yeah, I see. OK; I have prepared a patch that can be applied
independently after this series, if you'd like to stage this as-is. The
new patch changes behavior of this function a little, but is backwards
compatible with no changes because nobody was using these edge cases.
--js
[Qemu-devel] [PATCH v3 2/5] iotests.py: do not use infinite waits, John Snow, 2019/05/23
[Qemu-devel] [PATCH v3 5/5] iotests: add iotest 250 for testing blockdev-backup across iothread contexts, John Snow, 2019/05/23
Re: [Qemu-devel] [PATCH v3 0/5] blockdev-backup: don't check aio_context too early, Max Reitz, 2019/05/23
Re: [Qemu-devel] [PATCH v3 0/5] blockdev-backup: don't check aio_context too early, Max Reitz, 2019/05/27