Repository navigation
fix(fc): make pause/resume real disk snapshots, not just a vCPU freeze - #981
Merged
Merged
Conversation
Pause's doc comment promised a snapshot-then-delete (matching every other provider's cost-saving pause), but for fc it was only ever a live, in-memory vCPU freeze with the process left running — no snapshot, no compute savings, and nothing portable to another host. Pause now writes a full Firecracker snapshot (state + memory) to the VM's state dir, stops the process, and tears down its TAP device. Resume starts a fresh process and reloads it from that snapshot, remapping the TAP device via Firecracker's network_overrides (TAP devices are host-local and never survive a snapshot). Since the VM's state dir already holds its rootfs image, copying it to another identically-configured host and running `onctl resume` there is now enough to relocate a microVM. Also fixes loadAndReconcile, which would have wrongly flipped paused VMs to "dead" now that pause legitimately stops their process.
![chatgpt-codex-connector[bot]](/p/https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f8e046f75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address Codex review comments on PR #981: if saveFCMetadata fails after Pause has already snapshotted and stopped the VMM, or after Resume has already reloaded it, the on-disk record was left silently stale, and a retried Resume could start a second competing process. Resume now tears down the process/TAP it just started; Pause's error now says explicitly that the snapshot is still usable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaxWizDbb5vGpa9uGaEKbD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
onctl pause's help text already promised "snapshot the VM's disk, preserve its IP, then delete the VM" — matching every other provider's cost-saving pause — but for thefc(Firecracker) provider it was only ever a live, in-memory vCPU freeze with the process left running. No disk snapshot was written, no compute was actually freed, and there was nothing portable to move a microVM to another host.Pausenow writes a full Firecracker snapshot (device state + guest memory, via/snapshot/create) into the VM's state directory, stops the firecracker process, and tears down its TAP device — genuinely no compute cost while paused, like every other provider.Resumestarts a fresh firecracker process and reloads it from that snapshot via/snapshot/load, remapping the TAP device through Firecracker'snetwork_overrides(TAP devices are host-local and never survive a snapshot/restore).StateDirand runningonctl resumethere is now enough to relocate a microVM — this was the actual motivating use case (portable snapshots of long-running fc microVMs).loadAndReconcilewould have wrongly flipped paused VMs to"dead"now that pause legitimately stops their process (previously the process stayed alive, just frozen).Test plan
go build ./...go vet ./...golangci-lint run ./pkg/cloud/... ./internal/providerfc/...— 0 issuesgo test ./pkg/cloud/... ./internal/providerfc/...— pass, including new coverage for pause-when-not-alive, snapshot-create failure (rolls back to Resumed instead of leaving the VM stuck frozen), and resume with a missing snapshot filego test ./...— pass except two pre-existing failures in./cmd(TestReadConfig_NoConfigDirectory,TestCreateFlagsBindToViper) confirmed present onmainbefore this change, unrelatedTESTING-fc.md's existingsudo ONCTL_CLOUD=fc onctl pause mv2 && sudo ONCTL_CLOUD=fc onctl resume mv2), including copying a paused VM's state dir to a second host and resuming it there🤖 Generated with Claude Code