Skip to content

hypervisor: fix virtio device livelock on snapshot restore by kicking queues and injecting interrupt on resume#1084

Open
lisongqian wants to merge 4 commits into
TencentCloud:masterfrom
lisongqian:fix-resume-kick
Open

hypervisor: fix virtio device livelock on snapshot restore by kicking queues and injecting interrupt on resume#1084
lisongqian wants to merge 4 commits into
TencentCloud:masterfrom
lisongqian:fix-resume-kick

Conversation

@lisongqian

@lisongqian lisongqian commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

After a VM is snapshotted and later restored, virtio devices can wedge in a livelock:

  • A restored virtqueue may already contain pending descriptors at the moment the guest was snapshotted.
  • On the host side, the queue eventfd signal is not persistent snapshot state — it is only re-emitted when the guest issues a fresh kick.
  • After restore, cloud-hypervisor unparks the worker thread and then waits for a queue eventfd to fire. If the guest had already notified the queue before the snapshot, it won't notify again, so the worker sits idle.
  • At the same time, the guest is waiting on an interrupt for the in-flight request to complete, so it never kicks.

Both sides end up waiting for each other. In practice this reproduces as:

  • virtio-blk flush stalls during early boot after snapshot/restore.
  • virtio-net traffic freezes after resume unless the old driver_awake workaround is used.

Fix

Cherry-pick three upstream cloud-hypervisor patches that fix the resume-path kick properly in the shared VirtioCommon layer, and then retire the ad-hoc driver_awake workaround in virtio-net:

  1. virtio-devices: signal activated queue eventfds on resume
    VirtioCommon now keeps clones of the activated queue eventfds and signals each of them once on resume, right after unparking the worker threads. This guarantees the worker re-checks the queue even if the guest already kicked before the snapshot.
    (cherry-picked from cloud-hypervisor f7c3b775819f)

  2. virtio-devices: trigger interrupt into guest on resume
    Also inject a used-ring interrupt into the guest on resume, so the guest re-checks any request it thought was in flight instead of waiting forever for a notification that already happened before the snapshot.
    (cherry-picked from cloud-hypervisor abf32187f342)

  3. virtio-devices: net: Remove "driver_awake" workaround for restore
    With the generic resume path above, virtio-net no longer needs its own device-specific driver_awake hack — the shared kick + interrupt already covers it.
    (cherry-picked from cloud-hypervisor 5d02dc7c1ddd)

Scope

  • Files touched: hypervisor/virtio-devices/src/device.rs, hypervisor/virtio-devices/src/net.rs.
  • No behavior change on the cold-boot / activate path — only affects the resume() path used by snapshot/restore.
  • All three patches are straight cherry-picks from upstream cloud-hypervisor with proper Signed-off-by / (cherry picked from commit ...) trailers preserved.

Testing

  • Snapshot + restore a VM whose virtio-blk had a flush in flight at snapshot time — no longer stalls in early boot.
  • virtio-net traffic resumes correctly after restore without the removed driver_awake workaround.

Coffeeri and others added 3 commits July 22, 2026 17:14
@Coffeeri @lisongqian
virtio-devices: signal activated queue eventfds on resume
e4b9dfb 
A restored virtqueue can already contain pending descriptors when the VM
resumes. Before this change, the worker thread was unparked and then
waited for a fresh queue eventfd signal. That is normally fine, but not
when the queue was already non-empty at snapshot time. The virtqueue
state lives in guest memory and is restored, but the original host-side
queue eventfd signal is not persistent snapshot state. If the guest
already notified the queue before the snapshot, it may not notify it
again after resume.

That can leave the worker idle while the guest is still waiting for the
pending request to complete. In one observed case, this stalled a
virtio-blk flush during early boot after snapshot/restore.

We mitigate this in the shared `VirtioCommon` resume path.
`VirtioCommon` retains cloned queue eventfds for activated virtqueues
and signals each of them once on resume after unparking the worker
threads.

Keep virtio-net on its existing special-case path: it resumes worker
threads without signaling queue eventfds so the `driver_awake`
workaround remains intact until the guest performs a real notify.

On-behalf-of: SAP leander.kohler@sap.com
Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de>
(cherry picked from commit f7c3b775819f181284fe1272543a0f196f029ad1)
Signed-off-by: Songqian Li <sionli@tencent.com>
@rbradford @lisongqian
virtio-devices: trigger interrupt into guest on resume
a7c2bde 
This will wake up the guest and avoid a livelock situation by ensuring
that it will process any pending queues on its side.

Signed-off-by: Rob Bradford <rbradford@meta.com>
(cherry picked from commit abf32187f342828901fd405e0cc07e3f1a8eb2a9)
Signed-off-by: Songqian Li <sionli@tencent.com>
@rbradford @lisongqian
virtio-devices: net: Remove "driver_awake" workaround for restore
e57f938 
Now on the generic restore path the worker thread is notified on the
events and also the guest is notified via the interrupt. This avoids the
same "livelock" situation that required this "driver_awake" workaround
when restoring the net device.

Signed-off-by: Rob Bradford <rbradford@meta.com>
(cherry picked from commit 5d02dc7c1ddde0b53fcba70be2cce979c9efe50b)
Signed-off-by: Songqian Li <sionli@tencent.com>
@lisongqian
lisongqian requested a review from up2wing as a code owner July 22, 2026 10:06
@cubesandboxbot

cubesandboxbot Bot commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Review: PR #1084 — Fix virtio device livelock on snapshot restore

Overall verdict: ✅ Ship it. The change is correct, well-documented, and follows upstream cloud-hypervisor fixes. The logic is sound and the driver_awake retirement is a clean simplification.

What the PR does

Three cherry-picked patches that fix a snapshot/restore livelock in the shared VirtioCommon layer, then remove the now-unnecessary driver_awake workaround from virtio-net:

  1. device.rs — On resume, signal each activated queue eventfd so worker threads re-examine queues that may hold pending descriptors from before the snapshot.
  2. device.rs — On resume, inject a used-ring interrupt for each queue so the guest driver re-checks for completions rather than waiting forever for a lost notification.
  3. net.rs — Remove the driver_awake hack, since the generic resume path now covers what it was working around.

Correctness verification

All devices benefit. Every virtio device type in this codebase calls self.common.activate(&queues, &interrupt_cb)?; and delegates resume() through self.common.resume(). The fix is universal — virtio-block, virtio-fs, virtio-console, virtio-rng, etc. all receive the queue kick and interrupt injection on resume at no extra cost.

reset() is safe. The added self.queue_evts.clear() runs before self.resume() inside reset(). This is intentional — during device shutdown we want to unpark threads so they see the kill_evt signal, but we do not want to kick queues or inject interrupts. The clear() makes those loops no-ops.

Real queue index vs. position. The code correctly stores (queue_index, EventFd) tuples and uses VirtioInterruptType::Queue(queue_index) rather than the position in the queue_evts vec. This handles non-contiguous queue configurations correctly (e.g., virtio-net where only a subset of queue pairs are enabled and the control queue sits at index 2N). The inline comment in the diff explains this clearly.

#[derive(Default)] compatibility. The new queue_evts: Vec<(u16, EventFd)> field defaults to an empty vec via Default, so no constructor changes are needed anywhere.

Eventfd cloning is sound. EventFd::try_clone() creates a new fd referencing the same kernel eventfd object. Writing to the clone on resume is visible to the worker thread's read() on the original. The cloned eventfds share the same lifetime as the VirtioCommon struct and are cleaned up naturally.

driver_awake removal is correct. The old workaround forced signal_used_queue on every TX/RX tap event until the driver first kicked after restore. The new generic approach writes to each queue eventfd and injects an interrupt directly — so the initial driver kick is no longer needed to break the livelock. All three driver_awake = true assignments (RX_QUEUE_EVENT, TX_QUEUE_EVENT, TX_TAP_EVENT) and both || !self.driver_awake guards are correctly removed.

MigratableError::Resume usage is consistent with existing patterns in vhost_user/mod.rs, vdpa.rs, and watchdog.rs — all use Resume(anyhow!(...)).

Minor observations (not blockers)

  • The queue_evts field is pub, consistent with the rest of VirtioCommon's fields. The struct is already effectively treated as a bag of shared state; adding another pub field doesn't worsen the situation, though it could be documented as "populated by activate(), consumed by resume(), cleared by reset()" in a future refactor.

  • The diff does not add or modify tests. However, since these are cherry-picked upstream fixes with clear manual testing described in the PR body, and the code paths are exercised by existing snapshot/restore tests, this is acceptable.

Suggested inline comment

None — the code is well-commented, especially the block explaining why real queue indices must be used rather than positional indices. That comment would be easy to leave out, and including it shows good engineering judgment.

@chenhengqi

chenhengqi commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Just curious, where did these commit hashes come from?

For example, the third commit should be 72fc0976f113284ee229bc657926d3b712f74e6a.
cloud-hypervisor/cloud-hypervisor@72fc097

@lisongqian
virtio-devices: fix resume interrupt for non-contiguous ready queues
6423722 
VirtioCommon::resume signals each activated queue's eventfd and then
injects an interrupt so a restored guest driver does not livelock on a
used-ring completion that was lost across snapshot/restore. The
interrupt loop, however, used the position in `queue_evts` as the queue
index:

    for i in 0..self.queue_evts.len() {
        interrupt_cb.trigger(VirtioInterruptType::Queue(i as u16)).ok();
    }

`VirtioInterruptType::Queue(x)` is defined to carry the real virtio
queue index, and `VirtioInterruptMsix` uses it to look up
`queues_vectors`, which is sized to `total_queues` and populated by the
guest driver at real-index positions. When the set of ready queues is
not contiguous from 0, the position in `queue_evts` differs from the
real queue index. The mismatched index then either lands on a
`VIRTQ_MSI_NO_VECTOR` slot (silently dropping the interrupt) or on an
unrelated queue's vector.

The most realistic trigger is multi-queue virtio-net with a control
queue: when the guest enables only a subset of the queue pairs, the
ready set looks like {0, 1, ..., 2k-1, 2N} where 2N is the control
queue's real index. On resume the loop stops at position 2k and never
touches index 2N, so the control queue interrupt is dropped and the
driver can hang waiting for a control command completion. This is
especially likely right after live migration, when the announce
(gratuitous ARP/RARP) path exercises the control queue.

Preserve the real queue index alongside the eventfd by changing
`VirtioCommon::queue_evts` from `Vec<EventFd>` to
`Vec<(u16, EventFd)>`. `activate` fills the tuple from the
`(queue_index, Queue, EventFd)` slice it already receives, and `resume`
triggers `VirtioInterruptType::Queue(real_index)` for every activated
queue. Ready queues that are contiguous from 0 continue to work
unchanged (their real index equals their position).

`VirtioCommon` keeps `#[derive(Default)]` and `Vec<(u16, EventFd)>` has
an empty default, so the ~15 call sites that build `VirtioCommon` with
`..Default::default()` need no changes.

The upstream cloud-hypervisor PR #8004 that introduced this resume path
carries the same defect; this fix applies to that upstream code as
well.

Reported-by: Yi Wang <foxywang@tencent.com>
Signed-off-by: Songqian Li <sionli@tencent.com>
@lisongqian

lisongqian commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Just curious, where did these commit hashes come from?

For example, the third commit should be 72fc0976f113284ee229bc657926d3b712f74e6a. cloud-hypervisor/cloud-hypervisor@72fc097

From this PR: cloud-hypervisor/cloud-hypervisor#8004

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

Reviewers

@up2wing up2wing Awaiting requested review from up2wing up2wing is a code owner

Assignees

@lisongqian lisongqian

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lisongqian @chenhengqi @Coffeeri @rbradford