Skip to content

cloudhypervisor: converge a VM left paused by a killed capture - #176

Merged
CMGS merged 2 commits into
masterfrom
fix/converge-orphaned-pause
Jul 29, 2026
Merged

cloudhypervisor: converge a VM left paused by a killed capture#176
CMGS merged 2 commits into
masterfrom
fix/converge-orphaned-pause

Conversation

@tonicmuroq

Copy link
Copy Markdown
Contributor

A snapshot save or hibernate killed inside its pause window leaves the guest
frozen with nobody left to resume it: WithPausedVM's resume runs on the way out,
and a SIGKILLed process never gets there. Every later device op then failed on
that stale pause, because ensureNotPaused refused outright:

Error: vm is paused (snapshot or hibernate in flight); retry after it completes

For vk-cocoon that was terminal. Its resumed hibernate re-runs vm net --nics 0
first, which was already a no-op (the NIC had been dropped before the save), and
the refusal happened before the target==current early return -- so the wedge was
reachable through the ordinary systemctl restart vk-cocoon path and needed a
hand-written vm.resume on the API socket to clear.

Paused observed by these callers has no owner, and that is provable rather than
assumed: every pause window in cocoon runs under the VM ops lock end to end
(prepareSnapshot, prepareRestore), clone resumes a record still in creating which
runningVMClientWithRecord rejects, and all three ensureNotPaused callers already
hold that same lock. So a caller holding the lock cannot be racing a live
capture. convergeOrphanedPause resumes and returns a re-read vm.info, which
callers need since they classify devices off it. resumeVM was already idempotent,
and pauseVM already carried the mirror-image fix ("swallows CH's Paused->Paused
500 so a stuck-paused VM recovers") -- the capture path anticipated this, the
device path did not.

Verified on internal-cocoon-node-7: stop vk 3s into a save, then restart. The
resumed hibernate now reports hibernate_total{netresize,ok} and runs through
snapshot/push/remove to Suspended, where the old build logged netresize failures
until the pod was abandoned. The guest marker written before the hibernate was
present after the wake.

Surfaced by fault-injection against vk-cocoon (cocoonstack/vk-cocoon#58).

A `snapshot save` or `hibernate` killed inside its pause window leaves the guest
frozen with nobody left to resume it: WithPausedVM's resume runs on the way out,
and a SIGKILLed process never gets there. Every later device op then failed on
that stale pause, because ensureNotPaused refused outright:

    Error: vm is paused (snapshot or hibernate in flight); retry after it completes

For vk-cocoon that was terminal. Its resumed hibernate re-runs `vm net --nics 0`
first, which was already a no-op (the NIC had been dropped before the save), and
the refusal happened before the target==current early return -- so the wedge was
reachable through the ordinary `systemctl restart vk-cocoon` path and needed a
hand-written vm.resume on the API socket to clear.

Paused observed by these callers has no owner, and that is provable rather than
assumed: every pause window in cocoon runs under the VM ops lock end to end
(prepareSnapshot, prepareRestore), clone resumes a record still in creating which
runningVMClientWithRecord rejects, and all three ensureNotPaused callers already
hold that same lock. So a caller holding the lock cannot be racing a live
capture. convergeOrphanedPause resumes and returns a re-read vm.info, which
callers need since they classify devices off it. resumeVM was already idempotent,
and pauseVM already carried the mirror-image fix ("swallows CH's Paused->Paused
500 so a stuck-paused VM recovers") -- the capture path anticipated this, the
device path did not.

Verified on internal-cocoon-node-7: stop vk 3s into a save, then restart. The
resumed hibernate now reports hibernate_total{netresize,ok} and runs through
snapshot/push/remove to Suspended, where the old build logged netresize failures
until the pod was abandoned. The guest marker written before the hibernate was
present after the wake.
…less proof

TestConvergeOrphanedPause sat below the fixture helpers (asl testorder
violation) and wedged newCHStubClient's doc comment away from its func.
The converge warn now names the VM it resumes — every call site holds
the resolved record. The convergeOrphanedPause comment states the
complete proof in two lines: the ops lock covers every capture window
end to end, and the Running-record gate in runningVMClientWithRecord is
what keeps the restore and clone pause windows (record Stopped/creating)
unreachable.
@CMGS

CMGS commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Pushed a review commit on top (d8817ab), plus the investigation the PR body's proof implied but didn't state:

Review commit

  • TestConvergeOrphanedPause sat below the fixture helpers (our asl testorder gate flags it) and split newCHStubClient's doc comment from its func; moved into the test section, comment reattached.
  • The converge warn now names the VM it resumes (every call site holds the resolved record) and drops the no-verb Warnf.
  • The ownerless-proof comment is compressed to two lines and strengthened: the second half of the proof is the Running-record gate in runningVMClientWithRecord — restore pauses its fresh VMM while the record still reads Stopped (FinalizeRestore flips it only after resume), clone while creating, so those windows are structurally unreachable from the device ops, not just policy-excluded.

Interrupted-restore leftover (record Stopped, VMM paused, .restore-dirty): no code added, deliberately. vm start refuses loudly on the dirty tombstone and points at restore/rm; a restore retry admits Stopped, kills the paused VMM and clears the marker; rm kills via procScan; clone's window is #173's ReconcileStaleCreate. Retry converges, failure is loud, residue is bounded — adding a daemon-side guard for it would be defending an unobserved scenario.

Consciously skipped (from the quality round): replacing the recovery branch's second vm.info read with a synthesized struct copy — sub-ms on a per-incident cold branch, and re-reading CH's actual state is the more truthful form.

Gates on the branch: go test green, golangci-lint 0 issues and asl clean on both GOOS. Branch prod delta is +8 lines net.

@CMGS
CMGS merged commit dd799e9 into master Jul 29, 2026
4 checks passed
@CMGS
CMGS deleted the fix/converge-orphaned-pause branch July 29, 2026 09:29
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.

2 participants