Filed by review of the hibernate/restore path (follow-up to #89).
Problem
vm restore on a stopped (hibernated) VM is the wake path. It runs KillForRestore first to clear any residual VMM:
// hypervisor/restore.go
func (b *Backend) KillForRestore(ctx, vmID, rec, terminate, runtimeFiles) error {
killErr := b.WithRunningVM(ctx, rec, terminate)
if killErr != nil && !errors.Is(killErr, ErrNotRunning) {
b.MarkError(ctx, vmID) // <-- brick point
return fmt.Errorf("stop running VM: %w", killErr)
}
...
}
WithRunningVM (hypervisor/state.go) fails closed when the /proc liveness scan itself errors:
scanned, scanErr := utils.FindVMMByCmdline(b.Conf.BinaryName(), sockPath)
if scanErr != nil {
return fmt.Errorf("vm %s: pidfile-based check failed and /proc scan errored: %w ...", rec.ID, scanErr)
}
For a hibernated VM there is no live VMM (hibernate terminated it; a terminate failure would already have marked the record error, so state == stopped implies the VMM is gone). So a transient host /proc read error during the wake pre-check is a false liveness-unknown, not a real missed kill — yet KillForRestore treats it as a fatal kill failure and calls MarkError. ResolveForRestore then refuses the error-state VM, so every subsequent wake retry fails permanently until the sandbox is reaped. Nothing on disk was mutated.
Trigger
Narrow: requires FindVMMByCmdline to return an error (host procfs read failure/permission/hiccup) at the exact moment of a hibernated VM's wake. Rare and transient, but bricks a perfectly-restorable VM.
Proposed fix (symmetric with #89)
KillForRestore already has rec. Reuse the FailRestore helper introduced in #89 instead of the unconditional MarkError:
- b.MarkError(ctx, vmID)
+ b.FailRestore(ctx, vmID, rec.State) // stopped origin spared, running origin still MarkError
- running origin (in-place revert of a live VM): a failed kill of a genuinely-live VMM stays unknown state →
MarkError (unchanged, correct).
- stopped origin (hibernate wake): spared → stays
stopped, wake remains retryable.
This makes the kill step consistent with the rest of the restore path that #89 already made origin-aware. Small change + a unit test asserting a stopped-origin kill failure leaves state stopped.
Filed by review of the hibernate/restore path (follow-up to #89).
Problem
vm restoreon a stopped (hibernated) VM is the wake path. It runsKillForRestorefirst to clear any residual VMM:WithRunningVM(hypervisor/state.go) fails closed when the/procliveness scan itself errors:For a hibernated VM there is no live VMM (hibernate terminated it; a terminate failure would already have marked the record error, so
state == stoppedimplies the VMM is gone). So a transient host/procread error during the wake pre-check is a false liveness-unknown, not a real missed kill — yetKillForRestoretreats it as a fatal kill failure and callsMarkError.ResolveForRestorethen refuses the error-state VM, so every subsequent wake retry fails permanently until the sandbox is reaped. Nothing on disk was mutated.Trigger
Narrow: requires
FindVMMByCmdlineto return an error (host procfs read failure/permission/hiccup) at the exact moment of a hibernated VM's wake. Rare and transient, but bricks a perfectly-restorable VM.Proposed fix (symmetric with #89)
KillForRestorealready hasrec. Reuse theFailRestorehelper introduced in #89 instead of the unconditionalMarkError:MarkError(unchanged, correct).stopped, wake remains retryable.This makes the kill step consistent with the rest of the restore path that #89 already made origin-aware. Small change + a unit test asserting a stopped-origin kill failure leaves state
stopped.