Re: [PATCH v2 10/16] python/machine.py: Handle None events in event_wait

[John Snow <jsnow@redhat.com>]

On 6/4/20 10:20 AM, Kevin Wolf wrote:
> Am 02.06.2020 um 23:45 hat John Snow geschrieben:
>> If the timeout is 0, we can get None back. Handle this explicitly.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> Subject line: This is events_wait(), not event_wait(). Both functions
> exist.
> 
>> @@ -562,6 +564,8 @@ def _match(event):
>>          # Poll for new events
>>          while True:
>>              event = self._qmp.pull_event(wait=timeout)
>> +            if event is None:
>> +                break
>>              if _match(event):
>>                  return event
>>              self._events.append(event)
> 
> Hm... How could this ever work? I guess we just never really tested
> whether timeouts actually time out?
> 

It's weirder than you think.

pull_event, when a timeout is supplied, will pass that timeout to
QEMUMonitorProtocol.__get_events, which populates the received event
queue but does not return data on the stack. If there are no events in
the queue and we are given a timeout, we will raise QMPTimeoutError if
no event appears in that timeframe.

pull_event() will only return None in the case that you don't give it a
timeout or tell it to wait.

The typing of events_wait allows us to specify a timeout of 0 here,
which is treated as no-wait down the stack, which may return None if
there was no such event ready.

Thus, break and also return None.

(I should update the docstring here.)

> (It's still somewhat unintuitive that receiving an unrelated event
> resets the timeout, but not the problem of this series...)
> 
> Kevin
> 

Yes, it's weird. It's not a timeout on this one event, it's a timeout on
event-receiving activity. In practice, it works quite well for the
iotest suite where there are often not other event drivers present. It
was a quick hack (that I introduced!) to ensure that iotests would halt
in some finite amount of time.

--js