fix(clone): replace removed --on-demand with --restore-mode#36
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates vk-cocoon’s clone argument generation to match cocoon’s newer vm clone interface by replacing the removed --on-demand flag with --restore-mode copy|ondemand|mmap, and wires a node-wide restore mode configuration through startup and provider clone paths.
Changes:
- Replaced
CloneOptions.OnDemandwith aRestoreModeenum and addedParseRestoreModevalidation. - Updated clone argv construction to emit
--restore-mode <mode>for Cloud Hypervisor semantics while suppressing it for Firecracker (and emitting nothing forcopy/empty to preserve compatibility with older cocoon builds). - Added
VK_RESTORE_MODEconfiguration plumbing (defaultondemand), Windows override-to-copypolicy, updated unit tests, and documented the new env var.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vm/runtime.go | Introduces RestoreMode enum, parse/validation helper, and updates CloneOptions to use it. |
| vm/cocoon_cli.go | Switches clone argv emission from --on-demand to conditional --restore-mode <mode>. |
| vm/cocoon_cli_test.go | Updates clone argv table tests and adds ParseRestoreMode test coverage. |
| provider/cocoon/update.go | Uses restore-mode policy helper for hibernation wake clone path. |
| provider/cocoon/update_test.go | Replaces old on-demand policy test with restore-mode policy test. |
| provider/cocoon/provider.go | Adds node-wide RestoreMode field and defaults it to ondemand. |
| provider/cocoon/create.go | Applies restore-mode policy to all clone paths and defines restoreModeFor (Windows forced to copy). |
| provider/cocoon/create_test.go | Updates expectations for clone options to check RestoreMode. |
| main.go | Adds VK_RESTORE_MODE env var, validates via vm.ParseRestoreMode, and wires into provider. |
| docs/configuration.md | Documents VK_RESTORE_MODE, including Windows/Firecracker behavior and mmap caveats. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cocoon#127 removed the --on-demand flag in favor of --restore-mode copy|ondemand|mmap, so every non-Windows CH clone (normal, fork, from-dir, hibernation wake) fails at argv parse against current cocoon. CloneOptions.OnDemand becomes a RestoreMode enum. The node-wide default comes from VK_RESTORE_MODE (invalid values abort startup) and defaults to ondemand — the same restore semantics shipped today, so the compatibility fix carries no behavior change. Windows stays on copy (lazy restore stalls DHCP boot) and firecracker never receives the flag. copy emits no flag at all, keeping the argv valid on cocoon builds predating --restore-mode. mmap is opt-in only: it needs a CH build with mmap restore support and fails clones on any other build.
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.
Refs #34.
Problem
cocoon#127 removed
--on-demandfromvm clonein favor of--restore-mode copy|ondemand|mmap. vk-cocoon emitted--on-demandon every non-Windows Cloud Hypervisor clone path (normal, fork, from-dir, hibernation wake), so all of them fail at argv parse against current cocoon.Change
CloneOptions.OnDemand bool→RestoreModeenum (copy/ondemand/mmap), emitted as--restore-mode <mode>in the slot--on-demandoccupied.copy(and empty) emits no flag — cocoon omitsmemory_restore_modefor it, so the argv stays valid on cocoon builds predating the flag.--nics); an empty backend keeps its existing treated-as-CH semantics.copy(lazy restore stalls DHCP boot), everything else uses the node-wide mode.VK_RESTORE_MODE, defaultondemand— the same restore semantics shipped today, so the fix carries zero behavior delta. Invalid values abort startup.mmapis opt-in only: it requires a Cloud Hypervisor build with mmap restore support (cocoonstack/cloud-hypervisordev); on other CH builds clones fail. Documented indocs/configuration.md.The hibernation-wake path needs no carve-out for
ondemand/mmap: cocoon hardlinks the snapshot memory files into the VM run dir and CH restores from there, so the post-wakesnapshot rmonly unbinds the store name while the inode stays alive.Evidence
git grep -- '--on-demand' -- '*.go'→ zero matches; noOnDemandfield remains.copy/ondemand/mmapon CH, FC suppression (plain and from-dir), empty-backend-as-CH, and NIC combination;ParseRestoreModehas its own table.make lint0 issues on linux+darwin;go test -race ./...all packages ok;make buildok.