Skip to content

qmp: drain stale cached events before waiting for new ones #1112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gayadav wants to merge 1 commit into
base: master
Choose a base branch
from
Open

qmp: drain stale cached events before waiting for new ones #1112

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions lib/setupmanagers/qemuhck/qmp.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,17 @@ def run_cmd(cmd, arguments = nil)
send_cmd(cmd, arguments)
end

# accepted is an array of acceptable values; this returns as soon as
# any one of them is seen.
# Drain stale matching events from the cache, then block until a
# fresh event arrives on the QMP socket.
def wait_for(name, accepted, timeout = 60)
cached = find_cached_event(name, accepted)
return cached if cached

drain_cached_events(name, accepted)

@elizashurov elizashurov Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing these lines can cause test timeouts. It can delete valid events that arrived early and were cached, leaving wait_for waiting for an event that already happened.

@elizashurov elizashurov Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of draining the cache, we could tag events with a counter.
We can increment it on every run_cmd call and store the current value with each cached event. Then wait_for would only consider events from the current run and ignore older ones as stale.
That way we don’t need to drain the cache, and we also avoid accidentally dropping an event that just arrived.

@elizashurov elizashurov Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kostyanf14 what do you think?

@harshapa-rh harshapa-rh Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a fix for a similar issue here: #1099.
Do you want to check if that works, @gayadav ?

@kostyanf14 kostyanf14 Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gayadav @harshapa-rh As you both have a similar issue in the same place. Please work together and propose a fix that covers both use cases.

wait_for_new_event(name, accepted, timeout)
end

private

def find_cached_event(name, accepted)
index = @events.index { |e| accepted.include?(e[name]) }
@events.delete_at(index) if index
def drain_cached_events(name, accepted)
@events.reject! { |e| accepted.include?(e[name]) }
end

def wait_for_new_event(name, accepted, timeout)
Expand Down
Loading