Skip to content

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

Open
gayadav wants to merge 1 commit into
HCK-CI:masterfrom
gayadav:fix-qmp-wait-event-stale-cache
Open

qmp: drain stale cached events before waiting for new ones#1112
gayadav wants to merge 1 commit into
HCK-CI:masterfrom
gayadav:fix-qmp-wait-event-stale-cache

Conversation

@gayadav

@gayadav gayadav commented Aug 19, 2026

Copy link
Copy Markdown

QEMU emits two DEVICE_DELETED events per device_del (child + parent device). In cycled functest hotplug tests, wait_for consumed only the first event, leaving the second in the cache. On the next cycle, wait_for found the stale cached event and returned immediately without waiting for the current cycle's actual DEVICE_DELETED, allowing device_add to race ahead and fail with "Duplicate device ID".

Change wait_for to drain all matching cached events before blocking on the QMP socket for a fresh event. This ensures each qmp_wait_event step waits for the event from the current operation, not a leftover from a previous cycle.

Verified on Win2016x64_gui (the failing platform) — all 3 hotplug cycles now show real ~1s waits instead of <1ms stale cache hits.

QEMU emits two DEVICE_DELETED events per device_del (child +
parent device). In cycled functest hotplug tests, wait_for
consumed only the first event, leaving the second in the cache.
On the next cycle, wait_for found the stale cached event and
returned immediately without waiting for the current cycle's
actual DEVICE_DELETED, allowing device_add to race ahead and
fail with "Duplicate device ID".

Change wait_for to drain all matching cached events before
blocking on the QMP socket for a fresh event. This ensures each
qmp_wait_event step waits for the event from the current
operation, not a leftover from a previous cycle.

Verified on Win2016x64_gui (the failing platform) — all 3
hotplug cycles now show real ~1s waits instead of <1ms stale
cache hits.

Signed-off-by: Gaurav Yadav <gayadav@redhat.com>
cached = find_cached_event(name, accepted)
return cached if cached

drain_cached_events(name, accepted)

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.

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.

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?

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 ?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants