| Message ID | 20200602214528.12107-11-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | python: add mypy support to python/qemu | expand |
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 still somewhat unintuitive that receiving an unrelated event resets the timeout, but not the problem of this series...) Kevin
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
diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 2f12cebde40..a835b7550da 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -32,7 +32,7 @@ from types import TracebackType from . import qmp -from .qmp import SocketAddrT +from .qmp import SocketAddrT, QMPMessage LOG = logging.getLogger(__name__) @@ -553,6 +553,8 @@ def _match(event): return True return False + event: Optional[QMPMessage] + # Search cached events for event in self._events: if _match(event): @@ -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)
If the timeout is 0, we can get None back. Handle this explicitly. Signed-off-by: John Snow <jsnow@redhat.com> --- python/qemu/machine.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)