Add Windows snapshots and forks - #433
Conversation
ddd1d35 to
7886185
Compare
7886185 to
2723b54
Compare
19ab53d to
ec253b0
Compare
ec253b0 to
8c18bc9
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8c18bc9. Configure here.
| resetWindowsTPM := source.State != StateStandby | ||
| if err := m.prepareWindowsForkIdentity(&forkMeta, resetWindowsTPM); err != nil { | ||
| return nil, false, err | ||
| } |
There was a problem hiding this comment.
Stopped Windows forks keep parent TPM
High Severity
TPM reset is keyed only on whether the source is currently standby, not on whether the child will cold-boot. A running Windows source can only be forked with target_state=Stopped, but that path standbys first and then treats the child as a memory fork, so resetWindowsTPM stays false. applyForkTargetState later discards the snapshot without clearing the copied TPM directory. The child therefore cold-boots with the parent's endorsement key, despite stopped-fork semantics promising a new TPM identity.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8c18bc9. Configure here.
| return fmt.Errorf("%w: Windows snapshot restore requires instance %s with the same captured vsock CID to be stopped", ErrInvalidState, instance.Id) | ||
| } | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Windows vsock CID check is racy
Medium Severity
ensureWindowsVsockCIDAvailable only snapshots currently running instances and does not reserve the captured VioSock CID. Memory-restored Windows forks inherit that CID, and restore of source versus child uses different instance locks, so two restores can both pass the check and start QEMU with the same guest-cid. That breaks the invariant that no two concurrently runnable instances share a Vsock CID and can fail as a host vsock collision instead of a state error.
Additional Locations (1)
Triggered by learned rule: Stopped forks must get fresh VsockCIDs before they can be started
Reviewed by Cursor Bugbot for commit 8c18bc9. Configure here.
| } | ||
| if isWindowsPlatform(source.Platform) && source.State == StateRunning && targetState != StateStopped { | ||
| return nil, "", false, fmt.Errorf("%w: Windows forks from a running source require target_state=%s", ErrNotSupported, StateStopped) | ||
| } |
There was a problem hiding this comment.
BitLocker fork check runs after standby
Medium Severity
The new Windows from-running gate only rejects a non-Stopped target state before mutating the source. validateWindowsForkPolicy runs later inside forkInstanceFromStoppedOrStandby, after the running source has already been put into standby. A fork of a Windows VM whose image is unlabeled or reseal-required therefore pauses and snapshots a live guest, then fails admission. Restore is best-effort; if it fails the source is left in standby.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8c18bc9. Configure here.
1 participant
summary
bitlocker=disabledtargeted tests
Long-running KVM and full CI suites were not run for this review pass.
Note
High Risk
Changes core VM lifecycle (snapshots, forks, TPM, vsock CIDs) and guest machine identity; incorrect semantics could cause CID collisions, shared TPM attestation, or unsafe forked BitLocker disks.
Overview
Enables standby, restore, stopped snapshots, and forks for Windows QEMU instances by removing the prior blanket rejection of snapshot lifecycle operations on
windows/amd64.Fork admission and BitLocker: Windows images can declare
io.hypeman.machine-image.bitlocker(disabledvsreseal-required). Onlydisabledimages may be forked; the policy is stored on the instance at create time. Same-instance snapshots remain allowed for other policies because Hypeman does not clone BitLocker-encrypted disks without resealing to a child TPM.Fork identity semantics: Stopped (cold) forks clear copied TPM state, assign a new vsock CID on start, and call the guest agent to set a new
MachineGuid. Memory (standby) forks keep the parent’s TPM and captured VioSock CID in guest memory; restore blocks concurrent use when another running instance shares that CID, and running forks from a live Windows source requiretarget_state=Stopped(or stopping the snapshot source before a running standby fork).Guest agent: Adds
RebindIdentitygRPC (registryMachineGuid+ Hypeman instance marker) and host-sideRebindInstanceIdentity, invoked on start/restore whenWindowsIdentityPendingis set after a fork.CI and docs: New KVM integration test for stopped forks alongside the expanded lifecycle test;
docs/windows-snapshots.mdand image label docs describe TPM, vsock, and identity behavior.Reviewed by Cursor Bugbot for commit 8c18bc9. Bugbot is set up for automated code reviews on this repo. Configure here.