qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 04/15] python/qmp: clear events on get_events() call


From: Hanna Reitz
Subject: Re: [PATCH 04/15] python/qmp: clear events on get_events() call
Date: Fri, 17 Sep 2021 14:51:30 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 17.09.21 07:40, John Snow wrote:
All callers in the tree *already* clear the events after a call to
get_events(). Do it automatically instead and update callsites to remove
the manual clear call.

These semantics are quite a bit easier to emulate with async QMP, and
nobody appears to be abusing some emergent properties of what happens if
you decide not to clear them, so let's dial down to the dumber, simpler
thing.

Specifically: callers of clear() right after a call to get_events() are
more likely expressing their desire to not see any events they just
retrieved, whereas callers of clear_events() not in relation to a recent
call to pull_event/get_events are likely expressing their desire to
simply drop *all* pending events straight onto the floor. In the sync
world, this is safe enough; in the async world it's nearly impossible to
promise that nothing happens between getting and clearing the
events.

Making the retrieval also clear the queue is vastly simpler.

Signed-off-by: John Snow <jsnow@redhat.com>
---
  python/qemu/machine/machine.py | 1 -
  python/qemu/qmp/__init__.py    | 4 +++-
  python/qemu/qmp/qmp_shell.py   | 1 -
  3 files changed, 3 insertions(+), 3 deletions(-)

[...]

diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index 269516a79b..ba15668c25 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -374,7 +374,9 @@ def get_events(self, wait: bool = False) -> 
List[QMPMessage]:
          @return The list of available QMP events.
          """
          self.__get_events(wait)
-        return self.__events
+        events = self.__events
+        self.__events = []
+        return events

Maybe it’s worth updating the doc string that right now just says “Get a list of available QMP events”?

(Also, perhaps renaming it to get_new_events, but that’s an even weaker suggestion.)

Anyway:

Reviewed-by: Hanna Reitz <hreitz@redhat.com>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]