support clone and restore from specific dir#22
Merged
Conversation
Move the canonical snapshot.json envelope read/write into the snapshot package so cmd/vm can consume an envelope-bearing directory directly (needed for the upcoming \`vm clone --from-dir\` and \`vm restore --from-dir\` paths) without depending on localfile internals. - snapshot.SnapshotJSONName + envelopeVersion (single const block). - snapshot.ReadSnapshotEnvelope(dir) parses + version-checks; returns ErrEnvelopeMissing when the file is absent so callers can surface a precise message instead of a generic open error. - snapshot.WriteSnapshotEnvelope(dir, cfg) writes via AtomicWriteJSON. - localfile/export.go now references snapshot.SnapshotJSONName. - localfile/import.go readAndRemoveSnapshotJSON delegates the parse to the shared helper; remove-after-read semantics preserved.
Adds the snapshot.DirectoryExporter optional interface and a LocalFile.ExportToDir implementation. The dir form pairs with the upcoming \`vm clone --from-dir\` and \`vm restore --from-dir\`, giving users an rsync-friendly handoff path without a tar round-trip. - snapshot.DirectoryExporter: ExportToDir(ctx, ref, dir) error - LocalFile.ExportToDir: rejects non-empty target dirs to avoid silently merging into an unrelated tree, writes a fresh snapshot.json envelope via WriteSnapshotEnvelope, then ReflinkCopy's each data file. Reflink is zero-cost on btrfs/xfs and falls back to plain copy elsewhere; the result is always standalone (rsync-friendly), unlike the hardlinks DirectClone uses internally for memory pages. - cmd/snapshot: --to-dir flag, mutually exclusive with --output/--gzip.
Lets users clone from any directory containing a snapshot.json envelope (e.g. an extracted export.tar, an rsync'd \`snapshot export --to-dir\` result, or an NFS-mounted golden image) without first registering the snapshot in the local DB. - cloneCmd args relaxed to MaximumNArgs(1); positional SNAPSHOT and --from-dir are mutually exclusive (validated in the handler so the error message names both forms). - cloneFromDir reads the envelope, routes to the matching backend by cfg.Hypervisor, asserts hypervisor.Direct, then funnels through the same DirectClone path as the snapshot-DB form. - cloneDirect refactored to share cloneFromSrcDir with cloneFromDir so the prepareClone → DirectClone → finalize sequence isn't duplicated. The dir is read-only across the call so multiple clones of the same dir (golden image use case) are safe. Trust boundary unchanged: existing ValidateMetaPaths still rejects envelopes whose storage paths escape the local rootDir/runDir.
Symmetric with \`vm clone --from-dir\` but with an extra safety check: the envelope's snapshot ID must belong to vm.SnapshotIDs unless --force. This keeps the typical cross-host "sync the same VM's backup over here" flow zero-friction while making "use unrelated state to overwrite this VM" an explicit, opt-in operation (data-loss footgun otherwise). - restoreCmd args relaxed to RangeArgs(1, 2); positional SNAPSHOT and --from-dir are mutually exclusive (handler-validated). - restoreFromDir reads the envelope, resolves the VM's hypervisor, asserts hypervisor.Direct, runs the same NIC-count + resource-flag validation as the snapshot-DB form, then DirectRestore over the dir. - --force does NOT add the envelope's ID to vm.SnapshotIDs — restoring doesn't change ownership semantics, the foreign snapshot remains external to this VM's lineage.
Bullet point in features + extended export flag table + a worked example covering the three flows: golden image clone, cross-host same-VM restore, force-overriding a foreign-lineage restore.
Comments per SKILL.md should explain WHY, not narrate or restate the signature. This pass keeps the load-bearing rationale (atomic write reason, soft-gate semantics, reflink-vs-hardlink trade-off) and collapses the rest.
ExportToDir wrote snapshot.json before copying payload files. A concurrent \`vm restore --from-dir\` reading the envelope could pass preflight (which only stat()s presence, not byte-completeness) and kill the running VM while a memory-range or COW file was still mid-copy, leaving the source VM in Error state with a partial restore. Move WriteSnapshotEnvelope to after the copy loop so envelope existence is now an all-data-ready marker. ensureEmptyDir already guarantees the dir starts empty, so a stale envelope can't survive into the new export.
Returning *types.SnapshotConfig forced an awkward \`return &envelope.Config\` and a \`*cfg\` deref inside Write. SnapshotConfig is small (no identity, embedded Config + a few scalars + a map ref) so copying is cheap; switching to value drops the pointer dance in the helper bodies. Callers that need a pointer downstream now take \`&cfg\` once at the call site, which is symmetric with how every other local SnapshotConfig in cocoon is handled. readAndRemoveSnapshotJSON in snapshot/localfile/import.go follows the same shift; the Import flow now embeds cfg by value into the SnapshotRecord literal instead of dereferencing.
snapshot.Direct.DataDir, snapshotRecordToConfig, CloneVMConfigFromFlags, RestoreVMConfigFromFlags, mergeResourceFlags, prepareClone, cloneFromSrcDir, validateFCCloneOverrides — all of these previously took or returned *types.SnapshotConfig but only ever read fields. The chain forced \`return &cfg\` / \`*cfg\` / \`&cfg\` dances at every layer. Switch to value semantics inside the cmd-layer + snapshot-backend boundary; the hypervisor backend interface (Clone / DirectClone) keeps *SnapshotConfig because that's the wider stable API surface, and the single deref happens at that boundary. SnapshotConfig is small (no identity, embedded Config + a few scalars + a map ref) so the copy is cheap and pointer-only mattered for sharing semantics that nothing actually used here.
CMGS
force-pushed
the
feat/clone-from-dir
branch
from
5f468d3 to
88d82bd
Compare
Mirrors the DataDir flip: the snap-DB-side Restore was the only place in the cmd/vm clone path that still forced \`*cfg\`/\`&cfg\` dancing. Drop the pointer return on the interface, value-out from LocalFile, and the cmd-side caller derefs once at the hypervisor boundary just like DirectClone.
Clone and Restore had near-identical mutex / required-arg blocks; the only difference was the leading-positional count (0 for clone where SNAPSHOT is the only arg, 1 for restore where args[0] is VM). Capture that as baseArgs and let the helper return (fromDir, snapRef) with exactly one non-empty.
Single caller (the new ExportToDir); the helper added a function-call indirection without earning it. Inline the 8-line check; the comment above the block carries the rationale (no silent merge into an unrelated tree).
- ExportToDir: replace inline mkdir + 3-state switch with utils.EnsureDirs (existing 0o750 helper); drop the dead snapshot.json skip in the copy loop since registered DataDir results never carry the envelope (Create doesn't write it; Import removes it via readAndRemoveSnapshotJSON). - run.go: 'nic count mismatch' -> 'NIC count mismatch' (ALL-CAPS acronym rule); fixes both restoreFromDir and the pre-existing legacy Restore message for consistency. - cloneFromDir: copy *config.Config locally before flipping UseFirecracker so the mutation doesn't leak to the caller's shared conf (CLI tolerates it; daemons embedding cocoon would notice). - restoreCmd: PreRunE rejects --force without --from-dir so misuse fails loud instead of silently no-op.
…reFromDir restoreDirect (snapshot-DB path) and restoreFromDir (--from-dir path) duplicated the wantJSON-log + DirectRestore + output tail. Pull it out into a single helper, parameterized by sourceLabel. -7 lines net.
… check Two strict-walk findings: 1. snapshot/localfile/export.go:80 had a literal Version: 1 that should reference the snapshot package's version constant. Promote envelopeVersion -> EnvelopeVersion (exported) and use snapshot.EnvelopeVersion at the cross-package call site. Future format bumps now have one source of truth. 2. localfile.LocalFile didn't carry a compile-time assertion against snapshot.DirectoryExporter (the new interface). Add it alongside the existing Snapshot/Direct/CompressedExporter checks so signature drift in either direction surfaces at build time.
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
1 participant
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.
No description provided.