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”?
Yes, that's an oversight on my part. I'm updating it to:
"Get a list of available QMP events and clear the list of pending events." and adding your RB.
(Also, perhaps renaming it to get_new_events, but that’s an even weaker
suggestion.)
I tried to avoid churn in the iotests as much as I could manage, so I think I will leave this be for now. I have an impression that the number of cases where one might wish to see events that have already been witnessed is actually quite low, so I am not sure that it needs disambiguation from a case that is essentially never used. (I have certainly been wrong before, though.)
Anyway:
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
~ thankyou ~