From c94d958779507bc89d0364d301f90ce005aa8408 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:20:39 +0800 Subject: [PATCH 01/25] fix(utils): a TarDirStream failure between entries reaches the reader TarDirStream closed the tar writer after a TarDir error, so a failure between two entries wrote the end-of-archive trailer and ExtractTar saw a valid, shorter archive. PersistSnapshotDir's stream fallback then finalized a snapshot with files missing and deleted the source dir, because the error only surfaced on the deferred Close it discards. --- utils/stream.go | 7 +++---- utils/stream_test.go | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/utils/stream.go b/utils/stream.go index 08edc72e..253225d9 100644 --- a/utils/stream.go +++ b/utils/stream.go @@ -45,11 +45,10 @@ func PipeStream(cleanup func(), write func(io.Writer) error) io.ReadCloser { func TarDirStream(dir string, cleanup func()) io.ReadCloser { return PipeStream(cleanup, func(w io.Writer) error { tw := tar.NewWriter(w) - err := TarDir(tw, dir) - if closeErr := tw.Close(); err == nil { - err = closeErr + if err := TarDir(tw, dir); err != nil { + return err } - return err + return tw.Close() }) } diff --git a/utils/stream_test.go b/utils/stream_test.go index 126a84c4..61ca89f8 100644 --- a/utils/stream_test.go +++ b/utils/stream_test.go @@ -3,10 +3,30 @@ package utils import ( "bytes" "io" + "os" + "path/filepath" "strings" "testing" ) +func TestTarDirStreamFailsTheReaderOnAnUnreadableEntry(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root reads mode-000 files") + } + src := t.TempDir() + if err := os.WriteFile(filepath.Join(src, "a-config.json"), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "b-memory-range"), []byte("mem"), 0o000); err != nil { + t.Fatal(err) + } + stream := TarDirStream(src, nil) + defer stream.Close() //nolint:errcheck + if err := ExtractTar(t.TempDir(), stream); err == nil { + t.Fatal("ExtractTar accepted an archive that is missing b-memory-range") + } +} + func TestPeekReader_FullRead(t *testing.T) { src := strings.NewReader("hello world") head, full, err := PeekReader(src, 5) From 04b65c33d7ed81da6c93d9a5971f203f2dbc6d84 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:21:01 +0800 Subject: [PATCH 02/25] fix(core): the json backend refuses a root whose meta.db holds the records Setting meta_backend=json before running meta convert opened an empty json store over live sqlite data: every process in that window saw no VMs, so the cgroup, CNI, TAP and vmlock GC modules reclaimed live VMs' resources and new records landed in the wrong engine. The sqlite side already refuses to bootstrap over legacy json; this is the mirror. --- cmd/core/metastore.go | 5 +++++ cmd/core/metastore_test.go | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/cmd/core/metastore.go b/cmd/core/metastore.go index c824fada..9e6403db 100644 --- a/cmd/core/metastore.go +++ b/cmd/core/metastore.go @@ -2,6 +2,7 @@ package core import ( "context" + "fmt" "path/filepath" "slices" "sync" @@ -132,6 +133,10 @@ func MetaStore(conf *config.Config) (meta.Store, error) { metaStore, metaErr = openSQLiteStore(ctx, conf, dbPath) return } + if utils.FileExists(dbPath) { + metaErr = fmt.Errorf("meta_backend is json but %s holds sqlite metadata; run `cocoon meta convert`", dbPath) + return + } if conf.MetaBackend == "" { log.WithFunc("core.MetaStore").Info(ctx, "legacy json meta store in use; `cocoon meta convert` upgrades it to sqlite") } diff --git a/cmd/core/metastore_test.go b/cmd/core/metastore_test.go index c92bafa9..b17e8a34 100644 --- a/cmd/core/metastore_test.go +++ b/cmd/core/metastore_test.go @@ -96,6 +96,22 @@ func TestMetaStoreRepairsCrashedBootstrap(t *testing.T) { resetMetaStore() } +func TestMetaStoreRefusesJSONOverSQLiteData(t *testing.T) { + resetMetaStore() + conf := testConf(t) + if _, err := MetaStore(conf); err != nil { + t.Fatalf("bootstrap sqlite root: %v", err) + } + CloseMetaStore(t.Context()) + resetMetaStore() + conf.MetaBackend = config.MetaBackendJSON + if _, err := MetaStore(conf); err == nil { + t.Fatal("json backend opened over a live meta.db and would hide its records") + } + CloseMetaStore(t.Context()) + resetMetaStore() +} + func TestResolveMetaBackend(t *testing.T) { conf := testConf(t) if b := ResolveMetaBackend(conf); b != "sqlite" { From 3ff73bb843e0c35013a3899c177ecece49b77cc4 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:22:17 +0800 Subject: [PATCH 03/25] fix(hypervisor): a launch keeps a quiesce pending until the VMM is up Start and restore un-quiesce a VM's plumbing before the VMM runs. A SIGKILL in that window (vk-cocoon's exec.CommandContext sends one on cancel) left a stopped record with no QuiescePending, so nothing ever brought the veth back down and the TC redirect pointed at a TAP no VMM held, the #130 softirq condition. The placement write now schedules the quiesce, restore places before it recovers the network, and the transitions to running clear it as before. Clone placeholders are excluded: a dead clone is reclaimed as a stale create. Start also checks the record invariants before placing it, so a record it rejects no longer keeps its cpus counted in the host placement. --- hypervisor/placement.go | 3 +++ hypervisor/restore.go | 8 ++++---- hypervisor/start.go | 8 ++++---- hypervisor/start_test.go | 36 ++++++++++++++++++++++++++++++++++-- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/hypervisor/placement.go b/hypervisor/placement.go index 75a53ac8..8ae35a20 100644 --- a/hypervisor/placement.go +++ b/hypervisor/placement.go @@ -28,6 +28,9 @@ func (b *Backend) placeRecord(ctx context.Context, id string, cfg *types.Config) if err := b.placeVM(ctx, t.r, r, cfg, topo); err != nil { return err } + if r.State != types.VMStateCreating && needsQuiesce(r) { + r.QuiescePending = true + } if err := t.Put(id, r, meta.RelaxedOK); err != nil { return err } diff --git a/hypervisor/restore.go b/hypervisor/restore.go index 720381e6..4418daae 100644 --- a/hypervisor/restore.go +++ b/hypervisor/restore.go @@ -142,15 +142,15 @@ func (b *Backend) restoreCore(ctx context.Context, run restoreRun) (*types.VM, e if err := run.apply(run.rec); err != nil { return err } - // Inside the ops lock, like StartSequence: a hibernate resume after a host reboot finds no plumbing left to un-quiesce. - if err := b.RecoverNetwork(ctx, run.rec); err != nil { - return fmt.Errorf("recover network: %w", err) - } placed, err := b.placeRecord(ctx, run.vmID, &run.vmCfg.Config) if err != nil { return fmt.Errorf("place VM: %w", err) } run.rec.CPUSet, run.rec.QueueCPUs = placed.CPUSet, placed.QueueCPUs + // Inside the ops lock, like StartSequence: a hibernate resume after a host reboot finds no plumbing left to un-quiesce. + if err := b.RecoverNetwork(ctx, run.rec); err != nil { + return fmt.Errorf("recover network: %w", err) + } var afterErr error result, afterErr = run.afterExtract(ctx, run.vmID, run.vmCfg, run.rec) return afterErr diff --git a/hypervisor/start.go b/hypervisor/start.go index 9cec3117..29dfeae4 100644 --- a/hypervisor/start.go +++ b/hypervisor/start.go @@ -45,10 +45,6 @@ func (b *Backend) StartSequence(ctx context.Context, id string, scan *utils.Proc if rec == nil { return nil } - if vErr := validateRecordInvariants(rec); vErr != nil { - b.MarkError(ctx, id) - return vErr - } // Inside the ops lock so a concurrent stop's late quiesce cannot bring this VM's plumbing down after it is up. if err = b.RecoverNetwork(ctx, rec); err != nil { b.markFailedOperation(ctx, id, true) @@ -114,6 +110,10 @@ func (b *Backend) PrepareStart(ctx context.Context, id string, scan *utils.ProcS return nil, fmt.Errorf("ensure dirs: %w", err) } CleanupRuntimeFiles(ctx, rec.RunDir, spec.RuntimeFiles) + if err = validateRecordInvariants(&rec); err != nil { + b.MarkError(ctx, id) + return nil, err + } placed, err := b.placeRecord(ctx, id, &rec.Config.Config) if err != nil { return nil, fmt.Errorf("place VM: %w", err) diff --git a/hypervisor/start_test.go b/hypervisor/start_test.go index f4fe6a5e..02c36146 100644 --- a/hypervisor/start_test.go +++ b/hypervisor/start_test.go @@ -44,6 +44,7 @@ func TestStartSequenceRejectsNilNIC(t *testing.T) { seedStoppedVMWithDirs(t, b, id) if err := b.dbUpdate(ctx, func(idx *VMIndex) error { idx.VMs[id].NetworkConfigs = []*types.NetworkConfig{nil} + idx.VMs[id].Config.CPUSetCPUs = "0" return nil }); err != nil { t.Fatalf("seed nil NIC: %v", err) @@ -62,8 +63,39 @@ func TestStartSequenceRejectsNilNIC(t *testing.T) { if err != nil { t.Fatalf("load record: %v", err) } - if loaded.State != types.VMStateError { - t.Errorf("state = %s, want error", loaded.State) + if loaded.State != types.VMStateError || loaded.CPUSet != "" { + t.Errorf("state = %s cpuset = %q, want error and no placement", loaded.State, loaded.CPUSet) + } +} + +func TestStartSequenceLaunchWindowKeepsAQuiescePending(t *testing.T) { + b, _ := newMeteringTestBackend(t) + const id = "vm-launch-window" + seedStoppedVMWithDirs(t, b, id) + if err := b.dbUpdate(t.Context(), func(idx *VMIndex) error { + idx.VMs[id].NetSetup = types.NetSetup{ + NetBackend: types.BackendCNI, + NetworkConfigs: []*types.NetworkConfig{{}}, + } + return nil + }); err != nil { + t.Fatalf("seed network: %v", err) + } + b.SetNetwork(stubNetwork{}) + + err := b.StartSequence(t.Context(), id, nil, StartSpec{ + Launch: func(context.Context, *VMRecord, string) (int, error) { + if rec := recordOf(t, b, id); !rec.QuiescePending { + t.Error("a kill before the VMM runs would leave the plumbing up with no quiesce pending") + } + return 4242, nil + }, + }) + if err != nil { + t.Fatalf("StartSequence: %v", err) + } + if rec := recordOf(t, b, id); rec.State != types.VMStateRunning || rec.QuiescePending { + t.Fatalf("state = %s pending = %v, want running and cleared", rec.State, rec.QuiescePending) } } From 0e1162232f03c134d8e2e660b8ad6453e9d31baf Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:22:42 +0800 Subject: [PATCH 04/25] fix(hypervisor): a failed migrate round resumes before it ends the series A canceled capture returns while CH is still writing the dump, and CH serves its API one request at a time. Ending the series first sent that request on the 30s client timeout behind the dump; CH drops a request whose client has gone, so the series stayed open with dirty logging on and the directory was deleted, leaving abort nothing to act on. Resume raises the timeout and waits out the dump, then the series ends. --- hypervisor/migrate.go | 4 ++-- hypervisor/migrate_test.go | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hypervisor/migrate.go b/hypervisor/migrate.go index c57b2742..cdb1ead8 100644 --- a/hypervisor/migrate.go +++ b/hypervisor/migrate.go @@ -68,7 +68,7 @@ type roundSteps struct { afterPause func() error } -// MigrateCaptureSequence runs one round; any failure ends the series, resumes the guest and deletes the directory. +// MigrateCaptureSequence runs one round; a failed round resumes the guest, ends the series and deletes the directory. func (b *Backend) MigrateCaptureSequence(ctx context.Context, ref string, req MigrateRequest, spec MigrateSpec) (*MigrateRound, error) { vmID, rec, unlock, err := b.lockForCapture(ctx, ref) if err != nil { @@ -203,7 +203,7 @@ func (b *Backend) captureRound(ctx context.Context, rec *VMRecord, spec MigrateS } if err = b.WithRunningVM(ctx, rec, func(_ int) error { if runErr := run(); runErr != nil { - return errors.Join(runErr, spec.EndSeries(rec, hc), resumeAfterCapture(spec.Resume, rec, hc)) + return errors.Join(runErr, resumeAfterCapture(spec.Resume, rec, hc), spec.EndSeries(rec, hc)) } return nil }); err != nil { diff --git a/hypervisor/migrate_test.go b/hypervisor/migrate_test.go index 31ad5529..5121505d 100644 --- a/hypervisor/migrate_test.go +++ b/hypervisor/migrate_test.go @@ -82,6 +82,13 @@ func TestMigrateFailedRoundResumesAndDiscardsTheSeries(t *testing.T) { calls := &migrateCalls{} spec := migrateStubSpec(calls) spec.Capture = func(*VMRecord, *http.Client, string) error { return errors.New("dump failed") } + spec.EndSeries = func(_ *VMRecord, hc *http.Client) error { + calls.ended++ + if calls.resumed == 0 || hc.Timeout != VMMemTransferTimeout { + t.Error("the series ended on the short timeout before the resume waited out the dump") + } + return nil + } if _, err := b.MigrateCaptureSequence(t.Context(), id, MigrateRequest{Series: "s1", Final: true}, spec); err == nil || !strings.Contains(err.Error(), "dump failed") { t.Fatalf("capture: %v, want dump failed", err) } From 7d026caa9141eee00cc2279d51ff0e630bd2fe80 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:23:15 +0800 Subject: [PATCH 05/25] fix(firecracker): close the PTY master when the console relay fails The no-lease failure branch kept the master open in the CLI process. Nobody reads it, so a full PTY buffer can block the guest's console writes until the CLI exits; the relay child holds its own copy on the success path, so the master closes once on every path. --- hypervisor/firecracker/start.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index ca6ffbeb..20759273 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -159,11 +159,10 @@ func (fc *Firecracker) launchProcessWithLeases(ctx context.Context, rec *hypervi } leaseControl, relayErr := fc.startConsoleRelay(rec.RunDir, master, pid, leaseFiles) + _ = master.Close() switch { case relayErr == nil: - _ = master.Close() case len(leaseFiles) > 0: - _ = master.Close() fc.AbortLaunch(ctx, pid, sockPath, rec.RunDir, runtimeFiles) _ = fcCmd.Process.Kill() <-exited From 1be2f69ec6e196aa68d77277778eba5c47b7a53a Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:23:15 +0800 Subject: [PATCH 06/25] fix(vm): vm debug prints a max vCPU count no lower than --cpu --max-cpu defaults to 8 and --cpu has no upper bound, so --cpu 16 printed boot=16,max=8, which Cloud Hypervisor rejects. --- cmd/vm/debug.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/vm/debug.go b/cmd/vm/debug.go index 0fb2c0a3..760b9664 100644 --- a/cmd/vm/debug.go +++ b/cmd/vm/debug.go @@ -193,7 +193,7 @@ func printCommonCHArgs(s chDebugSpec) { if s.VMCfg.Windows { cpuExtra = ",kvm_hyperv=on" } - fmt.Printf(" --cpus boot=%d,max=%d%s \\\n", s.VMCfg.CPU, s.MaxCPU, cpuExtra) + fmt.Printf(" --cpus boot=%d,max=%d%s \\\n", s.VMCfg.CPU, max(s.MaxCPU, s.VMCfg.CPU), cpuExtra) fmt.Printf(" --memory %s \\\n", cloudhypervisor.DebugMemoryCLIArg(&s.VMCfg.Config)) fmt.Print(" --rng src=/dev/urandom \\\n") if s.Balloon > 0 { From c576e5d7938e23a4d06ab3933e6e3def165544d3 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:23:15 +0800 Subject: [PATCH 07/25] fix(cni): switch the thread back when creating a named netns fails netns.NewNamed enters the new netns before it creates and mounts the name, so a failure there returned the locked thread to the scheduler inside an empty netns, where a later exec in the same process could run host-side plugin work. --- network/cni/lifecycle_linux.go | 1 + 1 file changed, 1 insertion(+) diff --git a/network/cni/lifecycle_linux.go b/network/cni/lifecycle_linux.go index 327a82cb..5eef31fd 100644 --- a/network/cni/lifecycle_linux.go +++ b/network/cni/lifecycle_linux.go @@ -35,6 +35,7 @@ func createNetns(name string) error { ns, err := netns.NewNamed(name) if err != nil { + _ = netns.Set(origNS) return fmt.Errorf("create netns %s: %w", name, err) } _ = ns.Close() From 97864be4041916133f67b9f1a5e2ba90e8dcc2d1 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:34:35 +0800 Subject: [PATCH 08/25] cut: inline HandleStopResult and drop the cloudimg tmp-blob rename hop HandleStopResult had one caller and no user outside this repo. A qcow2 v3 source was renamed to a fixed shared temp name before commit renamed it again into the blob store; every caller already passes a temp file it owns in the same temp dir, so commit takes it directly. --- hypervisor/stop.go | 19 ++++++------------- images/cloudimg/commit.go | 8 ++------ images/cloudimg/config.go | 6 ------ 3 files changed, 8 insertions(+), 25 deletions(-) diff --git a/hypervisor/stop.go b/hypervisor/stop.go index 17165f31..354baa3b 100644 --- a/hypervisor/stop.go +++ b/hypervisor/stop.go @@ -53,13 +53,16 @@ func (b *Backend) StopOneLocked(ctx context.Context, id string, spec StopSpec) e return spec.Shutdown(ctx, &rec, sockPath, pid) }) settled := errors.Is(shutdownErr, ErrNotRunning) && rec.State == types.VMStateStopped && !hasOpenComputeInterval(&rec) - if err := b.HandleStopResult(ctx, id, rec.RunDir, spec.RuntimeFiles, shutdownErr); err != nil { - return err + if shutdownErr != nil && !errors.Is(shutdownErr, ErrNotRunning) { + b.MarkError(ctx, id) + return shutdownErr } + CleanupRuntimeFiles(ctx, rec.RunDir, spec.RuntimeFiles) + b.removeCgroupScope(ctx, id) transitioned := false if !settled { - // Warn-and-continue: the VMM is dead and the flip self-heals on the next reconcile. logger := log.WithFunc(b.Typ + ".StopOneLocked") + // Warn-and-continue: the VMM is dead and the flip self-heals on the next reconcile. if err := b.UpdateState(ctx, id, types.VMStateStopped); err != nil { logger.Warnf(ctx, "mark stopped %s: %v", id, err) } else { @@ -106,16 +109,6 @@ func (b *Backend) DeleteAll(ctx context.Context, refs []string, force bool, stop }) } -func (b *Backend) HandleStopResult(ctx context.Context, id, runDir string, runtimeFiles []string, shutdownErr error) error { - if shutdownErr != nil && !errors.Is(shutdownErr, ErrNotRunning) { - b.MarkError(ctx, id) - return shutdownErr - } - CleanupRuntimeFiles(ctx, runDir, runtimeFiles) - b.removeCgroupScope(ctx, id) - return nil -} - func (b *Backend) deleteOneLocked(ctx context.Context, id string, force bool, stopLocked VMOp, rec *VMRecord, procScan utils.ProcScan) error { sockPath := SocketPath(rec.RunDir) stoppedByUs := false diff --git a/images/cloudimg/commit.go b/images/cloudimg/commit.go index f1d338d0..a98b8de4 100644 --- a/images/cloudimg/commit.go +++ b/images/cloudimg/commit.go @@ -88,12 +88,8 @@ func prepareTmpBlob(ctx context.Context, conf *Config, tracker progress.Tracker, logger.Debugf(ctx, "detected source format: %s (compat=%q)", info.Format, info.Compat) if info.Format == "qcow2" && info.Compat == "1.1" { - tmpBlobPath := conf.tmpBlobPath(digestHex) - if err = os.Rename(sourcePath, tmpBlobPath); err != nil { - return "", fmt.Errorf("rename tmp blob: %w", err) - } - logger.Debugf(ctx, "source already qcow2 v3, renamed to %s", tmpBlobPath) - return tmpBlobPath, nil + logger.Debugf(ctx, "source already qcow2 v3: %s", sourcePath) + return sourcePath, nil } tracker.OnEvent(cloudimgProgress.Event{Phase: cloudimgProgress.PhaseConvert}) diff --git a/images/cloudimg/config.go b/images/cloudimg/config.go index 3b278b5a..07fb7d02 100644 --- a/images/cloudimg/config.go +++ b/images/cloudimg/config.go @@ -1,8 +1,6 @@ package cloudimg import ( - "path/filepath" - "github.com/cocoonstack/cocoon/images" "github.com/cocoonstack/cocoon/utils" ) @@ -25,7 +23,3 @@ func NewConfig(rootDir string, pullConns int) *Config { PullConns: utils.OrDefault(pullConns, defaultPullConns), } } - -func (c *Config) tmpBlobPath(digestHex string) string { - return filepath.Join(c.TempDir(), ".tmp-"+digestHex+".qcow2") -} From 81e59cdfe5e76a971ec8e58360c74d537338fb33 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:34:35 +0800 Subject: [PATCH 09/25] test: tests prove what they name The GC removal-failure test chmods the snapshot data dir, so it reaches the removal and checks the tombstone it keeps; the TC redirect test re-runs setup on partial plumbing; the final migrate round checks the reference file a reflink would create; teardown tests assert the operation happened. Remove tests that duplicated others or could not fail, add the missing vm migrate and no-watchdog cases, and move a logstream test next to its code. --- cmd/core/metering_test.go | 1 - cmd/core/utils_test.go | 2 +- cmd/images/handler_test.go | 4 +-- cmd/root_test.go | 2 +- cmd/vm/lifecycle_test.go | 39 ----------------------- cmd/vm/logstream_test.go | 38 ++++++++++++++++++++++ cmd/vm/run_test.go | 7 +++-- config/config_test.go | 23 +------------- daemon/reconcile_test.go | 7 ++--- hypervisor/choreo_trace_test.go | 9 ++---- hypervisor/cloudhypervisor/args_test.go | 3 -- hypervisor/firecracker/create_test.go | 2 +- hypervisor/firecracker/extend_test.go | 4 +-- hypervisor/gc_test.go | 9 ------ hypervisor/meta_test.go | 21 +++++-------- hypervisor/migrate_test.go | 2 +- hypervisor/restore_test.go | 4 +-- hypervisor/snapshot_test.go | 2 +- hypervisor/state_test.go | 8 ++--- hypervisor/stop_test.go | 3 +- hypervisor/supervisor_test.go | 4 +-- images/cloudimg/pull_test.go | 4 +-- meta/json/store_test.go | 32 ++----------------- metering/metering_test.go | 5 --- network/cni/cni_test.go | 35 +++++++++++---------- network/cni/lifecycle_linux_test.go | 12 +++++++ snapshot/localfile/gc_test.go | 32 ++++++++++--------- snapshot/localfile/localfile_test.go | 42 ------------------------- snapshot/localfile/teardown_test.go | 10 +++++- utils/extents_linux_test.go | 3 ++ utils/stream_test.go | 2 +- utils/tar_sparse_linux_test.go | 2 +- utils/tar_test.go | 16 ++++------ 33 files changed, 146 insertions(+), 243 deletions(-) diff --git a/cmd/core/metering_test.go b/cmd/core/metering_test.go index cf13337a..60029211 100644 --- a/cmd/core/metering_test.go +++ b/cmd/core/metering_test.go @@ -21,7 +21,6 @@ func TestBuildRecorderBackends(t *testing.T) { {"explicit file", config.MeteringFile, (*meteringfile.Recorder)(nil)}, {"nop", config.MeteringNop, metering.NopRecorder{}}, {"stderr", config.MeteringStderr, (*meteringstderr.Recorder)(nil)}, - {"unknown falls back to nop", "kafka", metering.NopRecorder{}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/cmd/core/utils_test.go b/cmd/core/utils_test.go index dc515865..af9ade1f 100644 --- a/cmd/core/utils_test.go +++ b/cmd/core/utils_test.go @@ -149,7 +149,7 @@ func TestReconcileStateClearsRuntimePathsOnStaleRunning(t *testing.T) { } } -// directErrSnap is a DirectCreator whose CreateFromDir always fails; the embedded interface panics on any other call, so the test proves the failure path alone. +// directErrSnap fails CreateFromDir; any other call panics on the nil embedded Snapshot. type directErrSnap struct { snapshot.Snapshot } diff --git a/cmd/images/handler_test.go b/cmd/images/handler_test.go index 6e8d7f95..2d2e1502 100644 --- a/cmd/images/handler_test.go +++ b/cmd/images/handler_test.go @@ -139,7 +139,7 @@ func TestDetectReader_GzipPreservesFullContent(t *testing.T) { func TestDetectLocalImportSource(t *testing.T) { dir := t.TempDir() - qcow2Path := writeTempFile(t, dir, "image.qcow2", append(utils.Qcow2Magic, make([]byte, 32)...)) + qcow2Path := writeTempFile(t, dir, "image.qcow2", slices.Concat(utils.Qcow2Magic, make([]byte, 32))) tarPath := writeTempFile(t, dir, "image.tar", tarWrap(t, "payload.txt", []byte("payload"))) gzipTarPath := writeTempFile(t, dir, "image.tar.gz", gzipWrap(t, tarWrap(t, "payload.txt", []byte("payload")))) invalidPath := writeTempFile(t, dir, "invalid.bin", []byte("not an archive")) @@ -177,7 +177,7 @@ func TestDetectLocalImportSource(t *testing.T) { func TestPlanLocalImportDetectsKindFromFirstFile(t *testing.T) { dir := t.TempDir() - part1 := writeTempFile(t, dir, "part-1.qcow2", append(utils.Qcow2Magic, make([]byte, 8)...)) + part1 := writeTempFile(t, dir, "part-1.qcow2", slices.Concat(utils.Qcow2Magic, make([]byte, 8))) part2 := writeTempFile(t, dir, "part-2.qcow2", []byte("second-part")) kind, err := planLocalImport([]string{part1, part2}) diff --git a/cmd/root_test.go b/cmd/root_test.go index 3dd1ecdb..d0769a8f 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -123,7 +123,7 @@ func TestEveryConfigKeyRegistered(t *testing.T) { } func TestCommandGroupsRejectAnUnknownVerb(t *testing.T) { - for _, group := range [][]string{{"vm"}, {"vm", "disk"}, {"vm", "fs"}, {"vm", "device"}, {"image"}, {"snapshot"}, {"meta"}} { + for _, group := range [][]string{{"vm"}, {"vm", "disk"}, {"vm", "fs"}, {"vm", "device"}, {"vm", "migrate"}, {"image"}, {"snapshot"}, {"meta"}} { if err := executeRoot(t, group...); err != nil { t.Errorf("%v: bare group: %v, want its help", group, err) } diff --git a/cmd/vm/lifecycle_test.go b/cmd/vm/lifecycle_test.go index c4a070c2..f258c386 100644 --- a/cmd/vm/lifecycle_test.go +++ b/cmd/vm/lifecycle_test.go @@ -1,8 +1,6 @@ package vm import ( - "io" - "os" "testing" "github.com/spf13/cobra" @@ -10,43 +8,6 @@ import ( "github.com/cocoonstack/cocoon/config" ) -func TestSeekToLastNLines(t *testing.T) { - tests := []struct { - name string - body string - n int - want string - }{ - {name: "no trailing newline", body: "a\nb\nc", n: 2, want: "b\nc"}, - {name: "trailing newline", body: "a\nb\nc\n", n: 2, want: "b\nc\n"}, - {name: "more than available", body: "a\nb", n: 5, want: "a\nb"}, - {name: "one line trailing", body: "a\nb\nc\n", n: 1, want: "c\n"}, - {name: "empty", body: "", n: 3, want: ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f, err := os.CreateTemp(t.TempDir(), "log") - if err != nil { - t.Fatal(err) - } - defer f.Close() - if _, err := f.WriteString(tt.body); err != nil { - t.Fatal(err) - } - if err := seekToLastNLines(f, tt.n); err != nil { - t.Fatal(err) - } - got, err := io.ReadAll(f) - if err != nil { - t.Fatal(err) - } - if string(got) != tt.want { - t.Fatalf("got %q, want %q", got, tt.want) - } - }) - } -} - func TestApplyStopFlags(t *testing.T) { tests := []struct { name string diff --git a/cmd/vm/logstream_test.go b/cmd/vm/logstream_test.go index 69f25a4a..63142406 100644 --- a/cmd/vm/logstream_test.go +++ b/cmd/vm/logstream_test.go @@ -2,6 +2,7 @@ package vm import ( "context" + "io" "os" "path/filepath" "testing" @@ -100,3 +101,40 @@ func TestWatchFileCancel(t *testing.T) { t.Fatal("timed out waiting for channel close") } } + +func TestSeekToLastNLines(t *testing.T) { + tests := []struct { + name string + body string + n int + want string + }{ + {name: "no trailing newline", body: "a\nb\nc", n: 2, want: "b\nc"}, + {name: "trailing newline", body: "a\nb\nc\n", n: 2, want: "b\nc\n"}, + {name: "more than available", body: "a\nb", n: 5, want: "a\nb"}, + {name: "one line trailing", body: "a\nb\nc\n", n: 1, want: "c\n"}, + {name: "empty", body: "", n: 3, want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "log") + if err != nil { + t.Fatal(err) + } + defer f.Close() + if _, err := f.WriteString(tt.body); err != nil { + t.Fatal(err) + } + if err := seekToLastNLines(f, tt.n); err != nil { + t.Fatal(err) + } + got, err := io.ReadAll(f) + if err != nil { + t.Fatal(err) + } + if string(got) != tt.want { + t.Fatalf("got %q, want %q", got, tt.want) + } + }) + } +} diff --git a/cmd/vm/run_test.go b/cmd/vm/run_test.go index 484f51a4..cd3d4d73 100644 --- a/cmd/vm/run_test.go +++ b/cmd/vm/run_test.go @@ -28,6 +28,7 @@ func TestValidateBackendFlagsRejectsFirecrackerIncompatibleConfig(t *testing.T) {"shared memory", types.Config{SharedMemory: true}, "--shared-memory"}, {"hugepages", types.Config{HugePages: true}, "--hugepages"}, {"mergeable", types.Config{Mergeable: true}, "--mergeable"}, + {"no watchdog", types.Config{NoWatchdog: true}, "--no-watchdog"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -70,12 +71,12 @@ func TestCloneNICPlan(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - init, resize, err := cloneNICPlan(tt.useFC, tt.cfg, tt.override, tt.target) + initNICs, resize, err := cloneNICPlan(tt.useFC, tt.cfg, tt.override, tt.target) if (err != nil) != tt.wantErr { t.Fatalf("err = %v, wantErr %v", err, tt.wantErr) } - if !tt.wantErr && (init != tt.wantInit || resize != tt.wantResize) { - t.Fatalf("plan = (%d, %d), want (%d, %d)", init, resize, tt.wantInit, tt.wantResize) + if !tt.wantErr && (initNICs != tt.wantInit || resize != tt.wantResize) { + t.Fatalf("plan = (%d, %d), want (%d, %d)", initNICs, resize, tt.wantInit, tt.wantResize) } }) } diff --git a/config/config_test.go b/config/config_test.go index 73f97b07..722652b7 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -104,7 +104,7 @@ func TestValidate_NetScope(t *testing.T) { scope string ok bool }{ - {"", true}, {"mt", true}, {"m", false}, {"mtap", false}, {"m-", false}, {"bt", false}, {"rm", false}, + {"", true}, {"mt", true}, {"A9", true}, {"m", false}, {"mtap", false}, {"m-", false}, {"m/", false}, {"bt", false}, {"rm", false}, } { c := &Config{ RootDir: "/r", RunDir: "/r/run", LogDir: "/l", @@ -139,27 +139,6 @@ func TestScopeFamilies(t *testing.T) { } } -func TestValidateScope(t *testing.T) { - for _, tt := range []struct { - scope string - ok bool - }{ - {"", true}, - {"mt", true}, - {"A9", true}, - {"m", false}, - {"mtap", false}, - {"m-", false}, - {"m/", false}, - {"bt", false}, - {"rm", false}, - } { - if err := validateScope(tt.scope); (err == nil) != tt.ok { - t.Errorf("validateScope(%q) = %v, want ok=%v", tt.scope, err, tt.ok) - } - } -} - func TestValidate_MetaBackend(t *testing.T) { for _, tt := range []struct { backend string diff --git a/daemon/reconcile_test.go b/daemon/reconcile_test.go index 8d2bd5f0..5be8e369 100644 --- a/daemon/reconcile_test.go +++ b/daemon/reconcile_test.go @@ -3,7 +3,6 @@ package daemon import ( "context" "errors" - "fmt" "maps" "slices" "sync" @@ -190,11 +189,11 @@ func TestPassCountsDegradedVMsWithoutGoingUnready(t *testing.T) { }{ {"clean pass", func(*fakeSupervisor) {}, 0, 1}, {"busy ops lock", func(f *fakeSupervisor) { f.busy["vm1"] = struct{}{} }, 0, 0}, - {"ops lock error", func(f *fakeSupervisor) { f.lockErr = fmt.Errorf("no such directory") }, 1, 0}, - {"inconclusive liveness", func(f *fakeSupervisor) { f.observeErr = fmt.Errorf("/proc scan errored") }, 1, 0}, + {"ops lock error", func(f *fakeSupervisor) { f.lockErr = errors.New("no such directory") }, 1, 0}, + {"inconclusive liveness", func(f *fakeSupervisor) { f.observeErr = errors.New("/proc scan errored") }, 1, 0}, {"delete cannot finish", func(f *fakeSupervisor) { f.tombstoned["vm1"] = struct{}{} - f.resumeErr = fmt.Errorf("cni down") + f.resumeErr = errors.New("cni down") }, 1, 0}, } for _, tt := range tests { diff --git a/hypervisor/choreo_trace_test.go b/hypervisor/choreo_trace_test.go index 9c4fac91..11861c53 100644 --- a/hypervisor/choreo_trace_test.go +++ b/hypervisor/choreo_trace_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "slices" "testing" "time" @@ -129,14 +130,8 @@ func TestLegacyChoreographyTrace(t *testing.T) { if got.Op != g.Op || got.ErrClass != g.ErrClass || got.ErrMsg != g.ErrMsg || got.Result != g.Result { t.Errorf("step %s drift:\n got: %+v\nwant: %+v", g.Op, got, g) } - if len(got.Metering) != len(g.Metering) { + if !slices.Equal(got.Metering, g.Metering) { t.Errorf("step %s metering drift: got %v, legacy %v", g.Op, got.Metering, g.Metering) - continue - } - for j := range g.Metering { - if got.Metering[j] != g.Metering[j] { - t.Errorf("step %s metering[%d]: got %s, legacy %s", g.Op, j, got.Metering[j], g.Metering[j]) - } } } diff --git a/hypervisor/cloudhypervisor/args_test.go b/hypervisor/cloudhypervisor/args_test.go index 41c74353..0670405c 100644 --- a/hypervisor/cloudhypervisor/args_test.go +++ b/hypervisor/cloudhypervisor/args_test.go @@ -135,9 +135,6 @@ func TestCmdlineFollowsTheLiveNICs(t *testing.T) { if !strings.Contains(cfg.Payload.Cmdline, "ip=10.211.0.134::10.211.0.1:255.255.252.0:vm1:eth0:off") { t.Errorf("cmdline = %q, want the address the record now holds", cfg.Payload.Cmdline) } - if strings.Contains(cfg.Payload.Cmdline, "10.211.0.20") { - t.Errorf("cmdline = %q, still replays the address the VM was created with", cfg.Payload.Cmdline) - } } func TestPayloadFollowsBootShape(t *testing.T) { diff --git a/hypervisor/firecracker/create_test.go b/hypervisor/firecracker/create_test.go index f7c5ecdb..b0c4f9ce 100644 --- a/hypervisor/firecracker/create_test.go +++ b/hypervisor/firecracker/create_test.go @@ -79,7 +79,7 @@ func TestDecompressKernel(t *testing.T) { got, err := decompressKernel(tt.data) if tt.wantErr { if err == nil { - t.Fatalf("want error, got nil") + t.Fatal("want error, got nil") } return } diff --git a/hypervisor/firecracker/extend_test.go b/hypervisor/firecracker/extend_test.go index 06bf07dd..3217b692 100644 --- a/hypervisor/firecracker/extend_test.go +++ b/hypervisor/firecracker/extend_test.go @@ -189,10 +189,10 @@ type fcStub struct { pending []string } -func (s *fcStub) put(path string) string { +func (s *fcStub) put(endpoint string) string { s.mu.Lock() defer s.mu.Unlock() - return s.puts[path] + return s.puts[endpoint] } func (s *fcStub) deleted() []string { diff --git a/hypervisor/gc_test.go b/hypervisor/gc_test.go index ed39d704..a0f68cee 100644 --- a/hypervisor/gc_test.go +++ b/hypervisor/gc_test.go @@ -54,15 +54,6 @@ func TestGCCollectKeepsLockedVM(t *testing.T) { } } -func TestWithOpsTryLockRecreatesMissingRunDir(t *testing.T) { - b, _ := newMeteringTestBackend(t) - dir := filepath.Join(t.TempDir(), "gone") - ran := false - if !b.withOpsTryLock(t.Context(), dir, func() { ran = true }) || !ran { - t.Fatal("a missing run dir (logDir-only orphan) must not read as busy") - } -} - func TestQuarantineVMSurvivesCanceledContext(t *testing.T) { b, _ := newMeteringTestBackend(t) const id = "vm-quarantine-cancel" diff --git a/hypervisor/meta_test.go b/hypervisor/meta_test.go index 10e3e70a..943bea39 100644 --- a/hypervisor/meta_test.go +++ b/hypervisor/meta_test.go @@ -59,21 +59,21 @@ func TestLegacyDifferentialTrace(t *testing.T) { t.Fatalf("round-trip not byte-identical to legacy baseline:\n got: %s\nwant: %s", got, baseline) } - t2 := time.Date(2026, 7, 3, 12, 45, 0, 0, time.UTC) - if err := b.update(ctx, func(t *vmTx) error { - g, err := t.Get("VMCCCCCCCCCCCCCCCCCCCCCCCC") + updatedAt := time.Date(2026, 7, 3, 12, 45, 0, 0, time.UTC) + if err := b.update(ctx, func(tx *vmTx) error { + g, err := tx.Get("VMCCCCCCCCCCCCCCCCCCCCCCCC") if err != nil { return err } g.State = types.VMStateCreated - g.UpdatedAt = t2 - if err := t.Put("VMCCCCCCCCCCCCCCCCCCCCCCCC", g); err != nil { + g.UpdatedAt = updatedAt + if err := tx.Put("VMCCCCCCCCCCCCCCCCCCCCCCCC", g); err != nil { return err } - if err := t.NameDel("beta"); err != nil { + if err := tx.NameDel("beta"); err != nil { return err } - return t.Del("VMBBBBBBBBBBBBBBBBBBBBBBBB") + return tx.Del("VMBBBBBBBBBBBBBBBBBBBBBBBB") }); err != nil { t.Fatal(err) } @@ -153,12 +153,7 @@ func newTestMetaStore(t *testing.T, typ, indexFile, lockPath string) *metajson.S func testNamespace(t *testing.T, typ, dir string) meta.Store { t.Helper() - store, err := metajson.Open(metajson.Namespace{Name: VMNamespaceName(typ), FilePath: filepath.Join(dir, "index.json"), LockPath: filepath.Join(dir, "index.lock"), Codec: testVMTables}) - if err != nil { - t.Fatalf("open meta store: %v", err) - } - t.Cleanup(func() { _ = store.Close() }) - return store + return newTestMetaStore(t, typ, filepath.Join(dir, "index.json"), filepath.Join(dir, "index.lock")) } func testSQLiteNamespace(t *testing.T, typ, dir string) meta.Store { diff --git a/hypervisor/migrate_test.go b/hypervisor/migrate_test.go index 5121505d..b7ec1773 100644 --- a/hypervisor/migrate_test.go +++ b/hypervisor/migrate_test.go @@ -71,7 +71,7 @@ func TestMigrateFinalRoundLeavesTheGuestPaused(t *testing.T) { if !round.Paused || calls.resumed != 0 || calls.ended != 0 { t.Errorf("paused %t resumed %d ended %d, want true 0 0", round.Paused, calls.resumed, calls.ended) } - if utils.FileExists(filepath.Join(dir, "cow.raw.ref")) { + if utils.FileExists(nextRefPath(filepath.Join(dir, types.COWRawFileName))) { t.Error("the final round reflinked a reference inside the pause") } } diff --git a/hypervisor/restore_test.go b/hypervisor/restore_test.go index bbddf5e2..88cf9eac 100644 --- a/hypervisor/restore_test.go +++ b/hypervisor/restore_test.go @@ -65,7 +65,7 @@ func TestDirectRestoreFailureKeepsOriginContract(t *testing.T) { want types.VMState }{ {"stopped origin survives a post-populate failure", types.VMStateStopped, failAfterExtract, types.VMStateStopped}, - {"running origin quarantines on a post-populate failure", types.VMStateRunning, failAfterExtract, types.VMStateError}, + {"running origin marks error on a post-populate failure", types.VMStateRunning, failAfterExtract, types.VMStateError}, {"partial populate quarantines even a stopped origin", types.VMStateStopped, failPopulate, types.VMStateError}, } for _, tt := range tests { @@ -235,7 +235,7 @@ func TestKillForRestoreFailureKeepsOriginContract(t *testing.T) { want types.VMState }{ {"stopped origin stays restorable (hibernate wake)", types.VMStateStopped, types.VMStateStopped}, - {"running origin quarantines", types.VMStateRunning, types.VMStateError}, + {"running origin marks error", types.VMStateRunning, types.VMStateError}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/hypervisor/snapshot_test.go b/hypervisor/snapshot_test.go index 8bbe26ac..f5f61090 100644 --- a/hypervisor/snapshot_test.go +++ b/hypervisor/snapshot_test.go @@ -33,7 +33,7 @@ func TestIsUnderDir(t *testing.T) { {"exactly root", "/srv/cocoon", "/srv/cocoon", false}, {"escape via dotdot", "/srv/cocoon/../etc/shadow", "/srv/cocoon", false}, {"sibling dir", "/srv/cocoon-other/x", "/srv/cocoon", false}, - {"empty dir disables check", "/anything", "", false}, + {"empty dir fails closed", "/anything", "", false}, {"trailing slash on dir", "/srv/cocoon/x", "/srv/cocoon/", true}, {"relative path normalized", "srv/cocoon/x/../y", "srv/cocoon", true}, } diff --git a/hypervisor/state_test.go b/hypervisor/state_test.go index 27ed3dbf..7ae959eb 100644 --- a/hypervisor/state_test.go +++ b/hypervisor/state_test.go @@ -2,7 +2,7 @@ package hypervisor import ( "context" - "fmt" + "errors" "os" "path/filepath" "testing" @@ -148,7 +148,7 @@ func TestStopAfterMarkErrorEmitsComputeStop(t *testing.T) { } } -func TestDeleteForceClosesIntervalAfterMarkError(t *testing.T) { +func TestEmitDeleteCloseClosesIntervalAfterMarkError(t *testing.T) { b, rec := newMeteringTestBackend(t) ctx := t.Context() seedRunningVM(t, b, "vm1", 2, 2<<30, 20<<30) @@ -300,7 +300,7 @@ func TestDirectRestoreSequenceEmitsOnlyComputeStopOnPopulateFailure(t *testing.T SourceSnapshotID: "snap-src", Preflight: func(string, *VMRecord) error { return nil }, Kill: func(context.Context, string, *VMRecord) error { return nil }, - Populate: func(*VMRecord, string) error { return fmt.Errorf("populate boom") }, + Populate: func(*VMRecord, string) error { return errors.New("populate boom") }, AfterExtract: func(_ context.Context, _ string, _ *types.VMConfig, _ *VMRecord) (*types.VM, error) { t.Fatal("AfterExtract should not run when Populate fails") return nil, nil @@ -409,7 +409,7 @@ func TestDeleteAfterErrorEmitsOnlyStorageStop(t *testing.T) { } rec.Reset() - b.emitDeleteClose(ctx, "vm1", metering.Shape{CPU: 2, MemBytes: 2 << 30, StorageBytes: 20 << 30}, metering.ReasonStopCrash, false) + b.emitDeleteClose(ctx, "vm1", metering.Shape{CPU: 2, MemBytes: 2 << 30, StorageBytes: 20 << 30}, metering.ReasonStopCrash, hasOpenComputeInterval(recordOf(t, b, "vm1"))) entries := rec.Entries() if len(entries) != 1 || entries[0].Kind != metering.KindVMStorageStop { t.Fatalf("post-Error delete: got %+v, want one storage.stop", entries) diff --git a/hypervisor/stop_test.go b/hypervisor/stop_test.go index 0ed3e258..b1199585 100644 --- a/hypervisor/stop_test.go +++ b/hypervisor/stop_test.go @@ -3,6 +3,7 @@ package hypervisor import ( "context" "errors" + "io/fs" "net" "os" "strings" @@ -44,7 +45,7 @@ func TestDeleteAllStoppedVM(t *testing.T) { t.Error("record should be gone after delete") } for _, dir := range []string{runDir, logDir} { - if _, statErr := os.Stat(dir); !os.IsNotExist(statErr) { + if _, statErr := os.Stat(dir); !errors.Is(statErr, fs.ErrNotExist) { t.Errorf("dir %s should be removed, stat err: %v", dir, statErr) } } diff --git a/hypervisor/supervisor_test.go b/hypervisor/supervisor_test.go index 4e0402bf..0a336454 100644 --- a/hypervisor/supervisor_test.go +++ b/hypervisor/supervisor_test.go @@ -2,7 +2,7 @@ package hypervisor import ( "context" - "fmt" + "errors" "testing" "time" @@ -123,7 +123,7 @@ func TestConvergeDeadKeepsPendingWhenQuiesceFails(t *testing.T) { ctx := t.Context() seedPlumbedVM(t, b, "vm1") b.SetNetwork(stubNetwork{quiesce: func(context.Context, *types.VM) error { - return fmt.Errorf("netns gone") + return errors.New("netns gone") }}) if err := b.ConvergeDead(ctx, "vm1", recordOf(t, b, "vm1").TransitionGeneration, timeNow()); err == nil { diff --git a/images/cloudimg/pull_test.go b/images/cloudimg/pull_test.go index 1bb56db7..fd41ccc1 100644 --- a/images/cloudimg/pull_test.go +++ b/images/cloudimg/pull_test.go @@ -281,9 +281,7 @@ func parseTestRange(h string, size int64) (start, end int64, ok bool) { if err != nil { return 0, 0, false } - if end >= size { - end = size - 1 - } + end = min(end, size-1) return start, end, true } diff --git a/meta/json/store_test.go b/meta/json/store_test.go index bf3cc055..b6a47425 100644 --- a/meta/json/store_test.go +++ b/meta/json/store_test.go @@ -40,8 +40,8 @@ func TestCommitAtomicityUnderCrash(t *testing.T) { t.Fatal(err) } - crash := errors.New("crash") - testCrashStep = (&crashPoint{step: step, err: crash}).hook + errCrash := errors.New("crash") + testCrashStep = (&crashPoint{step: step, err: errCrash}).hook defer func() { testCrashStep = nil }() err := s.Update(ctx, meta.Scope{Write: "alpha"}, meta.CommitDurable, func(w meta.Writer) error { next := map[string]int{"v": 2} @@ -51,7 +51,7 @@ func TestCommitAtomicityUnderCrash(t *testing.T) { fresh := map[string]int{"v": 2} return c.Insert(ctx, w, "b", &fresh) }) - if !errors.Is(err, crash) { + if !errors.Is(err, errCrash) { t.Fatalf("crash injection: %v", err) } testCrashStep = nil @@ -141,32 +141,6 @@ func TestCorruptBothGenerations(t *testing.T) { } } -func TestExternalProcessEvents(t *testing.T) { - ctx := t.Context() - dir := t.TempDir() - s := newStore(t, dir, "alpha") - - ch, release, err := s.Events(ctx) - if err != nil { - t.Fatal(err) - } - defer release() - - s2 := newStore(t, dir, "alpha") - c2 := meta.NewCollection[map[string]int]("alpha", "records") - v := map[string]int{"v": 1} - if err := s2.Update(ctx, meta.Scope{Write: "alpha"}, meta.CommitDurable, func(w meta.Writer) error { - return c2.Insert(ctx, w, "a", &v) - }); err != nil { - t.Fatal(err) - } - select { - case <-ch: - case <-time.After(10 * time.Second): - t.Fatal("no event after external commit") - } -} - func TestGenericCodecRoundTrip(t *testing.T) { m := NewModel() m.Put("records", "b", []byte(`{"n":2}`)) diff --git a/metering/metering_test.go b/metering/metering_test.go index c962ea89..1c52d4b7 100644 --- a/metering/metering_test.go +++ b/metering/metering_test.go @@ -48,11 +48,6 @@ func TestKindWireFormat(t *testing.T) { } } -func TestNopRecorder(t *testing.T) { - var r NopRecorder - r.Emit(t.Context(), Entry{Kind: KindVMComputeStart, VMID: "x"}) -} - func TestEntryWriteToProducesJSONLine(t *testing.T) { e := Entry{Kind: KindVMComputeStart, VMID: "vm1", Shape: Shape{CPU: 2}} var buf bytes.Buffer diff --git a/network/cni/cni_test.go b/network/cni/cni_test.go index 6e28e450..8fd3ee56 100644 --- a/network/cni/cni_test.go +++ b/network/cni/cni_test.go @@ -3,6 +3,7 @@ package cni import ( "cmp" "context" + "errors" "fmt" "io/fs" "net" @@ -250,8 +251,8 @@ func TestRemoveSweepsDuplicateIfNameRecords(t *testing.T) { ctx := t.Context() seedRecords(t, c, "vm1", "eth1") - if err := c.update(ctx, func(t *netTx) error { - return t.Put("n-eth1-dup", &networkRecord{ID: "n-eth1-dup", Type: "cni-bridge", VMID: "vm1", IfName: "eth1"}) + if err := c.update(ctx, func(tx *netTx) error { + return tx.Put("n-eth1-dup", &networkRecord{ID: "n-eth1-dup", Type: "cni-bridge", VMID: "vm1", IfName: "eth1"}) }); err != nil { t.Fatal(err) } @@ -341,7 +342,7 @@ func TestReclaimStaleNIC(t *testing.T) { assertRecordIDs(t, c, []string{"n-eth1"}) exec.failIf = "" - deleteTAPFn = func(string, string) error { return fmt.Errorf("device busy") } + deleteTAPFn = func(string, string) error { return errors.New("device busy") } if err := c.reclaimStaleNIC(ctx, "vm1", "/run/netns/vm1", rec); err == nil { t.Fatal("reclaimStaleNIC: want error on failed TAP delete") } @@ -376,9 +377,9 @@ func TestAddFailsClosedOnStaleReclaim(t *testing.T) { t.Fatalf("got %d configs, want 1", len(configs)) } var got []networkRecord - if err := c.view(ctx, func(t *netTx) error { + if err := c.view(ctx, func(tx *netTx) error { var err error - got, err = t.byVMID("vm1") + got, err = tx.byVMID("vm1") return err }); err != nil { t.Fatal(err) @@ -408,9 +409,9 @@ func TestAddRollsBackTheNICsItAttemptedOnAPartialFailure(t *testing.T) { t.Fatalf("deleted TAPs = %v, want %v", deletedTAPs, want) } var got []networkRecord - if err := c.view(t.Context(), func(t *netTx) error { + if err := c.view(t.Context(), func(tx *netTx) error { var err error - got, err = t.byVMID("vm1") + got, err = tx.byVMID("vm1") return err }); err != nil { t.Fatal(err) @@ -514,9 +515,9 @@ func TestAddRecoveryRecordsAnUnrecordedNIC(t *testing.T) { t.Fatalf("recover Add: %v", err) } var got []networkRecord - if err := c.view(t.Context(), func(t *netTx) error { + if err := c.view(t.Context(), func(tx *netTx) error { var err error - got, err = t.byVMID("vm1") + got, err = tx.byVMID("vm1") return err }); err != nil { t.Fatal(err) @@ -603,10 +604,10 @@ type recordingExec struct { } func (e *recordingExec) ExecPlugin(_ context.Context, _ string, _ []byte, environ []string) ([]byte, error) { - var ifName, args string + var ifn, args string for _, kv := range environ { if v, ok := strings.CutPrefix(kv, "CNI_IFNAME="); ok { - ifName = v + ifn = v } if v, ok := strings.CutPrefix(kv, "CNI_ARGS="); ok { args = v @@ -615,9 +616,9 @@ func (e *recordingExec) ExecPlugin(_ context.Context, _ string, _ []byte, enviro if slices.Contains(environ, "CNI_COMMAND=ADD") { e.addArgs = append(e.addArgs, args) } - e.attempted = append(e.attempted, ifName) - if ifName == e.failIf { - return nil, fmt.Errorf("simulated plugin failure on %s", ifName) + e.attempted = append(e.attempted, ifn) + if ifn == e.failIf { + return nil, fmt.Errorf("simulated plugin failure on %s", ifn) } return []byte(`{"cniVersion":"1.0.0"}`), nil } @@ -680,9 +681,9 @@ func testVMCfg() *types.VMConfig { func seedRecords(t *testing.T, c *CNI, vmID string, ifNames ...string) { t.Helper() if err := c.update(t.Context(), func(tx *netTx) error { - for _, ifName := range ifNames { - id := "n-" + ifName - if err := tx.Put(id, &networkRecord{ID: id, Type: "cni-bridge", VMID: vmID, IfName: ifName}); err != nil { + for _, ifn := range ifNames { + id := "n-" + ifn + if err := tx.Put(id, &networkRecord{ID: id, Type: "cni-bridge", VMID: vmID, IfName: ifn}); err != nil { return err } } diff --git a/network/cni/lifecycle_linux_test.go b/network/cni/lifecycle_linux_test.go index 676c98c4..8d921c06 100644 --- a/network/cni/lifecycle_linux_test.go +++ b/network/cni/lifecycle_linux_test.go @@ -79,6 +79,18 @@ func TestTCRedirectConvergesOnPartialPlumbing(t *testing.T) { if err := tapProvisionedInNetns(nsPath, "tap0"); err == nil { t.Fatal("tap without its ingress redirect must not verify") } + + if _, _, err := setupTCRedirect(nsPath, "eth0", "tap0", 1, ""); err != nil { + t.Fatalf("setupTCRedirect on partial plumbing: %v", err) + } + if err := tapProvisionedInNetns(nsPath, "tap0"); err != nil { + t.Fatalf("tap must verify after the redirect is re-established: %v", err) + } + for _, dev := range []string{"eth0", "tap0"} { + if n := ingressFilterCount(t, nsPath, dev); n != 1 { + t.Fatalf("%s carries %d ingress filters after convergence, want 1", dev, n) + } + } } func TestEnsureNetnsReplacesAStaleEntry(t *testing.T) { diff --git a/snapshot/localfile/gc_test.go b/snapshot/localfile/gc_test.go index dad905f4..8f9ea740 100644 --- a/snapshot/localfile/gc_test.go +++ b/snapshot/localfile/gc_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "slices" + "strings" "testing" "time" @@ -28,6 +29,9 @@ func TestPickLRU_NoCriteriaEvictsAll(t *testing.T) { if len(got) != 3 { t.Fatalf("want 3 evictions, got %v", got) } + if got["a"] != "lru-all" { + t.Errorf("reason: got %q, want lru-all", got["a"]) + } } func TestPickLRU_KeepLast(t *testing.T) { @@ -106,14 +110,6 @@ func TestPickLRU_ZeroTimeIsOldest(t *testing.T) { } } -func TestPickLRU_NoCriteriaAllReasonAll(t *testing.T) { - records := map[string]snapshotMeta{"a": agedMeta(1, 10)} - got := pickLRU(records, EvictionPolicy{Enabled: true}) - if got["a"] != "lru-all" { - t.Errorf("reason: got %q, want lru-all", got["a"]) - } -} - func TestGCModule_LRUEndToEnd(t *testing.T) { lf := newTestLF(t) ctx := t.Context() @@ -366,19 +362,25 @@ func TestGCModule_RemovalFailureKeepsDBRecord(t *testing.T) { t.Fatalf("ReadDB: %v", err) } - parent := lf.conf.DataDir() - if err := os.Chmod(parent, 0o500); err != nil { - t.Skipf("chmod failed: %v", err) + for _, id := range ids { + dir := lf.conf.SnapshotDataDir(id) + if err := os.Chmod(dir, 0o500); err != nil { + t.Skipf("chmod failed: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(dir, 0o750) }) } - t.Cleanup(func() { _ = os.Chmod(parent, 0o750) }) - if err := mod.Collect(ctx, ids, snap); err == nil { - t.Fatal("expected Collect to error on chmod-protected parent") + err = mod.Collect(ctx, ids, snap) + if err == nil || !strings.Contains(err.Error(), "tombstone kept") { + t.Fatalf("Collect error = %v, want a data dir removal failure keeping the tombstone", err) } for i, name := range []string{"a", "b"} { if _, err := lf.lookupRecord(ctx, ids[i], false); err != nil { t.Errorf("%s: DB record should survive removal failure, got: %v", name, err) } + if !snapTombstonePresent(t, lf, ids[i]) { + t.Errorf("%s: tombstone should be kept after removal failure", name) + } } } @@ -524,7 +526,7 @@ func TestGCSkipsPendingHeldByLiveBuild(t *testing.T) { lf := newTestLF(t) ctx := t.Context() id := testID(t) - release, err := lf.acquireBuildLease(t.Context(), id) + release, err := lf.acquireBuildLease(ctx, id) if err != nil { t.Fatalf("acquireBuildLease: %v", err) } diff --git a/snapshot/localfile/localfile_test.go b/snapshot/localfile/localfile_test.go index d44d5565..76363f6a 100644 --- a/snapshot/localfile/localfile_test.go +++ b/snapshot/localfile/localfile_test.go @@ -1212,48 +1212,6 @@ func TestExportCompressed_ImportRoundtrip(t *testing.T) { } } -func TestExport_ImportRoundtrip(t *testing.T) { - lf := newTestLF(t) - ctx := t.Context() - - origFiles := map[string][]byte{ - "cow.raw": []byte("disk data"), - "state.json": []byte(`{"ok":true}`), - } - makeExportableSnapshot(t, lf, "raw-export-src", origFiles) - - stream, err := lf.Export(ctx, "raw-export-src") - if err != nil { - t.Fatalf("Export: %v", err) - } - defer stream.Close() - - importedID, err := lf.Import(ctx, stream, "raw-imported", "") - if err != nil { - t.Fatalf("Import: %v", err) - } - - s, err := lf.Inspect(ctx, importedID) - if err != nil { - t.Fatalf("Inspect: %v", err) - } - if s.Name != "raw-imported" { - t.Errorf("Name: got %q, want %q", s.Name, "raw-imported") - } - - dataDir := lf.conf.SnapshotDataDir(importedID) - for name, wantContent := range origFiles { - got, readErr := os.ReadFile(filepath.Join(dataDir, name)) - if readErr != nil { - t.Errorf("read %s: %v", name, readErr) - continue - } - if !bytes.Equal(got, wantContent) { - t.Errorf("file %s: got %q, want %q", name, got, wantContent) - } - } -} - func TestImport_InvalidStream(t *testing.T) { lf := newTestLF(t) ctx := t.Context() diff --git a/snapshot/localfile/teardown_test.go b/snapshot/localfile/teardown_test.go index 80254642..bd57baca 100644 --- a/snapshot/localfile/teardown_test.go +++ b/snapshot/localfile/teardown_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "slices" "testing" "github.com/cocoonstack/cocoon/meta/tombstone" @@ -81,6 +82,9 @@ func TestDeletingPendingRecoverySkipsSnapStorageStop(t *testing.T) { if err := lf.recoverSnapTombstone(t.Context(), id); err != nil { t.Fatalf("recoverSnapTombstone: %v", err) } + if _, held, err := lf.NameOwner(t.Context(), "pending"); err != nil || held { + t.Errorf("NameOwner = (%v, %v), want the pending record gone", err, held) + } if entries := rec.Entries(); len(entries) != 0 { t.Errorf("entries = %+v, want no stop for pending snapshot", entries) } @@ -94,9 +98,13 @@ func TestDeletePendingSkipsSnapStorageStop(t *testing.T) { t.Fatalf("beginCreate: %v", err) } - if _, err := lf.Delete(t.Context(), []string{id}); err != nil { + deleted, err := lf.Delete(t.Context(), []string{id}) + if err != nil { t.Fatalf("Delete: %v", err) } + if !slices.Equal(deleted, []string{id}) { + t.Errorf("deleted = %v, want [%s]", deleted, id) + } if entries := rec.Entries(); len(entries) != 0 { t.Errorf("entries = %+v, want no stop for pending snapshot", entries) } diff --git a/utils/extents_linux_test.go b/utils/extents_linux_test.go index 7c46c4da..10b412d7 100644 --- a/utils/extents_linux_test.go +++ b/utils/extents_linux_test.go @@ -32,6 +32,9 @@ func TestFileExtentsReportsDataAndSkipsHoles(t *testing.T) { if first.Logical > 1<<21 || last.Logical+last.Length < 1<<21+4096 { t.Errorf("extents %+v do not cover the write", extents) } + if first.Logical < 1<<21 { + t.Errorf("extents %+v cover the leading hole", extents) + } } func TestPunchHoleKeepsSize(t *testing.T) { diff --git a/utils/stream_test.go b/utils/stream_test.go index 61ca89f8..90511986 100644 --- a/utils/stream_test.go +++ b/utils/stream_test.go @@ -21,7 +21,7 @@ func TestTarDirStreamFailsTheReaderOnAnUnreadableEntry(t *testing.T) { t.Fatal(err) } stream := TarDirStream(src, nil) - defer stream.Close() //nolint:errcheck + defer stream.Close() if err := ExtractTar(t.TempDir(), stream); err == nil { t.Fatal("ExtractTar accepted an archive that is missing b-memory-range") } diff --git a/utils/tar_sparse_linux_test.go b/utils/tar_sparse_linux_test.go index e0f5cde9..99bdba2a 100644 --- a/utils/tar_sparse_linux_test.go +++ b/utils/tar_sparse_linux_test.go @@ -112,7 +112,7 @@ func TestTarFileMaybeSparse_PreservesSparsePathForSmallMaps(t *testing.T) { t.Fatal(err) } if _, ok := hdr.PAXRecords[paxSparseMap]; !ok { - t.Errorf("expected sparse PAX record for small fragmentation, got none") + t.Error("expected sparse PAX record for small fragmentation, got none") } } diff --git a/utils/tar_test.go b/utils/tar_test.go index 2ef1024a..ace7acda 100644 --- a/utils/tar_test.go +++ b/utils/tar_test.go @@ -25,7 +25,7 @@ func TestTarFile(t *testing.T) { var buf bytes.Buffer tw := tar.NewWriter(&buf) if err := openAndTarFile(tw, src, "custom-name.txt"); err != nil { - t.Fatalf("tarFile: %v", err) + t.Fatalf("openAndTarFile: %v", err) } if err := tw.Close(); err != nil { t.Fatal(err) @@ -64,7 +64,7 @@ func TestTarFile_EmptyFile(t *testing.T) { var buf bytes.Buffer tw := tar.NewWriter(&buf) if err := openAndTarFile(tw, src, "empty.txt"); err != nil { - t.Fatalf("tarFile: %v", err) + t.Fatalf("openAndTarFile: %v", err) } tw.Close() @@ -334,18 +334,14 @@ func TestExtractTar_Sparse_SingleSegment(t *testing.T) { t.Fatalf("logical size: got %d, want %d", len(got), realSize) } - for i := range 8192 { - if got[i] != 0 { - t.Fatalf("byte %d: expected 0, got %d", i, got[i]) - } + if !isAllZero(got[:8192]) { + t.Fatal("leading hole not zero") } if !bytes.Equal(got[8192:8192+4096], dataContent) { t.Error("data segment content mismatch") } - for i := 8192 + 4096; i < realSize; i++ { - if got[i] != 0 { - t.Fatalf("byte %d: expected 0, got %d", i, got[i]) - } + if !isAllZero(got[8192+4096:]) { + t.Fatal("trailing hole not zero") } } From 522bac52abf214a57f70777a0f95cede6f158389 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:35:16 +0800 Subject: [PATCH 10/25] review: comment budget, naming and modern idioms from a whole-repo read Compress multi-fact comments to one fact, delete comments that restate the code or an interface's contract, fix comments the code outgrew (convergeOrphanedPause after live migration, Network's omitzero, the missingDir cause, ReverseLayers' parameter), name func types spelled more than once, rename parameters that shadow imported packages, and use slices.Concat, min and a direct return where they are shorter. --- cgroup/cgroup.go | 7 +++---- cgroup/topology.go | 6 +++--- cmd/cliutil/cliutil.go | 2 +- cmd/core/init.go | 4 ++-- cmd/core/metastore.go | 10 +++++----- cmd/core/utils.go | 24 ++++++++++++------------ cmd/core/vmconfig.go | 2 +- cmd/daemon/handler.go | 8 ++++---- cmd/images/import.go | 2 +- cmd/images/pull.go | 12 ++++++------ cmd/images/utils.go | 4 +++- cmd/meta/convert/convert.go | 18 ++++++++++-------- cmd/meta/handler.go | 1 - cmd/snapshot/handler.go | 1 - cmd/storebench/main.go | 3 +-- cmd/vm/logstream.go | 2 +- cmd/vm/run.go | 2 +- cmd/vm/status.go | 4 ++-- config/config.go | 1 - daemon/api.go | 2 +- daemon/reconcile.go | 4 ++-- extend/disk/disk.go | 8 ++++---- gc/refs.go | 2 -- hypervisor/baseconfig.go | 2 +- hypervisor/clone.go | 2 +- hypervisor/cloudhypervisor/clone.go | 2 +- hypervisor/cloudhypervisor/extend.go | 10 +++++----- hypervisor/cloudhypervisor/restore.go | 2 +- hypervisor/cloudhypervisor/start.go | 1 - hypervisor/db.go | 2 +- hypervisor/disks.go | 3 +-- hypervisor/firecracker/clone.go | 2 +- hypervisor/firecracker/create.go | 1 - hypervisor/firecracker/relay.go | 3 +-- hypervisor/gc.go | 6 +++--- hypervisor/netresize.go | 6 +++--- hypervisor/network.go | 3 +-- hypervisor/placement.go | 8 ++++---- hypervisor/restore.go | 13 ++++++------- hypervisor/snapshot.go | 2 +- hypervisor/start.go | 2 +- hypervisor/state.go | 10 +++++----- hypervisor/supervisor.go | 18 +++++++++--------- hypervisor/teardown.go | 8 ++++---- hypervisor/utils.go | 12 ++++-------- images/baseconfig.go | 2 +- images/cloudimg/cloudimg.go | 1 - images/cloudimg/inspect.go | 4 ++-- images/cloudimg/pull.go | 6 +++--- images/gc.go | 2 +- images/index.go | 6 +++--- images/oci/commit.go | 4 ++-- images/oci/pull.go | 2 +- images/store.go | 10 ++++++---- main.go | 2 +- meta/json/codec.go | 1 - meta/json/events.go | 2 +- meta/json/model.go | 4 +++- meta/json/store.go | 7 ++++--- meta/meta.go | 4 ++-- meta/sqlite/events.go | 4 ++-- meta/sqlite/store.go | 1 - meta/tombstone/tombstone.go | 2 +- metadata/fat12.go | 4 +--- metadata/metadata.go | 4 ++-- metering/metalog/metalog.go | 2 +- metering/metering.go | 2 +- network/bridge/bridge_linux.go | 6 +++--- network/cni/cni.go | 5 ++--- network/cni/db.go | 8 +++----- network/cni/lifecycle.go | 3 +-- network/cni/lifecycle_linux.go | 22 +++++++++++----------- network/cni/metans.go | 6 ++++-- network/cni/teardown.go | 3 +-- network/tap_linux.go | 10 +++++----- snapshot/envelope.go | 2 +- snapshot/localfile/export.go | 5 ++--- snapshot/localfile/gc.go | 9 ++++----- snapshot/localfile/import.go | 6 +++--- snapshot/localfile/localfile.go | 10 +++++----- snapshot/localfile/teardown.go | 2 +- types/config.go | 2 +- types/network.go | 2 +- types/storage.go | 2 +- types/vm.go | 6 +++--- utils/atomic.go | 6 +++--- utils/file.go | 7 ++----- utils/http.go | 2 +- utils/process.go | 4 ++-- utils/qcow2.go | 2 +- utils/qemuimg.go | 2 +- utils/reflink_linux.go | 14 +++++++------- utils/reflink_other.go | 6 +++--- utils/sparse_linux.go | 4 ++-- utils/sparse_other.go | 6 +++--- utils/stream.go | 2 +- utils/tar.go | 10 +++++----- 97 files changed, 238 insertions(+), 259 deletions(-) diff --git a/cgroup/cgroup.go b/cgroup/cgroup.go index 894087b0..daa7fca6 100644 --- a/cgroup/cgroup.go +++ b/cgroup/cgroup.go @@ -23,8 +23,7 @@ const ( // Root is the cgroup v2 unified hierarchy mount point. Root = "/sys/fs/cgroup" // DefaultParent holds every per-VM scope unless cgroup_parent overrides it. - DefaultParent = "cocoon.slice" - // defaultPeriodUs is the kernel's default cpu.max period. + DefaultParent = "cocoon.slice" defaultPeriodUs = 100000 // AutoCPUSet asks launch to place the VM on the least-loaded last-level-cache domain. AutoCPUSet = "auto" @@ -153,7 +152,7 @@ func ScopeDir(parentDir, vmID string) string { return filepath.Join(parentDir, scopePrefix+vmID+scopeSuffix) } -// Prepare creates or reconfigures vmID's scope and returns its opened directory for CLONE_INTO_CGROUP; idempotent, so a relaunch reuses a scope its dying predecessor still occupies. deferQuota leaves the ceiling at max for the provisioning window — weight, fence, and placement still apply — until Arm sets the finite quota. +// Prepare creates or reconfigures vmID's scope and returns its directory for CLONE_INTO_CGROUP; idempotent. func Prepare(parentDir, fence, vmID string, k Knobs, deferQuota bool) (*os.File, error) { if err := ensureParent(parentDir, fence, k.CPUSet != ""); err != nil { return nil, err @@ -189,7 +188,7 @@ func Prepare(parentDir, fence, vmID string, k Knobs, deferQuota bool) (*os.File, return scope, nil } -// Arm sets the finite quota on a scope prepared with deferQuota; called after memory load, before resume, so the guest never runs uncapped. Idempotent — a retry after a partial failure converges. +// Arm sets the finite quota on a scope prepared with deferQuota, which keeps the ceiling at max until now; call it after memory load, before resume. func Arm(parentDir, vmID string, k Knobs) error { return armQuota(ScopeDir(parentDir, vmID), k) } diff --git a/cgroup/topology.go b/cgroup/topology.go index c4346a60..e9a8b8bd 100644 --- a/cgroup/topology.go +++ b/cgroup/topology.go @@ -57,7 +57,7 @@ func ReadTopology(root string, fence []int) (*Topology, error) { return t, nil } -// Place picks the least-loaded domain and, for two or more queues, one hardware thread per queue inside it: distinct cores before SMT siblings, fewer pins than queues once the domain runs out. +// Place picks the least-loaded domain and, for 2+ queues, one thread per queue, preferring distinct cores. func (t *Topology) Place(queues int, load map[int]int) (domain, pins []int) { pick := 0 best := sumLoad(t.domains[0], load) @@ -76,8 +76,8 @@ func (t *Topology) Place(queues int, load map[int]int) (domain, pins []int) { for _, c := range t.cores[pick] { core := slices.Clone(c) slices.SortFunc(core, byLoad) - for i, c := range core { - layer[c] = i + for i, cpu := range core { + layer[cpu] = i } ranked = append(ranked, core...) } diff --git a/cmd/cliutil/cliutil.go b/cmd/cliutil/cliutil.go index 11dd663d..b84f7db6 100644 --- a/cmd/cliutil/cliutil.go +++ b/cmd/cliutil/cliutil.go @@ -92,7 +92,7 @@ func FormatSize(bytes int64) string { return units.BytesSize(float64(bytes)) } -// The discarded error on each Flag* accessor means "no such flag", which the command tree makes impossible. +// A flag absent on this command reads as its zero value. func FlagBool(cmd *cobra.Command, name string) bool { v, _ := cmd.Flags().GetBool(name); return v } func FlagInt(cmd *cobra.Command, name string) int { v, _ := cmd.Flags().GetInt(name); return v } diff --git a/cmd/core/init.go b/cmd/core/init.go index 90901645..b49f4795 100644 --- a/cmd/core/init.go +++ b/cmd/core/init.go @@ -130,7 +130,7 @@ func InitSnapshot(ctx context.Context, conf *config.Config, opts ...localfile.Op return s, nil } -// PinEnvelopeBlobs locks envelope-sourced pins (clone/restore --from-dir, snapshot import) on the owning backend; digests with no local blob are skipped. +// PinEnvelopeBlobs locks envelope-sourced blob pins on the owning backend, skipping digests with no local blob. func PinEnvelopeBlobs(ctx context.Context, conf *config.Config, blobIDs map[string]struct{}) (func(), error) { if len(blobIDs) == 0 { return func() {}, nil @@ -175,7 +175,7 @@ func wireHypervisor[H networkedHypervisor](newFn func(*config.Config, metering.R } } -// pinnedElsewhere unions VM and snapshot blob pins for image GC's under-lock recheck; the backends build once, on the first candidate blob. +// pinnedElsewhere unions VM and snapshot blob pins for image GC's under-lock recheck. func pinnedElsewhere(conf *config.Config) imagebackend.PinRecheck { var ( once sync.Once diff --git a/cmd/core/metastore.go b/cmd/core/metastore.go index 9e6403db..339a8c7c 100644 --- a/cmd/core/metastore.go +++ b/cmd/core/metastore.go @@ -38,7 +38,7 @@ var ( metaStore meta.Store metaErr error - // vmTables maps the legacy vms.json fields onto the vms-namespace tables; the json field names are engine knowledge and live only here. + // vmTables maps legacy vms.json fields onto the vms-namespace tables. vmTables = metajson.TableCodec{Specs: []metajson.TableSpec{ {Key: "vms", Table: hypervisor.TableRecords}, {Key: "names", Table: hypervisor.TableNames}, @@ -69,7 +69,7 @@ func MetaNamespaces() []metasqlite.Namespace { } } -// MetaJSONNamespaces declares the json engine's namespace set: legacy file locations and field mappings, consumed by open and by conversion. +// MetaJSONNamespaces declares the json engine's namespaces. func MetaJSONNamespaces(conf *config.Config) []metajson.Namespace { chCfg := cloudhypervisor.NewConfig(conf) fcCfg := firecracker.NewConfig(conf) @@ -96,7 +96,7 @@ func MetaDBPath(conf *config.Config) string { return filepath.Join(conf.RootDir, "meta", metasqlite.DBFileName) } -// ResolveMetaBackend returns the effective engine: an explicit setting wins, then an existing store binds (meta.db → sqlite, legacy json files → json), and a fresh root gets sqlite. +// ResolveMetaBackend returns the effective engine: explicit setting, then the existing store, else sqlite. func ResolveMetaBackend(conf *config.Config) string { if conf.MetaBackend != "" { return conf.MetaBackend @@ -117,7 +117,7 @@ func LegacyJSONPresent(conf *config.Config) bool { }) } -// MetaStore builds the process-wide meta store once, one store for every namespace and backend; the engine follows ResolveMetaBackend, and a fresh sqlite root bootstraps itself. +// MetaStore builds the process-wide meta store once. func MetaStore(conf *config.Config) (meta.Store, error) { metaOnce.Do(func() { // Bootstrap owns its context: the store outlives any single caller, and a canceled first caller must not poison the Once for everyone. @@ -180,7 +180,7 @@ func ImageJSONNamespace(c *images.BaseConfig) metajson.Namespace { } } -// openSQLiteStore opens the sqlite engine, bootstrapping a fresh root or repairing a crashed bootstrap; a legacy json root never bootstraps — that would shadow its data. +// openSQLiteStore never bootstraps over legacy json data, which a fresh store would shadow. func openSQLiteStore(ctx context.Context, conf *config.Config, dbPath string) (meta.Store, error) { if !LegacyJSONPresent(conf) { if err := metasqlite.InitIfMissing(ctx, dbPath, MetaNamespaces()...); err != nil { diff --git a/cmd/core/utils.go b/cmd/core/utils.go index 00976adc..9650de7e 100644 --- a/cmd/core/utils.go +++ b/cmd/core/utils.go @@ -152,31 +152,31 @@ func CloseOnCancel(ctx context.Context, c io.Closer) func() bool { } // SnapshotNameFlags reads the --name/--description pair and rejects a taken name; an empty name passes. -func SnapshotNameFlags(ctx context.Context, cmd *cobra.Command, snapBackend snapshot.Snapshot) (name, description string, err error) { - name, _ = cmd.Flags().GetString("name") +func SnapshotNameFlags(ctx context.Context, cmd *cobra.Command, snapBackend snapshot.Snapshot) (snapName, description string, err error) { + snapName, _ = cmd.Flags().GetString("name") description, _ = cmd.Flags().GetString("description") - if err = (&types.SnapshotConfig{Name: name}).Validate(); err != nil { + if err = (&types.SnapshotConfig{Name: snapName}).Validate(); err != nil { return "", "", err } - if name == "" { - return name, description, nil + if snapName == "" { + return snapName, description, nil } - // The name index, not Inspect: a killed save leaves a pending record still holding the name that Inspect reports as not-found, and passing that preflight wastes the whole capture before the insert rejects it. + // Use the name index: Inspect misses a pending record from a killed save that still holds the name. nh, ok := snapBackend.(snapshot.NameHolder) if !ok { return "", "", fmt.Errorf("backend %s has no snapshot name index", snapBackend.Type()) } - id, held, err := nh.NameOwner(ctx, name) + id, held, err := nh.NameOwner(ctx, snapName) if err != nil { return "", "", fmt.Errorf("check snapshot name: %w", err) } if held { - return "", "", fmt.Errorf("snapshot name %q already exists (held by %s)", name, id) + return "", "", fmt.Errorf("snapshot name %q already exists (held by %s)", snapName, id) } - return name, description, nil + return snapName, description, nil } -// PersistSnapshotDir stores a finalized capture dir, moving it in place when srcDir shares a filesystem with the backend and streaming a tar otherwise; srcDir is consumed on every path. +// PersistSnapshotDir stores a finalized capture dir; srcDir is consumed on every path. func PersistSnapshotDir(ctx context.Context, snapBackend snapshot.Snapshot, cfg *types.SnapshotConfig, srcDir, name, description string) (string, error) { logger := log.WithFunc("core.PersistSnapshotDir") cfg.Name = name @@ -234,7 +234,7 @@ func resolveOwner[T typed](backends []T, ref string, found func(T) (bool, error) } } -// resolveVMOwner returns the owning hypervisor and resolved *types.VM so callers use vm.ID instead of re-resolving the raw ref. +// resolveVMOwner returns the owning hypervisor and the resolved VM. func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref string) (hypervisor.Hypervisor, *types.VM, error) { var resolved *types.VM owner, err := resolveOwner( @@ -255,7 +255,7 @@ func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref str return owner, resolved, err } -// validateRefShape rejects URL/OCI ref mismatches early so backends don't surface misleading downstream errors. +// validateRefShape rejects URL/OCI ref mismatches before any backend runs. func validateRefShape(ref, imageType string) error { switch imageType { case types.ImageTypeCloudImg: diff --git a/cmd/core/vmconfig.go b/cmd/core/vmconfig.go index c6ce5f0f..ac42a35c 100644 --- a/cmd/core/vmconfig.go +++ b/cmd/core/vmconfig.go @@ -67,7 +67,7 @@ func VMConfigFromFlags(cmd *cobra.Command, image string) (*types.VMConfig, error return cfg, nil } -// CloneVMConfigFromFlags builds VMConfig for a clone. The snapshot's cgroup knobs record its source VM's policy and are never applied; the clone's policy comes from flags alone. +// CloneVMConfigFromFlags builds a clone's VMConfig; cgroup knobs come from flags alone, never the snapshot. func CloneVMConfigFromFlags(cmd *cobra.Command, snapCfg types.SnapshotConfig) (*types.VMConfig, error) { restoreMode, err := restoreModeFromFlags(cmd) if err != nil { diff --git a/cmd/daemon/handler.go b/cmd/daemon/handler.go index d6589e2a..c7e8eeb2 100644 --- a/cmd/daemon/handler.go +++ b/cmd/daemon/handler.go @@ -22,6 +22,10 @@ type Handler struct { func (h Handler) Run(cmd *cobra.Command, _ []string) error { ctx, conf := h.Init(cmd) + dconf, err := daemonConfig(cmd, conf) + if err != nil { + return err + } store, err := cmdcore.MetaStore(conf) if err != nil { return err @@ -34,10 +38,6 @@ func (h Handler) Run(cmd *cobra.Command, _ []string) error { for _, hyper := range hypers { supervisors = append(supervisors, hyper) } - dconf, err := daemonConfig(cmd, conf) - if err != nil { - return err - } return cocoond.New(dconf, store, supervisors).Run(ctx) } diff --git a/cmd/images/import.go b/cmd/images/import.go index 47102ec6..0dda2c23 100644 --- a/cmd/images/import.go +++ b/cmd/images/import.go @@ -113,7 +113,7 @@ func importCloudimg(ctx context.Context, conf *config.Config, name, downloadMsg return nil } -func importOCI(ctx context.Context, conf *config.Config, name string, pullMsg func(int) string, do func(*oci.OCI, progress.Tracker) error) error { +func importOCI(ctx context.Context, conf *config.Config, name string, pullMsg pullMsgFunc, do func(*oci.OCI, progress.Tracker) error) error { store, _, err := cmdcore.InitImageBackendsForPull(ctx, conf) if err != nil { return err diff --git a/cmd/images/pull.go b/cmd/images/pull.go index 8dff5e5f..7aaaee82 100644 --- a/cmd/images/pull.go +++ b/cmd/images/pull.go @@ -25,14 +25,14 @@ func (h Handler) Pull(cmd *cobra.Command, args []string) error { force, _ := cmd.Flags().GetBool("force") for _, image := range args { + var pullErr error if cliutil.IsURL(image) { - if err := h.pullCloudimg(ctx, cloudimgStore, image, force); err != nil { - return err - } + pullErr = h.pullCloudimg(ctx, cloudimgStore, image, force) } else { - if err := h.pullOCI(ctx, ociStore, image); err != nil { - return err - } + pullErr = h.pullOCI(ctx, ociStore, image) + } + if pullErr != nil { + return pullErr } } return nil diff --git a/cmd/images/utils.go b/cmd/images/utils.go index 7acfeef0..2a923be4 100644 --- a/cmd/images/utils.go +++ b/cmd/images/utils.go @@ -10,7 +10,9 @@ import ( ociProgress "github.com/cocoonstack/cocoon/progress/oci" ) -func ociTracker(ctx context.Context, logger *log.Fields, name string, pullMsg func(total int) string) progress.Tracker { +type pullMsgFunc func(total int) string + +func ociTracker(ctx context.Context, logger *log.Fields, name string, pullMsg pullMsgFunc) progress.Tracker { return progress.NewTracker(func(e ociProgress.Event) { switch e.Phase { case ociProgress.PhasePull: diff --git a/cmd/meta/convert/convert.go b/cmd/meta/convert/convert.go index 8c4e96b4..6179685a 100644 --- a/cmd/meta/convert/convert.go +++ b/cmd/meta/convert/convert.go @@ -1,4 +1,4 @@ -// Package convert is the explicit offline engine cutover (design §6): a standalone fsync-first manifest is the only recovery authority, targets are fresh, sources retire aside, and every step is crash-rerunnable. +// Package convert runs the offline meta engine cutover (design §6). package convert import ( @@ -140,7 +140,7 @@ func newManifest(ctx context.Context, spec Spec, target string, src meta.Store) return m, nil } -// checkQuiesced is §6's advisory activity check: an in-flight source write means live cocoon processes, and converting under them could lose writes landing on a namespace already marked done. +// checkQuiesced refuses conversion while a live process holds the source (§6). func checkQuiesced(ctx context.Context, spec Spec, target string, src meta.Store) error { if target == config.MetaBackendJSON { // sqlite source: an empty durable transaction fails ErrBusy while a writer is mid-flight. @@ -169,7 +169,7 @@ func checkQuiesced(ctx context.Context, spec Spec, target string, src meta.Store return nil } -// ensureJSONDirs creates namespace dirs for subsystems that never ran — their absence means empty, and the engine's flock needs the dir to exist. +// ensureJSONDirs creates namespace dirs for subsystems that never ran; flock needs them. func ensureJSONDirs(spec Spec) error { dirs := make([]string, 0, len(spec.JSON)) for _, jns := range spec.JSON { @@ -234,7 +234,7 @@ func convertNamespace(ctx context.Context, src, dst meta.Store, ns metasqlite.Na return crashStep("ns-done") } -// duplicateGeneration copies a freshly written json target to its .prev so the imported data survives a later torn main (§9: never less resilient than steady state). +// duplicateGeneration writes the json target's .prev so imported data survives a torn main (§9). func duplicateGeneration(spec Spec, nsName string) error { jns, ok := findJSON(spec, nsName) if !ok { @@ -311,7 +311,7 @@ func copyNamespace(ctx context.Context, src, dst meta.Store, ns metasqlite.Names }) } -// verifyNames is §6's referential name check: every name entry must point at an existing record. Table names are the record-SPI convention shared by every namespace declaration. +// verifyNames checks every name entry points at an existing record (§6). func verifyNames(ctx context.Context, s meta.Store, ns metasqlite.Namespace) error { if !slices.Contains(ns.Tables, "names") { return nil @@ -322,9 +322,11 @@ func verifyNames(ctx context.Context, s meta.Store, ns metasqlite.Namespace) err if err := json.Unmarshal(raw, &id); err != nil { return fmt.Errorf("name %q: %w", name, err) } - if _, ok, err := r.GetRaw(ctx, ns.Name, "records", id); err != nil { + _, ok, err := r.GetRaw(ctx, ns.Name, "records", id) + if err != nil { return err - } else if !ok { + } + if !ok { return fmt.Errorf("name %q points at missing record %q", name, id) } return nil @@ -360,7 +362,7 @@ func canonicalDigest(ctx context.Context, s meta.Store, ns metasqlite.Namespace) return hex.EncodeToString(h.Sum(nil)), count, nil } -// retireSources aside-renames every source file after full verification; a sqlite source checkpoints to a single file first (§6). Runs with both engines closed; already-renamed files are skipped, so a crash here reruns. +// retireSources renames verified source files aside; renamed files are skipped on rerun. func retireSources(ctx context.Context, spec Spec, target string) error { if target == config.MetaBackendJSON && utils.FileExists(spec.DBPath) { if err := metasqlite.Checkpoint(ctx, spec.DBPath); err != nil { diff --git a/cmd/meta/handler.go b/cmd/meta/handler.go index 0d32adae..76a74ee2 100644 --- a/cmd/meta/handler.go +++ b/cmd/meta/handler.go @@ -13,7 +13,6 @@ import ( metasqlite "github.com/cocoonstack/cocoon/meta/sqlite" ) -// Handler groups the meta store lifecycle commands (init, convert, backup). type Handler struct { cmdcore.BaseHandler } diff --git a/cmd/snapshot/handler.go b/cmd/snapshot/handler.go index 1a6434eb..f668e709 100644 --- a/cmd/snapshot/handler.go +++ b/cmd/snapshot/handler.go @@ -19,7 +19,6 @@ import ( "github.com/cocoonstack/cocoon/types" ) -// Handler groups the snapshot subcommands. type Handler struct { cmdcore.BaseHandler } diff --git a/cmd/storebench/main.go b/cmd/storebench/main.go index d6302847..6d9b079d 100644 --- a/cmd/storebench/main.go +++ b/cmd/storebench/main.go @@ -151,7 +151,6 @@ func runCreate(ctx context.Context, engine string, workers, per, resident int, d return nil } -// runWorker performs `per` creates (reserve placeholder + finalize to running) — the meta half of one VM creation each. func runWorker(ctx context.Context, engine, prefix string, per int, dir string) error { b, err := openBackend(ctx, engine, dir) if err != nil { @@ -287,7 +286,7 @@ func argDir(i int) string { return dir } -// uniquePayload defeats any identical-bytes write elision so replace ops measure a REAL durable commit. +// uniquePayload defeats identical-bytes write elision. func uniquePayload(i int) jsontext.Value { return jsontext.Value(fmt.Sprintf(`{"name":"bench","state":"running","seq":%d}`, i)) } diff --git a/cmd/vm/logstream.go b/cmd/vm/logstream.go index c3e46a70..3c4e7acf 100644 --- a/cmd/vm/logstream.go +++ b/cmd/vm/logstream.go @@ -111,7 +111,7 @@ func seekToLastNLines(f *os.File, n int) error { return err } -// watchFile watches a file via fsnotify on the parent dir (atomic rename changes inode). +// watchFile watches the parent dir because an atomic rename changes the inode. func watchFile(ctx context.Context, filePath string, debounce time.Duration) (<-chan struct{}, error) { watcher, err := fsnotify.NewWatcher() if err != nil { diff --git a/cmd/vm/run.go b/cmd/vm/run.go index 263009d3..ff31b25b 100644 --- a/cmd/vm/run.go +++ b/cmd/vm/run.go @@ -402,7 +402,7 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t info, createErr := hyper.Create(ctx, vmID, vmCfg, storageConfigs, netSetup, bootCfg) if createErr != nil { rollbackNetwork(ctx, netProvider, vmID) - // Idempotent belt over Create's own rollback: pre-adoption failures (e.g. CPU validation) leave the placeholder squatting the name until GC. + // Pre-adoption failures leave the placeholder squatting the name until GC. rollbackReserve() return nil, nil, nil, fmt.Errorf("create VM: %w", createErr) } diff --git a/cmd/vm/status.go b/cmd/vm/status.go index 20731bb9..9b881594 100644 --- a/cmd/vm/status.go +++ b/cmd/vm/status.go @@ -105,7 +105,7 @@ func statusOnce(ctx context.Context, hypers []hypervisor.Hypervisor, filters []s return renderVMList(vms, format, scopeDir, reconcileVMStates(vms)) } -// renderVMList prints vms as JSON or a table; stale marks records whose VMM is gone, a flag in JSON and a state suffix in the table. +// Stale marks records whose VMM is gone: a flag in JSON, a state suffix in the table. func renderVMList(vms []*types.VM, format, scopeDir string, stale map[string]bool) error { if format == cliutil.FormatJSON { items := make([]vmOutput, len(vms)) @@ -136,7 +136,7 @@ func metaEvents(ctx context.Context, conf *config.Config) <-chan struct{} { return ch } -// runLoop warns on backend errors so polling ticks don't break; one-shot callers use cmdcore.ListAllVMs directly. +// Warn on list errors so a failed tick does not stop the loop. func runLoop(ctx context.Context, hypers []hypervisor.Hypervisor, filters []string, watchCh <-chan struct{}, tick <-chan time.Time, fn func([]*types.VM)) { poll := func() { vms, err := cmdcore.ListAllVMs(ctx, hypers) diff --git a/config/config.go b/config/config.go index 3cadd467..a49d6b78 100644 --- a/config/config.go +++ b/config/config.go @@ -25,7 +25,6 @@ const ( // RestoreTAPPrefix names CH's throwaway restore TAPs; a scope equal to it would collide with a clone's own bridge TAPs. RestoreTAPPrefix = "rm" - // defaultPullConns is the default concurrent Range connections per cloud-image download. defaultPullConns = 8 // maxPullConns caps it so a misconfigured pull_conns can't exhaust file descriptors. maxPullConns = 64 diff --git a/daemon/api.go b/daemon/api.go index 28675dcb..d153c015 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -34,7 +34,7 @@ type vmsResponse struct { VMs []VMStatus `json:"vms"` } -// healthResponse: degraded VMs do not flip readiness — a daemon restart cannot fix a VM nobody can converge. +// Degraded does not affect OK. type healthResponse struct { OK bool `json:"ok"` Degraded int `json:"degraded"` diff --git a/daemon/reconcile.go b/daemon/reconcile.go index 2cfedbbd..dcebef80 100644 --- a/daemon/reconcile.go +++ b/daemon/reconcile.go @@ -153,7 +153,7 @@ func (d *Daemon) collectOwnerless(ctx context.Context, b Supervisor, rec *hyperv return nil } -// resumeDelete finishes a delete a previous worker left mid-protocol; supervision starts deletes of its own, and an unfinished one strands the record and its name until a manual gc. +// resumeDelete finishes a delete a previous worker left mid-protocol. func (d *Daemon) resumeDelete(ctx context.Context, b Supervisor, vmID string) error { unlock, ok, err := d.tryLock(ctx, b, vmID) if !ok { @@ -226,7 +226,7 @@ func (d *Daemon) confirmDead(ctx context.Context, b Supervisor, rec *hypervisor. return false, err } -// tryLock reports ok=false for a busy lock and for a lock error alike; only the error is worth logging, and only it makes the pass unhealthy. +// tryLock reports ok=false for a busy lock and for an error alike; only the error is logged and counted. func (d *Daemon) tryLock(ctx context.Context, b Supervisor, vmID string) (func(), bool, error) { unlock, ok, err := b.TryLockVMOps(ctx, vmID) if err != nil { diff --git a/extend/disk/disk.go b/extend/disk/disk.go index 7451bcba..8c812e4f 100644 --- a/extend/disk/disk.go +++ b/extend/disk/disk.go @@ -1,4 +1,4 @@ -// Package disk is the runtime attach interface for extra virtio-blk disks backed by existing raw files or a snapshot's COW copy; attach is runtime-only (snapshot/hibernate refuse while one is attached) and detach deletes only cocoon's own copy. +// Package disk is the runtime attach interface for extra virtio-blk disks. package disk import ( @@ -20,7 +20,7 @@ const ( // ErrUnsupportedBackend signals the backend cannot hot-plug virtio-blk disks (Firecracker on the MMIO transport). var ErrUnsupportedBackend = errors.New("backend does not support disk attach") -// Spec is one attach request for an existing raw disk file; DirectIO nil means the create-path default (O_DIRECT unless writable disks have it disabled), tmpfs-backed files need an explicit off. +// Spec is one attach request for an existing raw disk file. type Spec struct { Path string Name string @@ -94,12 +94,12 @@ func SnapshotName(snapID string) string { return snapshotNamePrefix + strings.ToLower(snapID[:min(len(snapID), snapshotNameIDLen)]) } -// DeriveID returns the deterministic Cloud Hypervisor device id for a disk name (attach and detach share it so concurrent attaches collide on CH's id check); Firecracker derives its own underscore form. +// DeriveID returns the deterministic CH device id for a disk name. func DeriveID(name string) string { return diskIDPrefix + name } -// NameFromID reverses DeriveID; empty when id is not a hot-added disk (foreign same-prefix ids with an illegal name suffix are not ours). +// NameFromID reverses DeriveID; empty when id is not a hot-added disk. func NameFromID(id string) string { name, ok := strings.CutPrefix(id, diskIDPrefix) if ok && types.ValidDataDiskName(name) { diff --git a/gc/refs.go b/gc/refs.go index 7e0c0fc5..7bccf207 100644 --- a/gc/refs.go +++ b/gc/refs.go @@ -2,12 +2,10 @@ package gc import "maps" -// usedBlobIDs is implemented by snapshots that reference image blobs. type usedBlobIDs interface { UsedBlobIDs() map[string]struct{} } -// activeVMIDs is implemented by snapshots that track live VMs. type activeVMIDs interface { ActiveVMIDs() map[string]struct{} } diff --git a/hypervisor/baseconfig.go b/hypervisor/baseconfig.go index 822733fb..e77e7572 100644 --- a/hypervisor/baseconfig.go +++ b/hypervisor/baseconfig.go @@ -99,7 +99,7 @@ func (c *BaseConfig) PreflightRestore(srcDir string, rec *VMRecord, integrity In func (c *BaseConfig) RootDirPath() string { return c.RootDir } -// ResolveExternalVolume canonicalizes path (EvalSymlinks also asserts existence) and refuses cocoon-managed roots — vm rm / GC delete those trees, and a symlink must not smuggle one past the check. +// ResolveExternalVolume returns path with symlinks resolved and refuses cocoon-managed roots, which vm rm and GC delete. func (c *BaseConfig) ResolveExternalVolume(path string) (string, error) { resolved, err := filepath.EvalSymlinks(path) if err != nil { diff --git a/hypervisor/clone.go b/hypervisor/clone.go index a16614fd..859dccc5 100644 --- a/hypervisor/clone.go +++ b/hypervisor/clone.go @@ -13,7 +13,7 @@ import ( // CloneFilesFunc copies a snapshot's files from srcDir into dstDir. type CloneFilesFunc func(dstDir, srcDir string) error -// AfterExtractFn finalizes a cloned VM after snapshot files are in place; rec is the placed placeholder, sourceSnapshotID flows through for metering lineage. +// AfterExtractFn finalizes a cloned VM once snapshot files are in place. type AfterExtractFn func(ctx context.Context, rec *VMRecord, vmCfg *types.VMConfig, net types.NetSetup, sourceSnapshotID string) (*types.VM, error) // CloneSpec carries one clone's inputs through the shared placeholder→populate→finalize skeleton. diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index 81992a61..7ed6bbcd 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -99,7 +99,7 @@ func (ch *CloudHypervisor) cloneAfterExtractParsed(ctx context.Context, rec *hyp return nil, fmt.Errorf("validate post-cidata storage: %w", vErr) } - // vm.restore attaches the snapshot's TAP names and vm.remove-device releases a TAP only after the guest's eject ACK, so every restored NIC gets a clone-unique throwaway TAP that CH creates and drops itself. + // Each restored NIC gets a clone-unique throwaway TAP: vm.restore reuses snapshot TAP names and CH frees a TAP only after the eject ACK. netTAPs := make([]string, len(chCfg.Nets)) for i := range netTAPs { netTAPs[i] = network.TAPName(config.RestoreTAPPrefix, vmID, i) diff --git a/hypervisor/cloudhypervisor/extend.go b/hypervisor/cloudhypervisor/extend.go index 64969381..6ab28d51 100644 --- a/hypervisor/cloudhypervisor/extend.go +++ b/hypervisor/cloudhypervisor/extend.go @@ -88,7 +88,7 @@ func (ch *CloudHypervisor) DiskAttachSnapshot(ctx context.Context, vmRef string, os.Remove(path) //nolint:errcheck,gosec return "", "", err } - // A lost add-disk response may have attached the copy, so only a retry, detach or stop reclaims it + // A lost add-disk response may have attached the copy, so only a retry, detach or stop reclaims it. deviceID, err := addDevice(ctx, hc, "vm.add-disk", hotDisk(&rec, id, &types.StorageConfig{ Role: types.StorageRoleData, Path: path, Serial: src.Name, })) @@ -222,7 +222,7 @@ func (ch *CloudHypervisor) DeviceList(ctx context.Context, vmRef string) ([]vfio }) } -// lockedDeviceOp serializes device-set mutations per VM and returns the record plus a vm.info snapshot taken under the lock, with an ownerless pause already resumed. +// lockedDeviceOp locks device-set mutations per VM and returns the record plus vm.info, resuming an ownerless pause. func (ch *CloudHypervisor) lockedDeviceOp(ctx context.Context, vmRef string) (*http.Client, hypervisor.VMRecord, *chVMInfoResponse, func(), error) { hc, rec, unlock, err := ch.LockedRunningOp(ctx, vmRef) if err != nil { @@ -270,7 +270,7 @@ func (ch *CloudHypervisor) detachWith(ctx context.Context, vmRef string, findID return fmt.Errorf("device %s removal initiated but the guest has not ejected it: %w", deviceID, err) } } - // A detach whose remove-device response was lost finds no device, so the copy is reclaimed here too + // A detach whose remove-device response was lost finds no device, so the copy is reclaimed here too. if afterEject == nil { return nil } @@ -305,7 +305,7 @@ func addDevice(ctx context.Context, hc *http.Client, endpoint string, body any) return pci.ID, nil } -// The CH fork refuses disks without an explicit image_type; build from the record reloaded under the ops lock so DirectIO/queue semantics match create-path data disks (restore rewrites Config). +// The CH fork refuses disks without an explicit image_type; build from the record reloaded under the ops lock. func hotDisk(rec *hypervisor.VMRecord, id string, sc *types.StorageConfig) chDisk { d := storageConfigToDisk(sc, rec.Config.CPU, rec.Config.DiskQueueSize, rec.Config.NoDirectIO, hypervisor.QueueCPUs(rec)) d.ID = id @@ -331,7 +331,7 @@ func diskAttached(info *chVMInfoResponse, id, name, path string, readOnly bool) return "", nil } -// The pause is ownerless: callers hold the ops lock every capture window holds. +// convergeOrphanedPause resumes a pause no capture holds, including a final-round or received migration. func convergeOrphanedPause(ctx context.Context, hc *http.Client, vmID string) (*chVMInfoResponse, error) { info, err := getVMInfo(ctx, hc) if err != nil { diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index 56dcbf41..93df0ed0 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -17,7 +17,7 @@ import ( ) func (ch *CloudHypervisor) Restore(ctx context.Context, vmRef string, vmCfg *types.VMConfig, stream io.Reader, sourceSnapshotID string) (*types.VM, error) { - // Preflight's validated meta feeds AfterExtract: cocoon.json is merged verbatim, so re-reading it would decode identical bytes. + // Preflight's meta feeds AfterExtract; cocoon.json is merged verbatim. var meta *snapshot.SnapshotMeta return ch.RestoreSequence(ctx, vmRef, hypervisor.RestoreSpec{ VMCfg: vmCfg, diff --git a/hypervisor/cloudhypervisor/start.go b/hypervisor/cloudhypervisor/start.go index b519cf78..2a984cae 100644 --- a/hypervisor/cloudhypervisor/start.go +++ b/hypervisor/cloudhypervisor/start.go @@ -56,7 +56,6 @@ func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VM defer logFile.Close() //nolint:errcheck } - // Shell out: the cloud-hypervisor binary is the authoritative VMM. cmd := exec.Command(ch.conf.CHBinary, args...) //nolint:gosec // Setpgid so CH survives if this process exits. cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} diff --git a/hypervisor/db.go b/hypervisor/db.go index ba17a8ca..f23d91b9 100644 --- a/hypervisor/db.go +++ b/hypervisor/db.go @@ -15,7 +15,7 @@ type VMRecord struct { RunDir string `json:"run_dir,omitempty"` LogDir string `json:"log_dir,omitempty"` - // Quarantine names why start must refuse (e.g. partial restore merge); only a successful restore clears it — stop's state flip cannot. + // Quarantine names why start must refuse; only a successful restore clears it. Quarantine string `json:"quarantine,omitempty"` // QuiescePending survives a crash between the stop transition and the network Quiesce, so a later pass retries it. diff --git a/hypervisor/disks.go b/hypervisor/disks.go index 7314c1a2..517e9cd8 100644 --- a/hypervisor/disks.go +++ b/hypervisor/disks.go @@ -42,7 +42,6 @@ func (b *Backend) PrepareOCIDisks(ctx context.Context, vmID string, vmCfg *types } func InitCOWFilesystem(ctx context.Context, path string) error { - // Shell out because no Go ext4 formatter library; mkfs.ext4 is authoritative. out, err := exec.CommandContext( //nolint:gosec ctx, "mkfs.ext4", "-F", "-m", "0", "-q", @@ -55,7 +54,7 @@ func InitCOWFilesystem(ctx context.Context, path string) error { return nil } -// DataDiskBaseName is the canonical file name (centralized so matchers, reflink loops, and clone path rewrites stay in sync). +// DataDiskBaseName returns the canonical data disk file name. func DataDiskBaseName(serial string) string { return "data-" + serial + ".raw" } diff --git a/hypervisor/firecracker/clone.go b/hypervisor/firecracker/clone.go index 2eb0c9e2..626a4139 100644 --- a/hypervisor/firecracker/clone.go +++ b/hypervisor/firecracker/clone.go @@ -85,7 +85,7 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, rec *hypervisor.VM if prepErr != nil { return nil, prepErr } - if err := types.ValidateStorageConfigs(append(slices.Clone(storageConfigs), dataDisks...)); err != nil { + if err := types.ValidateStorageConfigs(slices.Concat(storageConfigs, dataDisks)); err != nil { return nil, fmt.Errorf("validate storage configs: %w", err) } bootCfg := meta.BootConfig diff --git a/hypervisor/firecracker/create.go b/hypervisor/firecracker/create.go index cb678477..4c91486e 100644 --- a/hypervisor/firecracker/create.go +++ b/hypervisor/firecracker/create.go @@ -52,7 +52,6 @@ func EnsureVmlinuxBoot(boot *types.BootConfig) error { return nil } -// ensureVmlinux decompresses kernelPath if needed and returns the ELF path. func ensureVmlinux(kernelPath string) (string, error) { f, err := os.Open(kernelPath) //nolint:gosec if err != nil { diff --git a/hypervisor/firecracker/relay.go b/hypervisor/firecracker/relay.go index 9700ce1c..764b21c9 100644 --- a/hypervisor/firecracker/relay.go +++ b/hypervisor/firecracker/relay.go @@ -39,7 +39,6 @@ const ( relayLeaseCommitAck = byte(3) ) -// broadcaster swaps the PTY reader's sink atomically. type broadcaster struct { master io.Reader mu sync.Mutex @@ -224,7 +223,7 @@ func relaySession(ctx context.Context, master io.Writer, conn net.Conn, bc *broa select { case <-done: - case <-ctx.Done(): // FC died + case <-ctx.Done(): // firecracker exited } // Closing conn unblocks the io.Copy(master, conn) goroutine. } diff --git a/hypervisor/gc.go b/hypervisor/gc.go index 6462b3cf..1bbfed82 100644 --- a/hypervisor/gc.go +++ b/hypervisor/gc.go @@ -123,7 +123,7 @@ func (b *Backend) gcCollect(ctx context.Context, ids []string, snap VMGCSnapshot errs := b.sweepStaleCaptureDirs(ctx, snap.sweepDirs(b.Conf.RunDir())) errs = append(errs, b.sweepStaleCloneLocks(ctx)...) for _, id := range ids { - // Ops lock excludes in-flight owners: a create pre-locks and mkdirs before its DB record lands, so an unlocked "orphan" may be seconds old. + // Hold the ops lock: a create mkdirs before its record lands. ok := b.withOpsTryLock(ctx, id, func() { rec, err := b.PeekRecord(ctx, id) if err != nil { @@ -209,7 +209,7 @@ func (b *Backend) sweepStaleCloneLocks(ctx context.Context) []error { return errs } -// withOpsTryLock runs fn holding the VM ops lock, reporting false (fn skipped) when an in-flight operation owns it; GC never blocks on ops locks, so the reversed order vs. the ops-outer/index-inner convention cannot deadlock. +// withOpsTryLock runs fn under the VM ops lock and reports false when another operation holds it; GC only try-locks, so lock order cannot deadlock. func (b *Backend) withOpsTryLock(ctx context.Context, vmID string, fn func()) bool { unlock, ok, err := b.TryLockVMOps(ctx, vmID) if err != nil || !ok { @@ -220,7 +220,7 @@ func (b *Backend) withOpsTryLock(ctx context.Context, vmID string, fn func()) bo return true } -// ensureOrphanVMMDead terminates any VMM bound to runDir's socket and errors unless none survive; a missing pidfile is not proof of death, so it also scans /proc by socket path. +// ensureOrphanVMMDead kills any VMM on runDir's socket; the /proc scan covers a missing pidfile. func (b *Backend) ensureOrphanVMMDead(ctx context.Context, runDir string) error { sockPath := SocketPath(runDir) if pid, err := utils.ReadPIDFile(b.PIDFilePath(runDir)); err == nil { diff --git a/hypervisor/netresize.go b/hypervisor/netresize.go index 42b30b80..6e2e0018 100644 --- a/hypervisor/netresize.go +++ b/hypervisor/netresize.go @@ -71,7 +71,7 @@ func (b *Backend) netResizeAdd(ctx context.Context, vmID string, rec *VMRecord, nc := ncs[0] devID, err := dev.AddNIC(ctx, i, nc) if err != nil { - // Survives Ctrl-C: an abandoned half-add is exactly the divergence the reconcile above exists for. + // Detach from ctx so Ctrl-C cannot abandon a half-add. rctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), nicRollbackTimeout) if rmErr := plumbing.Remove(rctx, vmID, i); rmErr != nil { logger.Warnf(rctx, "rollback host plumbing for nic %d: %v", i, rmErr) @@ -130,7 +130,7 @@ func (b *Backend) netResizeRemove(ctx context.Context, vmID string, rec *VMRecor devID = live[j].ID } if devID == "" { - // Already gone after an interrupted prior resize: resume with the host/DB halves so retries converge instead of wedging on "no live device". + // Device already gone from an interrupted resize: finish the host and DB halves so retries converge. logger.Warnf(ctx, "nic %d (%s): no live device, resuming interrupted remove", i, nc.TAP) } else if err := dev.RemoveNIC(ctx, devID); err != nil { return res, fmt.Errorf("remove nic %d (%s): %w", i, devID, err) @@ -171,7 +171,7 @@ func (b *Backend) truncateNetworkConfigs(ctx context.Context, vmID string, lengt }) } -// reconcileOrphanNICs removes live NICs in slots the record does not know — leftovers of a resize interrupted between the device add and the DB write — and best-effort reclaims their host slot (bridge TAPs have no DB record, and a leftover TAP wedges every retry at CreateTAP). +// reconcileOrphanNICs removes live NICs in slots the record does not know and reclaims their host slot. func reconcileOrphanNICs(ctx context.Context, dev NICDeviceOps, vmID string, live []LiveNIC, known int, plumbing netresize.Plumbing) error { logger := log.WithFunc("hypervisor.reconcileOrphanNICs") for _, n := range live { diff --git a/hypervisor/network.go b/hypervisor/network.go index 527c52a0..43179f59 100644 --- a/hypervisor/network.go +++ b/hypervisor/network.go @@ -8,14 +8,13 @@ import ( "github.com/cocoonstack/cocoon/types" ) -// VMNetwork converges a VM's host networking; the command layer supplies it so a state transition and its network action share one VM ops lock (design §5). +// VMNetwork converges a VM's host networking under the caller's VM ops lock. type VMNetwork interface { Recover(ctx context.Context, vm *types.VM) error Quiesce(ctx context.Context, vm *types.VM) error Cleanup(ctx context.Context, vmID string) error } -// SetNetwork injects the host-networking seam. func (b *Backend) SetNetwork(n VMNetwork) { b.Net = n } // RecoverNetwork rebuilds and un-quiesces a VM's plumbing before launch; the caller holds the VM ops lock. diff --git a/hypervisor/placement.go b/hypervisor/placement.go index 8ae35a20..e8bcd038 100644 --- a/hypervisor/placement.go +++ b/hypervisor/placement.go @@ -13,7 +13,7 @@ import ( // sysCPURoot is the sysfs cpu tree; tests point it at a fixture. var sysCPURoot = cgroup.SysCPURoot -// placeRecord resolves id's cpu placement for a launch under cfg and persists it in one write transaction, so concurrent launches see each other's placements. +// placeRecord resolves and persists id's cpu placement in one write transaction so concurrent launches see each other. func (b *Backend) placeRecord(ctx context.Context, id string, cfg *types.Config) (VMRecord, error) { topo, topoErr := b.placementTopology(cfg) if topoErr != nil { @@ -40,7 +40,7 @@ func (b *Backend) placeRecord(ctx context.Context, id string, cfg *types.Config) return placed, err } -// placementTopology reads the host's cache domains inside the fence when launch picks cfg's cpus; nil when cfg names them or pins nothing. +// placementTopology reads the host cache domains; nil when cfg names cpus or pins nothing. func (b *Backend) placementTopology(cfg *types.Config) (*cgroup.Topology, error) { if cfg.CPUSetCPUs != cgroup.AutoCPUSet && (cfg.CPUSetCPUs != "" || !b.pinsQueues(cfg)) { return nil, nil @@ -58,7 +58,7 @@ func (b *Backend) placementTopology(cfg *types.Config) (*cgroup.Topology, error) return b.topo, b.topoErr } -// placeVM fills rec's placement from cfg: an explicit cpuset is copied; with a topology, "auto" and no cpuset pick the least-loaded cache domain against the placements live VMs already hold. +// placeVM fills rec's placement from cfg; with a topology it picks the least-loaded cache domain. func (b *Backend) placeVM(ctx context.Context, r meta.Reader, rec *VMRecord, cfg *types.Config, topo *cgroup.Topology) error { rec.CPUSet, rec.QueueCPUs = "", "" pins := b.pinsQueues(cfg) @@ -89,7 +89,7 @@ func (b *Backend) pinsQueues(cfg *types.Config) bool { return b.PinsQueues && cfg.CPU > 1 } -// placementLoad counts per host cpu the VMs across every backend holding it, from the placement rows; self is the VM being placed. +// placementLoad counts live VMs per host cpu across all backends, excluding self. func (b *Backend) placementLoad(ctx context.Context, r meta.Reader, self string) (map[int]int, error) { load := map[int]int{} count := func(id string, cpuList *string) error { diff --git a/hypervisor/restore.go b/hypervisor/restore.go index 4418daae..dbfef397 100644 --- a/hypervisor/restore.go +++ b/hypervisor/restore.go @@ -87,7 +87,7 @@ func (b *Backend) RestoreSequence(ctx context.Context, vmRef string, spec Restor return err } if mergeErr := MergeDirInto(stagingDir, rec.RunDir); mergeErr != nil { - // A partial merge leaves mixed-vintage files in the run dir; quarantine regardless of origin so vm start cannot boot them. + // A partial merge mixes vintages; quarantine any origin so vm start cannot boot it. b.QuarantineVM(ctx, vmID, "partial snapshot merge") return fmt.Errorf("apply staged snapshot: %w", mergeErr) } @@ -109,7 +109,7 @@ func (b *Backend) DirectRestoreSequence(ctx context.Context, vmRef string, spec } apply := func(rec *VMRecord) error { if populateErr := spec.Populate(rec, spec.SrcDir); populateErr != nil { - // Populate cleans then clones with no rollback; a partial run dir must quarantine regardless of origin, like the merge. + // Populate has no rollback; quarantine any origin on a partial run dir. b.QuarantineVM(ctx, vmID, "partial restore populate") return populateErr } @@ -126,7 +126,6 @@ func (b *Backend) failRestore(ctx context.Context, vmID string, origin types.VMS b.MarkError(ctx, vmID) } -// restoreCore is the shared kill→emit→apply→finalize tail of both restore sequences. func (b *Backend) restoreCore(ctx context.Context, run restoreRun) (*types.VM, error) { oldShape := shapeFromConfig(run.rec.Config) if killErr := run.kill(ctx, run.vmID, run.rec); killErr != nil { @@ -182,7 +181,7 @@ func (b *Backend) prepareRestore(ctx context.Context, vmRef string, vmCfg *types unlock() return "", nil, nil, err } - // Revalidate under the lock (from the guard's own transaction): the pre-lock record may predate a concurrent mutating verb, and preflight anchors the external trust set to it. + // Revalidate under the lock; the pre-lock record may predate a concurrent mutating verb. rec, err := b.EntryGuardLoad(ctx, vmID) if err != nil { return fail(err) @@ -199,7 +198,7 @@ func (b *Backend) prepareRestore(ctx context.Context, vmRef string, vmCfg *types return vmID, &rec, unlock, nil } -// emitRestoreComputeStop closes the compute interval after a confirmed kill; fail-closed on DB error and skip on vanished record so the ledger never gets a phantom entry. +// emitRestoreComputeStop closes the compute interval after a confirmed kill; a vanished record gets no phantom entry. func (b *Backend) emitRestoreComputeStop(ctx context.Context, vmID string, oldShape metering.Shape, sourceSnapshotID string) { now := timeNow() closed := false @@ -226,7 +225,7 @@ func (b *Backend) emitRestoreComputeStop(ctx context.Context, vmID string, oldSh b.Metering.Emit(ctx, b.makeSourceEntry(metering.KindVMComputeStop, vmID, sourceSnapshotID, metering.ReasonRestore, oldShape, now)) } -// PrepareStagingDir extracts the snapshot into a staging dir inside the run dir: a top-level sibling would look like an orphan to the GC run-dir scan and be reaped mid-restore. +// PrepareStagingDir extracts inside the run dir; a top-level sibling would look like an orphan to the GC run-dir scan. func PrepareStagingDir(runDir string, snapshot io.Reader) (stagingDir string, cleanup func(), err error) { stagingDir = filepath.Join(runDir, restoreStagingName) if err = os.RemoveAll(stagingDir); err != nil { @@ -243,7 +242,7 @@ func PrepareStagingDir(runDir string, snapshot io.Reader) (stagingDir string, cl return stagingDir, cleanup, nil } -// Stopped is a hibernate resume; Error is allowed because restore rebuilds the run dir and is the recovery path out of a crashed restore, which would otherwise dead-end at vm rm. +// Error is restorable: restore is the recovery path out of a crashed restore. func restorableState(vmID string, rec *VMRecord) error { if rec.State != types.VMStateRunning && rec.State != types.VMStateStopped && rec.State != types.VMStateError { return fmt.Errorf("vm %s is %s and cannot be restored", vmID, rec.State) diff --git a/hypervisor/snapshot.go b/hypervisor/snapshot.go index 753ea31b..d49a1604 100644 --- a/hypervisor/snapshot.go +++ b/hypervisor/snapshot.go @@ -245,7 +245,7 @@ func ValidateMetaPaths(meta *snapshot.SnapshotMeta, rootDir, runDir string) erro return nil } -// ReverseLayers projects Role==Layer entries through fn in reverse order (topmost layer first, matching overlayfs lowerdir semantics). +// ReverseLayers projects layer entries through project, topmost layer first (overlayfs lowerdir order). func ReverseLayers[T any](storageConfigs []*types.StorageConfig, project func(idx int, sc *types.StorageConfig) T) []T { var out []T for _, sc := range storageConfigs { diff --git a/hypervisor/start.go b/hypervisor/start.go index 29dfeae4..2f85d7bb 100644 --- a/hypervisor/start.go +++ b/hypervisor/start.go @@ -121,7 +121,7 @@ func (b *Backend) PrepareStart(ctx context.Context, id string, scan *utils.ProcS return &placed, nil } -// LaunchVMProcess starts spec.Cmd and waits for the API socket; exited closes once the VMM is reaped, and any post-Start error kills the process + removes the PID file. +// LaunchVMProcess starts spec.Cmd and waits for the API socket; exited closes when the VMM is reaped. func (b *Backend) LaunchVMProcess(ctx context.Context, spec LaunchSpec) (pid int, exited <-chan struct{}, err error) { reaped := make(chan struct{}) started := false diff --git a/hypervisor/state.go b/hypervisor/state.go index c64d3013..5a635274 100644 --- a/hypervisor/state.go +++ b/hypervisor/state.go @@ -45,7 +45,7 @@ func (b *Backend) IsAPISocketLive(ctx context.Context, rec *VMRecord) (bool, err return true, err } -// UpdateState flips id to Stopped or Error and emits compute.stop on Running→Stopped; an Error path leaves the interval open for a confirmed-dead helper. +// UpdateState flips id to Stopped or Error; Running→Stopped emits compute.stop. func (b *Backend) UpdateState(ctx context.Context, id string, state types.VMState) error { now := timeNow() return b.updateVM(ctx, id, func(r *VMRecord) []metering.Entry { @@ -200,7 +200,7 @@ func (b *Backend) updateVM(ctx context.Context, id string, mutate func(r *VMReco return nil } -// markFailedOperation records retryable network convergence after an operation may have brought plumbing up without committing a usable VMM; the daemon re-observes before acting on it. +// markFailedOperation schedules network convergence after an operation may have left plumbing up without a usable VMM. func (b *Backend) markFailedOperation(ctx context.Context, id string, markError bool) { ctx, cancel := detachedWrite(ctx) defer cancel() @@ -221,19 +221,19 @@ func detachedWrite(ctx context.Context) (context.Context, context.CancelFunc) { return context.WithTimeout(context.WithoutCancel(ctx), persistTimeout) } -// hasOpenComputeInterval reports whether the VM's record shows an unmatched compute.start (StoppedAt is the ledger-close sentinel; transitions to Running clear it). +// hasOpenComputeInterval reports an unmatched compute.start. func hasOpenComputeInterval(r *VMRecord) bool { return r != nil && r.StartedAt != nil && r.StoppedAt == nil } -// markTransition stamps one committed state change; every transition after the creating placeholder goes through it so generations stay dense enough to fence stale observations. +// markTransition stamps one committed state change and bumps the generation. func markTransition(r *VMRecord, state types.VMState, reason types.TransitionReason, at time.Time) { r.State = state r.TransitionGeneration++ r.LastTransitionReason = reason r.LastTransitionAt = &at r.UpdatedAt = at - // A recorded placement means the VMM may hold those cpus; only Stopped proves it is gone, an Error-state VMM may still be pinned + // A recorded placement means the VMM may hold those cpus; only Stopped proves it is gone, an Error-state VMM may still be pinned. if state == types.VMStateStopped { r.CPUSet, r.QueueCPUs = "", "" } diff --git a/hypervisor/supervisor.go b/hypervisor/supervisor.go index db3010c3..6d466394 100644 --- a/hypervisor/supervisor.go +++ b/hypervisor/supervisor.go @@ -17,13 +17,13 @@ import ( ) const ( - StaleCreateCollected StaleCreateOutcome = "collected" // ownerless placeholder reclaimed; record and name freed - StaleCreateBusy StaleCreateOutcome = "busy" // in-flight operation owns the VM; nothing touched - StaleCreateNotCreating StaleCreateOutcome = "not-creating" // record left the creating state under the lock - StaleCreateNotFound StaleCreateOutcome = "not-found" // no record under the id + StaleCreateCollected StaleCreateOutcome = "collected" + StaleCreateBusy StaleCreateOutcome = "busy" + StaleCreateNotCreating StaleCreateOutcome = "not-creating" + StaleCreateNotFound StaleCreateOutcome = "not-found" ) -// StaleCreateOutcome reports what ReconcileStaleCreate did with the record. +// StaleCreateOutcome reports what ReconcileStaleCreate did; busy means an in-flight operation owns the VM. type StaleCreateOutcome string // Supervisable is the backend surface a resident supervisor drives. @@ -47,7 +47,7 @@ type SupervisionScan struct { Procs utils.ProcScan } -// ScanSupervision reads every record plus the unfinished-delete set in one transaction; the json engine holds the namespace flock for the whole closure, so never widen this into per-VM reads. +// ScanSupervision reads records and pending tombstones in one transaction; the json engine holds the namespace flock across it. func (b *Backend) ScanSupervision(ctx context.Context) (SupervisionScan, error) { var scan SupervisionScan if err := b.view(ctx, func(t *vmTx) error { @@ -99,7 +99,7 @@ func (b *Backend) TryLockVMOps(ctx context.Context, vmID string) (unlock func(), return func() { _ = l.Unlock(ctx) }, true, nil } -// ConvergeDead lands the stop transition for a VMM that exited outside a cocoon stop; gen fences a stale observation from re-labeling a newer one. +// ConvergeDead lands the stop transition for a VMM that exited outside cocoon; gen fences a stale observation. func (b *Backend) ConvergeDead(ctx context.Context, id string, gen uint64, observedAt time.Time) error { if err := b.convergeDeadRecord(ctx, id, gen, observedAt); err != nil { return err @@ -133,7 +133,7 @@ func (b *Backend) QuiesceIfPending(ctx context.Context, id string) error { return b.clearQuiescePending(ctx, id, gen) } -// ReconcileStaleCreate reclaims id when it is an ownerless creating placeholder; a free ops lock is the proof of ownerlessness, since create and clone hold it from prereserve through the final record commit. +// ReconcileStaleCreate reclaims an ownerless creating placeholder; a free ops lock proves ownerlessness. func (b *Backend) ReconcileStaleCreate(ctx context.Context, id string) (StaleCreateOutcome, error) { unlock, ok, err := b.TryLockVMOps(ctx, id) if err != nil { @@ -216,7 +216,7 @@ func (b *Backend) clearQuiescePending(ctx context.Context, id string, gen uint64 }) } -// convergeCrashedStart closes a crashed VM's ledger on the start path, skipping the quiesce the imminent launch would undo; the in-memory precheck exists because even an empty transaction takes the namespace flock. +// convergeCrashedStart closes a crashed VM's ledger on the start path without quiescing. func (b *Backend) convergeCrashedStart(ctx context.Context, rec *VMRecord) { if !NeedsDeadConvergence(rec) { return diff --git a/hypervisor/teardown.go b/hypervisor/teardown.go index 8d9d46ae..fa3800b4 100644 --- a/hypervisor/teardown.go +++ b/hypervisor/teardown.go @@ -13,7 +13,7 @@ import ( "github.com/cocoonstack/cocoon/types" ) -// vmCleanup is the vms-namespace tombstone payload: everything teardown needs once the record is gone. +// vmCleanup is the vms-namespace tombstone payload: what teardown needs once the record is gone. type vmCleanup struct { Name string `json:"name,omitempty"` RunDir string `json:"run_dir,omitempty"` @@ -32,7 +32,7 @@ func (b *Backend) EntryGuardLoad(ctx context.Context, id string) (VMRecord, erro return *rec, nil } -// RecoverTombstone drives id's tombstone to completion under the held ops lock; supervision starts deletes of its own, so it must be able to finish them. +// RecoverTombstone drives id's tombstone to completion under the held ops lock. func (b *Backend) RecoverTombstone(ctx context.Context, id string) (done bool, err error) { ts := b.tombstones() var ( @@ -68,7 +68,7 @@ func (b *Backend) tombstones() *tombstone.Table { return tombstone.NewTable(b.NS) } -// deleteVMProtocol runs the §5 phase protocol (lease → deleting → teardown → fenced finalize) under the held ops lock; a crash at any point leaves a phase-directed tombstone a later worker resumes. +// deleteVMProtocol runs lease, deleting, teardown and fenced finalize under the held ops lock; a crash leaves a tombstone a later worker resumes. func (b *Backend) deleteVMProtocol(ctx context.Context, id string, rec *VMRecord) error { ts := b.tombstones() cl := vmCleanup{Name: rec.Config.Name, RunDir: rec.RunDir, LogDir: rec.LogDir} @@ -104,7 +104,7 @@ func (b *Backend) deleteVMProtocol(ctx context.Context, id string, rec *VMRecord return b.finishVMTeardown(ctx, id, leaseID, cl) } -// finishVMTeardown runs the slow cleanup outside any transaction, then the fenced finalize that deletes record, name and tombstone together. +// finishVMTeardown runs the slow cleanup outside any transaction, then the fenced finalize. func (b *Backend) finishVMTeardown(ctx context.Context, id, leaseID string, cl vmCleanup) error { if err := b.cleanupNetwork(ctx, id); err != nil { return fmt.Errorf("vm %s network teardown (tombstone kept, retry or gc resumes): %w", id, err) diff --git a/hypervisor/utils.go b/hypervisor/utils.go index b35e99c7..8422021e 100644 --- a/hypervisor/utils.go +++ b/hypervisor/utils.go @@ -28,16 +28,12 @@ import ( ) const ( - // SnapshotFileMemory is a read-only memory/state file (hard link or symlink). SnapshotFileMemory SnapshotFileKind = iota - // SnapshotFileCOW is a writable disk that must be copied (reflink/sparse). SnapshotFileCOW - // SnapshotFileMeta is small metadata that is plain-copied. SnapshotFileMeta - // SnapshotFileSkip is left out of the clone. SnapshotFileSkip - // CloneLocksDirName holds FC clone locks under the backend run root, outside any VM dir — a clone must be able to lock a source whose dir is already gone. + // CloneLocksDirName holds FC clone locks outside any VM dir so a clone can lock a source whose dir is gone. CloneLocksDirName = "clone-locks" restoreStagingName = ".restore-staging" @@ -53,7 +49,7 @@ const ( var hostCPUCount = sync.OnceValue(func() int { return hostCPUCountAt(onlineCPUsPath) }) -// SnapshotFileKind classifies a snapshot file for CloneSnapshotFiles. +// SnapshotFileKind tells clone how to bring a file over: Memory links, COW copies, Meta plain-copies, Skip omits. type SnapshotFileKind int // LockVMOps serializes mutating verbs on one VM across processes; the flock dies with the holder so a crash never wedges the VM. @@ -99,7 +95,7 @@ func (b *Backend) PIDFilePath(runDir string) string { return filepath.Join(runDir, b.Conf.PIDFileName()) } -// LogFilePath returns the per-VM hypervisor log file under logDir (named after the backend so write/read can't drift). +// LogFilePath returns the per-VM hypervisor log file under logDir. func (b *Backend) LogFilePath(logDir string) string { return filepath.Join(logDir, b.Typ+".log") } @@ -250,7 +246,7 @@ func MergeDirInto(src, dst string) error { return nil } -// HostCPUCount returns the host's online CPU count; runtime.NumCPU reports the process affinity mask, which undersizes guests when the control plane is core-pinned. +// HostCPUCount returns the online CPU count; runtime.NumCPU honors the affinity mask and undersizes guests. func HostCPUCount() int { return hostCPUCount() } // ValidateHostCPU rejects vCPU counts beyond the host's online cores. diff --git a/images/baseconfig.go b/images/baseconfig.go index 7fd93341..5402803c 100644 --- a/images/baseconfig.go +++ b/images/baseconfig.go @@ -21,7 +21,7 @@ func (c *BaseConfig) DBDir() string { return filepath.Join(c.BackendDir(), func (c *BaseConfig) TempDir() string { return filepath.Join(c.BackendDir(), "temp") } func (c *BaseConfig) BlobsDir() string { return filepath.Join(c.BackendDir(), "blobs") } -// BlobLockPath is the per-digest lock beside the blob, taken by GC and by any flow that materializes that digest; never removed by cleanup. +// BlobLockPath is the per-digest lock beside the blob; cleanup never removes it. func (c *BaseConfig) BlobLockPath(hex string) string { return filepath.Join(c.BlobsDir(), hex+".lock") } diff --git a/images/cloudimg/cloudimg.go b/images/cloudimg/cloudimg.go index 84d22215..742f55c5 100644 --- a/images/cloudimg/cloudimg.go +++ b/images/cloudimg/cloudimg.go @@ -72,7 +72,6 @@ func (c *CloudImg) Config(ctx context.Context, vm *types.VMConfig) (configs []*t if !utils.ValidFile(blobPath) { return fmt.Errorf("blob invalid for VM %s (%s)", vm.Name, entry.ContentSum) } - // Stamped last: ResolveImage probes every backend, and a loser must not leave its identity on the VM. vm.ImageDigest = entry.EntryID() vm.ImageType = c.Type() diff --git a/images/cloudimg/inspect.go b/images/cloudimg/inspect.go index f75b6986..cc7ce6d0 100644 --- a/images/cloudimg/inspect.go +++ b/images/cloudimg/inspect.go @@ -32,7 +32,7 @@ var nonImageSignatures = []struct { type sourceImageInfo struct { Format string - Compat string // qcow2 compat level (e.g. "0.10", "1.1"); empty for non-qcow2 + Compat string // empty for non-qcow2 } func sniffHead(head []byte) error { @@ -58,7 +58,7 @@ func inspectImage(ctx context.Context, path string) (*sourceImageInfo, error) { if isQcow2 { return info, nil } - // Not qcow2: shell out to qemu-img to distinguish raw from unsupported (vmdk/vdi/vhd/etc.) — treating non-qcow2 as raw would corrupt the converted blob. + // Treating non-qcow2 as raw would corrupt the blob, so qemu-img distinguishes raw from vmdk/vdi/vhd. log.WithFunc("cloudimg.inspectImage").Debugf(ctx, "exec qemu-img info %s", path) cmd := exec.CommandContext(ctx, "qemu-img", "info", "--output=json", path) //nolint:gosec // path is controlled out, err := cmd.Output() diff --git a/images/cloudimg/pull.go b/images/cloudimg/pull.go index 14a6a519..cfdc39c5 100644 --- a/images/cloudimg/pull.go +++ b/images/cloudimg/pull.go @@ -35,7 +35,7 @@ type Doer interface { Do(*http.Request) (*http.Response, error) } -// DownloadBlob writes url into dst and returns the sha256 hex of what landed: pullConns Range requests when the server honors them, else the single stream the probe already opened; the first bytes are sniffed before any bulk transfer. +// DownloadBlob writes url into dst and returns its sha256 hex, using pullConns Range requests when the server honors them. func DownloadBlob(ctx context.Context, client Doer, url string, dst *os.File, pullConns int, tracker progress.Tracker) (string, error) { logger := log.WithFunc("cloudimg.DownloadBlob") req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) @@ -73,14 +73,14 @@ func DownloadBlob(ctx context.Context, client Doer, url string, dst *os.File, pu } logger.Debugf(ctx, "downloading %s in %d parallel range(s)", url, pullConns) - // the post-redirect URL spares every range request the redirect chain + // The post-redirect URL spares each range request the redirect chain. if err := downloadRangesParallel(ctx, client, resp.Request.URL.String(), dst, size, pullConns, tracker); err != nil { return "", err } return hashDigest(dst) } -// progressCounter emits PhaseDownload events every ~1 MiB; mutex-guarded so it serves both the serial writer and parallel range workers. +// progressCounter is mutex-guarded because the serial writer and range workers share it. type progressCounter struct { mu sync.Mutex written int64 diff --git a/images/gc.go b/images/gc.go index d0f6d8ad..609cfdf7 100644 --- a/images/gc.go +++ b/images/gc.go @@ -100,7 +100,7 @@ func gcStaleTemp(ctx context.Context, dir string) []error { func collectBlob[E any](ctx context.Context, cfg GCModuleConfig[E], logger *log.Fields, hex string) error { fl := gofrsflock.New(cfg.LockPath(hex)) defer fl.Close() //nolint:errcheck - // TryLock: a publish holding it is mid rename-to-index and must not lose its blob; the lock file is kept since unlinking it would split exclusion for a live waiter. + // TryLock: a publish holding it is mid rename-to-index and must not lose its blob. ok, err := fl.TryLock() if err != nil { return fmt.Errorf("lock blob %s: %w", hex, err) diff --git a/images/index.go b/images/index.go index ab1f9de2..8697fe81 100644 --- a/images/index.go +++ b/images/index.go @@ -42,7 +42,7 @@ func ReferencedDigests[E Entry](images map[string]*E) map[string]struct{} { return refs } -// LookupOne resolves id to a single entry via LookupRefs' rules; multiple refs must all name one digest (tag aliases) — a prefix spanning distinct digests resolves to nothing rather than map-iteration luck. +// LookupOne resolves id to one entry; refs spanning distinct digests resolve to nothing. func LookupOne[E Entry](images map[string]*E, id string) (string, *E, bool) { refs := LookupRefs(images, id) if len(refs) == 0 || !refsShareDigest(images, refs) { @@ -56,7 +56,7 @@ func LookupRefs[E Entry](images map[string]*E, id string) []string { if entry, ok := images[id]; ok && entry != nil { return []string{id} } - // Digest match (exact or prefix); minHexLen guards against over-broad prefixes like "sha256:a". + // minHexLen guards against over-broad prefixes like "sha256:a". idHex := strings.TrimPrefix(id, "sha256:") var refs []string for ref, ep := range images { @@ -92,7 +92,7 @@ func refsShareDigest[E Entry](images map[string]*E, refs []string) bool { return true } -// deleteByID removes every ref matching an id, so "delete " sweeps all refs pointing at it. A prefix spanning distinct digests is rejected — guessing on a destructive op would delete unrelated images. +// deleteByID removes every ref matching an id; a prefix spanning distinct digests is rejected. func deleteByID[E Entry](ctx context.Context, logPrefix string, images map[string]*E, ids []string) ([]string, error) { logger := log.WithFunc(logPrefix) var deleted []string diff --git a/images/oci/commit.go b/images/oci/commit.go index 0f6cad45..b5c9a8ad 100644 --- a/images/oci/commit.go +++ b/images/oci/commit.go @@ -27,7 +27,7 @@ func moveBootFile(src, dst, bootDir string, layerIdx int, name string) error { return nil } -// finishImport commits blobs, then records the entry in one pure transaction; verb names the operation for logging. +// finishImport commits blobs, then records the entry in one pure transaction. func finishImport(ctx context.Context, conf *Config, store *images.Store[imageEntry], name string, manifestDigest images.Digest, results []pullLayerResult, tracker progress.Tracker, verb string) error { logger := log.WithFunc("oci.finishImport") @@ -59,7 +59,7 @@ func finishImport(ctx context.Context, conf *Config, store *images.Store[imageEn return nil } -// commitBlobs moves each layer's blob and boot files into their final paths, outside any transaction and under per-digest locks so GC cannot delete a not-yet-indexed blob. +// commitBlobs moves blobs and boot files into place under per-digest locks so GC cannot delete a not-yet-indexed blob. func commitBlobs(conf *Config, results []pullLayerResult) error { for i := range results { r := &results[i] diff --git a/images/oci/pull.go b/images/oci/pull.go index ee2c7297..1434f8d0 100644 --- a/images/oci/pull.go +++ b/images/oci/pull.go @@ -33,7 +33,7 @@ func pull(ctx context.Context, conf *Config, store *images.Store[imageEntry], im return err } - // Heavy work (downloads, EROFS conversion, blob renames) runs outside any transaction under finishImport's per-digest locks; the up-to-date pre-check is racy but benign since the whole flow is idempotent per digest. + // Heavy work runs outside any transaction; the racy up-to-date pre-check is benign since the flow is idempotent per digest. var upToDate bool var knownBootHexes map[string]struct{} if err := store.View(ctx, func(idx *imageIndex) error { diff --git a/images/store.go b/images/store.go index e0a14b0f..3d904026 100644 --- a/images/store.go +++ b/images/store.go @@ -16,6 +16,8 @@ import ( // TableRecords is the image-namespace records table. const TableRecords = "records" +type IndexFunc[E any] func(*Index[E]) error + // Store is one image namespace on the shared meta engine; image lookups are whole-map by nature (ref normalization, digest-prefix matching). type Store[E any] struct { meta meta.Store @@ -28,7 +30,7 @@ func NewMetaStore[E any](store meta.Store, ns string) *Store[E] { } // View runs fn over a materialized read snapshot. -func (s *Store[E]) View(ctx context.Context, fn func(*Index[E]) error) error { +func (s *Store[E]) View(ctx context.Context, fn IndexFunc[E]) error { return s.meta.View(ctx, []string{s.ns}, func(r meta.Reader) error { idx, _, err := s.materialize(ctx, r) if err != nil { @@ -39,13 +41,13 @@ func (s *Store[E]) View(ctx context.Context, fn func(*Index[E]) error) error { } // Update runs a pure fn against the materialized index and writes the difference back as record operations. -func (s *Store[E]) Update(ctx context.Context, fn func(*Index[E]) error) error { +func (s *Store[E]) Update(ctx context.Context, fn IndexFunc[E]) error { return s.meta.Update(ctx, meta.Scope{Write: s.ns}, meta.CommitDurable, func(w meta.Writer) error { return s.applyDiff(ctx, w, fn) }) } -func (s *Store[E]) applyDiff(ctx context.Context, w meta.Writer, fn func(*Index[E]) error) error { +func (s *Store[E]) applyDiff(ctx context.Context, w meta.Writer, fn IndexFunc[E]) error { idx, before, err := s.materialize(ctx, w) if err != nil { return err @@ -95,7 +97,7 @@ func (s *Store[E]) materialize(ctx context.Context, r meta.Reader) (*Index[E], m return idx, before, nil } -// BlobLocks holds per-digest blob locks for a publish critical section: sorted acquisition so concurrent multi-digest publishes cannot deadlock; Release never removes the lock files (flock synchronizes on the inode). +// BlobLocks holds per-digest blob locks for a publish; Lock sorts paths so concurrent publishes cannot deadlock. type BlobLocks struct { held []*gofrsflock.Flock } diff --git a/main.go b/main.go index 52030419..24008028 100644 --- a/main.go +++ b/main.go @@ -13,7 +13,7 @@ import ( func main() { ctx := context.Background() - // Console relay mode: this binary is re-exec'd as a background process by FC launchProcess for the PTY bridge. + // Console relay mode: this binary is re-exec'd as a background process by FC startConsoleRelay for the PTY bridge. if firecracker.IsRelayMode() { firecracker.RunRelay(ctx) return diff --git a/meta/json/codec.go b/meta/json/codec.go index b6aa6433..3a52c414 100644 --- a/meta/json/codec.go +++ b/meta/json/codec.go @@ -10,7 +10,6 @@ import ( // Codec maps between a namespace file's bytes and its Model; Encode output is the complete file content including the trailing newline. type Codec interface { - // Decode parses file bytes. Decode(data []byte) (*Model, error) Encode(m *Model) ([]byte, error) } diff --git a/meta/json/events.go b/meta/json/events.go index 19c7b2b1..753853bd 100644 --- a/meta/json/events.go +++ b/meta/json/events.go @@ -34,7 +34,7 @@ type fileToken struct { mtime int64 } -// tokens are touched only by newNotifier and the Run goroutine, never concurrently. +// Tokens are touched only by newNotifier and the Run goroutine, never concurrently. type notifier struct { b *meta.Broadcaster watcher *fsnotify.Watcher // kept for the severed-watch test seam diff --git a/meta/json/model.go b/meta/json/model.go index 136979d7..64192e7d 100644 --- a/meta/json/model.go +++ b/meta/json/model.go @@ -5,6 +5,8 @@ import ( "encoding/json/jsontext" "maps" "slices" + + "github.com/cocoonstack/cocoon/meta" ) type table struct { @@ -62,7 +64,7 @@ func (m *Model) Delete(tbl, id string) { func (m *Model) Dirty() bool { return m.dirty } // Scan yields (tbl, id) pairs in insertion order; fn errors abort and propagate. -func (m *Model) Scan(tbl string, fn func(id string, raw jsontext.Value) error) error { +func (m *Model) Scan(tbl string, fn meta.RawScanFunc) error { t := m.tables[tbl] if t == nil { return nil diff --git a/meta/json/store.go b/meta/json/store.go index b1ca8468..7140db62 100644 --- a/meta/json/store.go +++ b/meta/json/store.go @@ -149,11 +149,12 @@ func (s *Store) resolve(nss []string) ([]*nsState, error) { // withLocked holds every namespace flock in sorted order; unlock errors log only, since joining them would make callers roll back an already-durable commit. func (s *Store) withLocked(ctx context.Context, states []*nsState, fn func() error) error { + logger := log.WithFunc("meta.json.withLocked") for i, st := range states { if err := st.locker.Lock(ctx); err != nil { for _, held := range slices.Backward(states[:i]) { if uerr := held.locker.Unlock(ctx); uerr != nil { - log.WithFunc("meta.json.withLocked").Errorf(ctx, uerr, "unlock %s", held.def.Name) + logger.Errorf(ctx, uerr, "unlock %s", held.def.Name) } } return fmt.Errorf("lock %s: %w", st.def.Name, err) @@ -162,7 +163,7 @@ func (s *Store) withLocked(ctx context.Context, states []*nsState, fn func() err defer func() { for _, st := range slices.Backward(states) { if err := st.locker.Unlock(ctx); err != nil { - log.WithFunc("meta.json.withLocked").Errorf(ctx, err, "unlock %s", st.def.Name) + logger.Errorf(ctx, err, "unlock %s", st.def.Name) } } }() @@ -303,7 +304,7 @@ func commitLocked(st *nsState, l *loaded) error { return crash("main-renamed") } -// syncCommitted runs after the flock is released: main first so a .prev sync failure still leaves the caller's own generation durable; the parent-dir sync is what CommitRelaxed relinquishes. +// Syncs main first so a .prev sync failure still leaves the caller's generation durable. func syncCommitted(st *nsState, mode meta.CommitMode) error { path := st.def.FilePath if err := utils.SyncFile(path); err != nil { diff --git a/meta/meta.go b/meta/meta.go index 3f1608f6..a7848613 100644 --- a/meta/meta.go +++ b/meta/meta.go @@ -38,13 +38,13 @@ type CommitMode uint8 // WriteOpt modifies a single write; see RelaxedOK. type WriteOpt uint8 -// Scope declares, before the closure runs, every namespace a transaction touches: Write is the single namespace it may modify, Read the others it may read. Engines acquire all of them in one fixed global order before invoking the closure. +// Scope declares, before the closure runs, every namespace a transaction touches. type Scope struct { Write string Read []string } -// Store is the engine-neutral transaction boundary. Closures must be pure and retryable: they may run more than once, all effects go through the handle, and results are published only after the transaction returns nil. +// Store is the engine-neutral transaction boundary. Closures must be pure and retryable. type Store interface { // View runs fn over a consistent snapshot of the given namespaces. View(ctx context.Context, nss []string, fn ViewFunc) error diff --git a/meta/sqlite/events.go b/meta/sqlite/events.go index b1e87c74..be469202 100644 --- a/meta/sqlite/events.go +++ b/meta/sqlite/events.go @@ -10,7 +10,7 @@ import ( "github.com/cocoonstack/cocoon/meta" ) -// Events subscribes to committed-change signals; data_version is comparable only across calls on one pinned connection and never moves for that connection's own commits (§7). +// Events pins one connection; data_version moves only for other connections' commits (§7). func (s *Store) Events(ctx context.Context) (<-chan struct{}, func(), error) { s.mu.Lock() defer s.mu.Unlock() @@ -26,7 +26,7 @@ func (s *Store) Events(ctx context.Context) (<-chan struct{}, func(), error) { return ch, func() { stop(); release() }, nil } -// notifier holds the pinned data_version connection; version is touched only by the init call and the Run goroutine. ctx is the notifier's own lifetime — it outlives every Events caller and ends at stop. +// Version is touched only by the init call and the Run goroutine; ctx lives until stop. type notifier struct { b *meta.Broadcaster watcher *fsnotify.Watcher // kept for the severed-watch test seam diff --git a/meta/sqlite/store.go b/meta/sqlite/store.go index 6fa7c4b6..0557e70b 100644 --- a/meta/sqlite/store.go +++ b/meta/sqlite/store.go @@ -33,7 +33,6 @@ const ( DBFileName = "meta.db" ManifestName = "meta-convert.manifest" - // busyRetryCeiling bounds the BEGIN IMMEDIATE retry loop; busyRetryPause caps the jittered pause between retries (§4). busyRetryCeiling = 30 * time.Second busyRetryPause = 2 * time.Millisecond slowTxnWarn = 500 * time.Millisecond diff --git a/meta/tombstone/tombstone.go b/meta/tombstone/tombstone.go index 27cfaa7f..8316ecb1 100644 --- a/meta/tombstone/tombstone.go +++ b/meta/tombstone/tombstone.go @@ -156,7 +156,7 @@ func (t *Table) Acquire(ctx context.Context, w meta.Writer, id string, build fun return leaseID, nil, err } -// Recover takes over id's tombstone under the held entity lock and decodes a deleting-phase payload into cl; a leased entry rolls back in place and a nil Record means nothing to roll forward. +// Recover takes over id's tombstone and decodes a deleting-phase payload into cl; a leased entry rolls back and returns nil. func (t *Table) Recover(ctx context.Context, w meta.Writer, id string, cl any) (*Record, error) { rec, err := t.TakeOver(ctx, w, id) if err != nil || rec == nil { diff --git a/metadata/fat12.go b/metadata/fat12.go index 7ba9e7fd..a69a2f4a 100644 --- a/metadata/fat12.go +++ b/metadata/fat12.go @@ -29,7 +29,6 @@ const ( writeBufSize = 64 << 10 ) -// fat12Builder constructs a FAT12 image in memory (FAT + root dir only) and streams the full image on writeTo. type fat12Builder struct { label string fat []byte // single FAT copy (written twice) @@ -37,7 +36,7 @@ type fat12Builder struct { data [][]byte // file data in cluster-allocation order nextCluster uint16 rootUsed int - shortSeq int // counter for ~N short-name suffixes + shortSeq int } func newFAT12Builder(label string) *fat12Builder { @@ -202,7 +201,6 @@ func setFATEntry(fat []byte, cluster int, val uint16) { fat[off+1] = byte(word >> 8) } -// needsLFN reports whether name requires VFAT long-filename entries. func needsLFN(name string) bool { upper := strings.ToUpper(name) base, ext := splitName(upper) diff --git a/metadata/metadata.go b/metadata/metadata.go index 58db354f..7a584fb8 100644 --- a/metadata/metadata.go +++ b/metadata/metadata.go @@ -22,7 +22,7 @@ var ( "instance-id: {{.InstanceID}}\nlocal-hostname: {{.Hostname}}\n", )) - // userDataTmpl renders cloud-config; also writes systemd-networkd fallback units so clone reinit survives netplan PERM-MAC mismatch. + // Fallback units survive netplan PERM-MAC mismatch on clone reinit. userDataTmpl = template.Must(template.New("user-data").Funcs(tmplFuncs).Parse(`#cloud-config {{- if and .Password .Username}} ssh_pwauth: true @@ -119,7 +119,7 @@ type Config struct { Username string Password string Networks []NetworkInfo - Mounts []MountSpec // optional fstab entries written by cloud-init + Mounts []MountSpec DNS []string } diff --git a/metering/metalog/metalog.go b/metering/metalog/metalog.go index 1cef3dd6..91a5f73b 100644 --- a/metering/metalog/metalog.go +++ b/metering/metalog/metalog.go @@ -1,4 +1,4 @@ -// Package metalog records metering entries in the meta store's log (§1 P3): one Relaxed append per entry with the Seq cursor committed alongside. +// Package metalog records metering entries in the meta store's log, one relaxed append per entry. package metalog import ( diff --git a/metering/metering.go b/metering/metering.go index f98a542b..d3edac8a 100644 --- a/metering/metering.go +++ b/metering/metering.go @@ -1,4 +1,4 @@ -// Package metering emits append-only VM/snapshot lifecycle endpoints through Recorder; backends live in subpackages (file, stderr, capture). +// Package metering emits append-only VM and snapshot lifecycle entries through Recorder; backends live in subpackages. package metering import ( diff --git a/network/bridge/bridge_linux.go b/network/bridge/bridge_linux.go index d45f41cf..2c135f10 100644 --- a/network/bridge/bridge_linux.go +++ b/network/bridge/bridge_linux.go @@ -30,7 +30,7 @@ type Bridge struct { bridgeIdx int } -// New: the bridge device must already exist. +// New returns a Bridge on bridgeDev, which must already exist. func New(conf *config.Config, bridgeDev string) (*Bridge, error) { br, err := netlink.LinkByName(bridgeDev) if err != nil { @@ -96,7 +96,7 @@ func (b *Bridge) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, sp if spec.Existing != nil { mac = spec.Existing.MAC } - // Fresh adds only: a same-name TAP is an interrupted-resize leftover that would wedge every retry; recovery specs re-attach the existing TAP (their slot may hold a live VMM's TAP). + // Fresh adds reclaim a same-name stale TAP; recovery specs re-attach the existing one. if spec.Existing == nil { if old, lErr := netlink.LinkByName(name); lErr == nil { if delErr := netlink.LinkDel(old); delErr != nil { @@ -141,7 +141,7 @@ func (b *Bridge) Remove(_ context.Context, vmID string, index int) error { return nil } -// Quiesce and Unquiesce are no-ops: bridge TAPs sit directly on the host bridge, with no TC redirect to storm when the VM stops. +// Bridge TAPs have no TC redirect to storm. func (b *Bridge) Quiesce(_ context.Context, _ string) error { return nil } func (b *Bridge) Unquiesce(_ context.Context, _ string) error { return nil } diff --git a/network/cni/cni.go b/network/cni/cni.go index 0c047f5c..98eb5ad1 100644 --- a/network/cni/cni.go +++ b/network/cni/cni.go @@ -80,7 +80,6 @@ func New(conf *config.Config, store meta.Store) (*CNI, error) { func (c *CNI) Type() string { return typ } -// Verify checks that no teardown is pending, the netns exists, and every expected TAP carries its ingress redirect. func (c *CNI) Verify(ctx context.Context, vmID string, expected []*types.NetworkConfig) error { var pending *tombstone.Record if err := c.view(ctx, func(t *netTx) error { @@ -123,7 +122,7 @@ func (c *CNI) Delete(ctx context.Context, vmID string) error { return c.teardownProtocol(ctx, vmID, nil, false) } -// tearDownNICs runs CNI DEL (+ optional TAP delete) on every record, returning the fully-torn-down record IDs (the caller's sweep set) and the joined failures. +// tearDownNICs runs CNI DEL (and the optional TAP delete) per record; it returns fully released IDs and the joined errors. func (c *CNI) tearDownNICs(ctx context.Context, vmID, nsPath string, records []networkRecord, deleteTAP bool) ([]string, error) { logger := log.WithFunc("cni.tearDownNICs") // Zero records need no plugin: a missing conflist must not wedge a 0-NIC VM's netns removal. @@ -220,7 +219,7 @@ func loadConfLists(dir string) (map[string]*libcni.NetworkConfigList, string, er if len(files) == 0 { return nil, "", fmt.Errorf("no .conflist files in %s", dir) } - // files are already sorted by ConfFiles. + // Files are already sorted by ConfFiles. lists := make(map[string]*libcni.NetworkConfigList, len(files)) var defaultName string for _, f := range files { diff --git a/network/cni/db.go b/network/cni/db.go index be33dedd..0d916696 100644 --- a/network/cni/db.go +++ b/network/cni/db.go @@ -8,9 +8,7 @@ import ( type networkRecord struct { types.Network `json:"network"` ID string `json:"id"` - // Type is the CNI conflist name (e.g. "cocoon", "calico"). - Type string `json:"type"` - VMID string `json:"vm_id"` - // IfName is the CNI interface name inside the netns (eth0, eth1, ...). - IfName string `json:"if_name"` + Type string `json:"type"` + VMID string `json:"vm_id"` + IfName string `json:"if_name"` } diff --git a/network/cni/lifecycle.go b/network/cni/lifecycle.go index 6d28c2c0..0fd7d170 100644 --- a/network/cni/lifecycle.go +++ b/network/cni/lifecycle.go @@ -34,7 +34,6 @@ func (c *CNI) Prepare(_ context.Context, vmID string, _ *types.VMConfig) (string return nsPath, nil } -// Add allocates each NIC's CNI plumbing inside the netns Prepare created. func (c *CNI) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs ...network.AddSpec) (configs []*types.NetworkConfig, retErr error) { if c.cniConf == nil { return nil, c.errNoConflist() @@ -70,7 +69,7 @@ func (c *CNI) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs if retErr == nil { return } - // Survives caller cancellation, bounded so a hung plugin can't wedge the caller; a failed DEL keeps its intent record so a later re-add, vm rm or GC releases the lease. + // Rollback survives caller cancellation but is bounded; a failed DEL keeps its intent record. rctx, rcancel := context.WithTimeout(context.WithoutCancel(ctx), rollbackTimeout) defer rcancel() var releasedIDs []string diff --git a/network/cni/lifecycle_linux.go b/network/cni/lifecycle_linux.go index 5eef31fd..4094d626 100644 --- a/network/cni/lifecycle_linux.go +++ b/network/cni/lifecycle_linux.go @@ -46,7 +46,7 @@ func createNetns(name string) error { return nil } -// deleteNetns removes a named netns with retry for async kernel cleanup. +// deleteNetns retries because kernel netns cleanup is async. func deleteNetns(ctx context.Context, name string) error { path := filepath.Join(netnsBasePath, name) var last error @@ -137,23 +137,23 @@ func setLinkStateInNetns(nsPath string, ifNames []string, up bool) error { return ignoreMissingNetns(err) } -func setupTCRedirect(nsPath, ifName, tapName string, queues int, overrideMAC string) (string, int, error) { +func setupTCRedirect(nsPath, ifn, tapName string, queues int, overrideMAC string) (string, int, error) { var ( mac string mtu int ) err := cns.WithNetNSPath(nsPath, func(_ cns.NetNS) error { var nsErr error - mac, mtu, nsErr = tcRedirectInNS(ifName, tapName, queues, overrideMAC) + mac, mtu, nsErr = tcRedirectInNS(ifn, tapName, queues, overrideMAC) return nsErr }) return mac, mtu, err } -func tcRedirectInNS(ifName, tapName string, queues int, overrideMAC string) (string, int, error) { - link, err := netlink.LinkByName(ifName) +func tcRedirectInNS(ifn, tapName string, queues int, overrideMAC string) (string, int, error) { + link, err := netlink.LinkByName(ifn) if err != nil { - return "", 0, fmt.Errorf("find %s: %w", ifName, err) + return "", 0, fmt.Errorf("find %s: %w", ifn, err) } if overrideMAC != "" { @@ -162,7 +162,7 @@ func tcRedirectInNS(ifName, tapName string, queues int, overrideMAC string) (str return "", 0, fmt.Errorf("parse MAC %s: %w", overrideMAC, parseErr) } if setErr := netlink.LinkSetHardwareAddr(link, hwAddr); setErr != nil { - return "", 0, fmt.Errorf("set MAC on %s: %w", ifName, setErr) + return "", 0, fmt.Errorf("set MAC on %s: %w", ifn, setErr) } } @@ -170,11 +170,11 @@ func tcRedirectInNS(ifName, tapName string, queues int, overrideMAC string) (str addrs, err := netlink.AddrList(link, netlink.FAMILY_ALL) if err != nil { - return "", 0, fmt.Errorf("list addrs on %s: %w", ifName, err) + return "", 0, fmt.Errorf("list addrs on %s: %w", ifn, err) } for _, addr := range addrs { if delErr := netlink.AddrDel(link, &addr); delErr != nil { - return "", 0, fmt.Errorf("flush addr %s on %s: %w", addr.IPNet, ifName, delErr) + return "", 0, fmt.Errorf("flush addr %s on %s: %w", addr.IPNet, ifn, delErr) } } @@ -212,10 +212,10 @@ func tcRedirectInNS(ifName, tapName string, queues int, overrideMAC string) (str } if err := replaceTCRedirect(link, tapLink); err != nil { - return "", 0, fmt.Errorf("redirect %s -> %s: %w", ifName, tapName, err) + return "", 0, fmt.Errorf("redirect %s -> %s: %w", ifn, tapName, err) } if err := replaceTCRedirect(tapLink, link); err != nil { - return "", 0, fmt.Errorf("redirect %s -> %s: %w", tapName, ifName, err) + return "", 0, fmt.Errorf("redirect %s -> %s: %w", tapName, ifn, err) } return mac, link.Attrs().MTU, nil } diff --git a/network/cni/metans.go b/network/cni/metans.go index d89ac5ad..c87ccdf3 100644 --- a/network/cni/metans.go +++ b/network/cni/metans.go @@ -11,6 +11,8 @@ const ( TableRecords = "records" ) +type netTxFn func(*netTx) error + // netTx is the CNI view of one meta transaction: a records-only namespace with the vm_id secondary lookup served by scan. type netTx struct { *meta.RecordTx[networkRecord] @@ -30,13 +32,13 @@ func (t *netTx) byVMID(vmID string) ([]networkRecord, error) { return out, nil } -func (c *CNI) view(ctx context.Context, fn func(*netTx) error) error { +func (c *CNI) view(ctx context.Context, fn netTxFn) error { return c.meta.View(ctx, []string{NamespaceName}, func(r meta.Reader) error { return fn(c.tx(ctx, r, nil)) }) } -func (c *CNI) update(ctx context.Context, fn func(*netTx) error) error { +func (c *CNI) update(ctx context.Context, fn netTxFn) error { return c.meta.Update(ctx, meta.Scope{Write: NamespaceName}, meta.CommitDurable, func(w meta.Writer) error { return fn(c.tx(ctx, w, w)) }) diff --git a/network/cni/teardown.go b/network/cni/teardown.go index ac4559f7..36feecb5 100644 --- a/network/cni/teardown.go +++ b/network/cni/teardown.go @@ -102,13 +102,12 @@ func (c *CNI) finishTeardown(ctx context.Context, vmID, leaseID string, cl netCl for _, r := range cl.Records { records = append(records, networkRecord{ID: r.ID, Type: r.Type, VMID: vmID, IfName: r.IfName}) } - // A retry after the netns already went (crash between netns removal and the sweep) skips TAP deletion — the TAPs died with the ns; CNI DEL still runs, releasing IPAM by container ID without entering the ns. + // A retry after the netns is gone skips TAP deletion; CNI DEL still releases IPAM. nsPath := c.conf.netnsPath(vmID) if _, err := statNetnsFn(nsPath); errors.Is(err, fs.ErrNotExist) { deleteTAP, nsPath = false, "" } downIDs, tdErr := c.tearDownNICs(ctx, vmID, nsPath, records, deleteTAP) - // Slow cleanup stays outside the transaction (clause 1): the netns goes before the commit so a pure retryable closure never carries side effects. if tdErr == nil && cl.Netns != "" { if err := deleteNetnsFn(ctx, c.conf.netnsName(vmID)); err != nil && !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("remove netns %s (tombstone kept, retry resumes): %w", cl.Netns, err) diff --git a/network/tap_linux.go b/network/tap_linux.go index cc61842b..8eff7063 100644 --- a/network/tap_linux.go +++ b/network/tap_linux.go @@ -10,16 +10,16 @@ import ( ) const ( - // TAPTxQueueLen absorbs traffic bursts (especially UDP) without dropping; the kernel default of 1000 is too small for VM workloads. + // TAPTxQueueLen absorbs UDP bursts; the kernel default of 1000 is too small. TAPTxQueueLen = 10000 - // GROMaxSize matches the maximum virtio-net segment size so the kernel aggregates inbound packets before CH reads them. + // GROMaxSize matches the maximum virtio-net segment size. GROMaxSize = 65536 ) -// CreateTAP adds a multi-queue TAP sized for numQueues virtio-net queues and returns its index, then closes the kernel fds (CH/QEMU reopen it by name). +// CreateTAP adds a multi-queue TAP and returns its index; the VMM reopens it by name. func CreateTAP(name string, numQueues int) (int, error) { - // queue_pairs = num_queues / 2 (TX+RX pair); multi-queue needs >1 and must match the VMM's IFF_MULTI_QUEUE expectation. + // Queue pairs = num_queues / 2 (TX+RX pair); multi-queue needs >1 and must match the VMM's IFF_MULTI_QUEUE expectation. queuePairs := max(1, numQueues/2) flags := netlink.TUNTAP_VNET_HDR | netlink.TUNTAP_NO_PI if queuePairs <= 1 { @@ -36,7 +36,7 @@ func CreateTAP(name string, numQueues int) (int, error) { if err := netlink.LinkAdd(tap); err != nil { return 0, fmt.Errorf("add tap %s: %w", name, err) } - // netlink's post-TUNSETIFF index lookup drops its error; on a zero index drop persistence so the device dies with the fds instead of outliving the failure. + // Netlink drops the index-lookup error; on index 0, clear persistence so the TAP dies with the fds. index := tap.Attrs().Index if index == 0 { _ = unix.IoctlSetInt(int(tap.Fds[0].Fd()), unix.TUNSETPERSIST, 0) diff --git a/snapshot/envelope.go b/snapshot/envelope.go index 82280568..1d6bb6b4 100644 --- a/snapshot/envelope.go +++ b/snapshot/envelope.go @@ -66,7 +66,7 @@ func WriteSnapshotEnvelope(dir string, cfg types.SnapshotConfig, files, diskAtta return utils.AtomicWriteFile(filepath.Join(dir, SnapshotJSONName), data, 0o644, utils.Sync) } -// VerifyManifest checks that every file the envelope lists landed in dir; a tar cut at an entry boundary reads as complete, so the manifest is what catches it. +// VerifyManifest checks that every file the envelope lists is present in dir. func VerifyManifest(dir string, files []string) error { for _, name := range files { if _, err := os.Stat(filepath.Join(dir, name)); err != nil { diff --git a/snapshot/localfile/export.go b/snapshot/localfile/export.go index 3d79c9f2..6c4a4a1b 100644 --- a/snapshot/localfile/export.go +++ b/snapshot/localfile/export.go @@ -20,12 +20,11 @@ func (lf *LocalFile) Export(ctx context.Context, ref string) (io.ReadCloser, err return lf.export(ctx, ref, false) } -// ExportCompressed streams the snapshot as a gzip-compressed tar archive. func (lf *LocalFile) ExportCompressed(ctx context.Context, ref string) (io.ReadCloser, error) { return lf.export(ctx, ref, true) } -// ExportToDir reflinks snapshot data into dir + writes snapshot.json last so its presence is the all-data-ready marker for --from-dir. +// snapshot.json is written last as the all-data-ready marker. func (lf *LocalFile) ExportToDir(ctx context.Context, ref, dir string) (err error) { dataDir, cfg, release, err := lf.DataDir(ctx, ref) if err != nil { @@ -51,7 +50,7 @@ func (lf *LocalFile) ExportToDir(ctx context.Context, ref, dir string) (err erro if err != nil { return err } - // a partial tree would make the retry refuse the non-empty target + // A partial tree would make the retry refuse the non-empty target defer func() { if err != nil { for _, name := range names { diff --git a/snapshot/localfile/gc.go b/snapshot/localfile/gc.go index 1a2f5e52..9fbef65a 100644 --- a/snapshot/localfile/gc.go +++ b/snapshot/localfile/gc.go @@ -45,7 +45,7 @@ type snapshotGCSnapshot struct { dataDirs []string leaseIDs []string stalePending []string - missingDir []string // finalized records whose data dir vanished (rm dir-deleted, DB update failed) + missingDir []string // finalized records whose data dir vanished records map[string]snapshotMeta reasons map[string]string policy EvictionPolicy @@ -67,7 +67,6 @@ func (lf *LocalFile) PinnedBlobIDs(ctx context.Context) (map[string]struct{}, er return pins, nil } -// gcRecover resumes existing snapshot tombstones by phase before discovery. func (lf *LocalFile) gcRecover(ctx context.Context) []error { var ids []string if err := lf.view(ctx, func(t *snapTx) error { @@ -114,7 +113,7 @@ func gcModule(lf *LocalFile, policy EvictionPolicy) gc.Module[snapshotGCSnapshot }); err != nil { return snap, err } - // the stats run after the read transaction closes: a large store would otherwise pin WAL frames for the whole walk + // The stats run after the read transaction closes: a large store would otherwise pin WAL frames for the whole walk for id := range snap.records { if _, statErr := os.Stat(conf.SnapshotDataDir(id)); errors.Is(statErr, fs.ErrNotExist) { snap.missingDir = append(snap.missingDir, id) @@ -163,7 +162,7 @@ func gcModule(lf *LocalFile, policy EvictionPolicy) gc.Module[snapshotGCSnapshot errs = append(errs, err) break } - // an exclusive acquire proves the pending record's owner died; no age gate needed + // An exclusive acquire proves the pending record's owner died; no age gate needed release, ok, lockErr := lf.tryExclusiveLease(ctx, id) if lockErr != nil { errs = append(errs, lockErr) @@ -173,7 +172,7 @@ func gcModule(lf *LocalFile, policy EvictionPolicy) gc.Module[snapshotGCSnapshot logger.Warnf(ctx, "skip %s: leased by an active holder", id) continue } - // the reason picked at ReadDB must still hold, or a create in the window evicts the wrong snapshot + // The reason picked at ReadDB must still hold, or a create in the window evicts the wrong snapshot var sawRecord bool revalidate := func(rec *snapshotRecord) bool { sawRecord = true diff --git a/snapshot/localfile/import.go b/snapshot/localfile/import.go index a136abd3..bf179f19 100644 --- a/snapshot/localfile/import.go +++ b/snapshot/localfile/import.go @@ -45,7 +45,7 @@ func (lf *LocalFile) Import(ctx context.Context, r io.Reader, name, description if err = utils.ExtractTar(dataDir, tarReader); err != nil { return "", fmt.Errorf("extract archive: %w", err) } - // Drain to true EOF before Close: tar stops at its end-of-archive blocks, and gzip only verifies the CRC32/size trailer once the stream is read out. + // Drain to EOF before Close: gzip verifies its CRC trailer only once the stream is read out. if gzCloser != nil { if _, err = io.Copy(io.Discard, tarReader); err != nil { return "", fmt.Errorf("gzip integrity check: %w", err) @@ -104,7 +104,7 @@ func (lf *LocalFile) Import(ctx context.Context, r io.Reader, name, description return id, nil } -// unwrapGzip sniffs the 2-byte gzip magic (0x1f 0x8b) and returns the tar reader plus a gzip closer (nil for raw tar). +// unwrapGzip returns the tar reader plus a gzip closer (nil for raw tar). func unwrapGzip(r io.Reader) (io.Reader, io.Closer, error) { head, full, err := utils.PeekReader(r, 2) if err != nil { @@ -123,7 +123,7 @@ func unwrapGzip(r io.Reader) (io.Reader, io.Closer, error) { return full, nil, nil } -// readAndRemoveSnapshotJSON reads the envelope and deletes it; the registered snapshot dir keeps only runtime sidecars (cocoon.json), not import metadata. +// readAndRemoveSnapshotJSON reads the envelope and deletes it so the data dir holds only runtime sidecars. func readAndRemoveSnapshotJSON(dataDir string) (types.SnapshotExport, error) { envelope, err := snapshot.ReadSnapshotEnvelope(dataDir) if err != nil { diff --git a/snapshot/localfile/localfile.go b/snapshot/localfile/localfile.go index fe05c02f..1a972edc 100644 --- a/snapshot/localfile/localfile.go +++ b/snapshot/localfile/localfile.go @@ -52,6 +52,7 @@ var ( _ snapshot.DirectCreator = (*LocalFile)(nil) _ snapshot.CompressedExporter = (*LocalFile)(nil) _ snapshot.DirectoryExporter = (*LocalFile)(nil) + _ snapshot.NameHolder = (*LocalFile)(nil) ) // LocalFile is the local-filesystem snapshot backend. @@ -84,13 +85,12 @@ func New(conf *config.Config, rec metering.Recorder, store meta.Store, opts ...O func (lf *LocalFile) Type() string { return typ } -// DataDir returns the local data directory, snapshot config, and a read-lease release for direct file access. func (lf *LocalFile) DataDir(ctx context.Context, ref string) (string, types.SnapshotConfig, func(), error) { rec, err := lf.lookupRecord(ctx, ref, true) if err != nil { return "", types.SnapshotConfig{}, nil, err } - // A shared holder can't drive recovery, so this releases the shared lease, recovers exclusively, then re-acquires shared to avoid self-deadlocking an in-place upgrade. + // Release shared, recover exclusively, then re-acquire: a shared holder cannot upgrade in place. for range 2 { release, err := lf.acquireReadLease(ctx, rec.ID) if err != nil { @@ -138,7 +138,7 @@ func (lf *LocalFile) Create(ctx context.Context, cfg *types.SnapshotConfig, stre return cfg.ID, nil } -// CreateFromDir renames srcDir into the store data dir when they share a filesystem and reports ok=false (srcDir untouched) otherwise so the caller streams instead; the placeholder→move→finalize protocol matches Create, so a mid-flight crash leaves a GC-collectable pending record. +// CreateFromDir renames srcDir into the store; ok=false (srcDir untouched) means the caller must stream. func (lf *LocalFile) CreateFromDir(ctx context.Context, cfg *types.SnapshotConfig, srcDir string) (_ string, _ bool, err error) { sameFS, err := utils.SameFilesystem(srcDir, lf.conf.DataDir()) if err != nil { @@ -230,7 +230,7 @@ func (lf *LocalFile) RegisterGC(orch *gc.Orchestrator) { orch.Register(gcModule(lf, lf.gcPolicy)) } -// NameOwner reports the record holding name in the index, pending or not — this is the reservation insertRecord enforces, so the save preflight sees the same thing the insert will. +// NameOwner reports the record holding name, pending or not, matching what insertRecord enforces. func (lf *LocalFile) NameOwner(ctx context.Context, name string) (string, bool, error) { if name == "" { return "", false, nil @@ -310,7 +310,7 @@ func (lf *LocalFile) deleteOne(ctx context.Context, id string) error { return nil } -// beginBuild is the shared Create/CreateFromDir preamble: build lease + pending record; done (deferred with the caller's named error) removes the half-built dir and rolls the record back on failure, then releases the lease. +// beginBuild takes the build lease and inserts the pending record; done rolls back on a non-nil *err and releases the lease. func (lf *LocalFile) beginBuild(ctx context.Context, cfg *types.SnapshotConfig) (dataDir string, done func(*error), err error) { if cfg.ID == "" { return "", nil, fmt.Errorf("snapshot ID is required (must be set by caller)") diff --git a/snapshot/localfile/teardown.go b/snapshot/localfile/teardown.go index 0f032bf9..6fd1ac29 100644 --- a/snapshot/localfile/teardown.go +++ b/snapshot/localfile/teardown.go @@ -25,7 +25,7 @@ func (lf *LocalFile) tombstones() *tombstone.Table { return tombstone.NewTable(NamespaceName) } -// deleteSnapshotProtocol runs the phase protocol for one snapshot under its held exclusive lease; revalidate, when set, re-checks candidacy inside the lease transaction and a false return skips without error. +// deleteSnapshotProtocol deletes id under its held exclusive lease; a false revalidate skips without error. func (lf *LocalFile) deleteSnapshotProtocol(ctx context.Context, id string, revalidate func(*snapshotRecord) bool) (deleted bool, cl snapCleanup, err error) { ts := lf.tombstones() var ( diff --git a/types/config.go b/types/config.go index 03daa86f..aceadd7d 100644 --- a/types/config.go +++ b/types/config.go @@ -14,7 +14,7 @@ type Config struct { QueueSize int `json:"queue_size,omitzero"` // virtio-net ring depth per queue; 0 = default DiskQueueSize int `json:"disk_queue_size,omitzero"` // virtio-blk ring depth per device; 0 = default Image string `json:"image,omitempty"` - ImageDigest string `json:"image_digest,omitempty"` // resolved image digest (e.g. "sha256:abc123") + ImageDigest string `json:"image_digest,omitempty"` ImageType string `json:"image_type,omitempty"` Network string `json:"network,omitempty"` // CNI conflist name; empty = default NoDirectIO bool `json:"no_direct_io,omitzero"` // disable O_DIRECT on writable disks diff --git a/types/network.go b/types/network.go index 8a760e83..ff412331 100644 --- a/types/network.go +++ b/types/network.go @@ -32,7 +32,7 @@ type NetworkConfig struct { Network *Network `json:"network,omitzero"` } -// Network is the guest-visible IP config for a NIC; all fields omitempty so DHCP NICs serialize empty. +// Network is the guest-visible IP config for a NIC; DHCP NICs serialize empty. type Network struct { IP string `json:"ip,omitempty"` // dotted decimal, e.g. "10.0.0.2" Gateway string `json:"gateway,omitempty"` // dotted decimal, e.g. "10.0.0.1" diff --git a/types/storage.go b/types/storage.go index 91d5bfdd..1d0fdf95 100644 --- a/types/storage.go +++ b/types/storage.go @@ -49,7 +49,7 @@ type DataDiskSpec struct { Size int64 FSType string MountPoint string - MountPointSet bool `json:"-"` // distinguishes mount= (set) from omitted + MountPointSet bool // distinguishes mount= (set) from omitted DirectIO *bool } diff --git a/types/vm.go b/types/vm.go index d3325776..f38083c7 100644 --- a/types/vm.go +++ b/types/vm.go @@ -11,9 +11,9 @@ import ( const ( VMStateCreating VMState = "creating" // DB placeholder written, dirs/disks being prepared VMStateCreated VMState = "created" // registered, VMM process not yet started - VMStateRunning VMState = "running" // VMM process alive, guest is up - VMStateStopped VMState = "stopped" // VMM process has exited cleanly - VMStateError VMState = "error" // start, stop, or restore failed + VMStateRunning VMState = "running" + VMStateStopped VMState = "stopped" + VMStateError VMState = "error" TransitionCreate TransitionReason = "create" TransitionBoot TransitionReason = "boot" diff --git a/utils/atomic.go b/utils/atomic.go index 382d7c1c..9b243f86 100644 --- a/utils/atomic.go +++ b/utils/atomic.go @@ -30,7 +30,7 @@ func ReadJSONFile(path string, v any) error { return nil } -// SyncParentDir fsyncs the directory containing the file to ensure the directory entry is persisted. +// SyncParentDir fsyncs dir so its entries are durable. func SyncParentDir(dir string) error { parent, err := os.Open(dir) //nolint:gosec // directory is derived from cocoon-managed target path if err != nil { @@ -55,7 +55,7 @@ func SyncFile(path string) (err error) { return f.Sync() } -// SyncTree fsyncs every regular file directly under dir (snapshot dirs are flat), then dir and its parent — the durable-before-kill barrier hibernate needs, since a bare rename fsyncs nothing. +// SyncTree fsyncs dir's regular files, then dir and its parent; a bare rename persists nothing. func SyncTree(dir string) error { entries, err := os.ReadDir(dir) if err != nil { @@ -75,7 +75,7 @@ func SyncTree(dir string) error { return SyncParentDir(filepath.Dir(dir)) } -// AtomicWriteFile writes data via temp + rename so readers never see a partial file; Sync fsyncs it, NoSync suits run-dir files regenerated on the next launch. +// AtomicWriteFile writes data via temp + rename so readers never see a partial file. func AtomicWriteFile(path string, data []byte, perm os.FileMode, mode SyncMode) error { dir := filepath.Dir(path) tmp, err := os.CreateTemp(dir, ".tmp-*") diff --git a/utils/file.go b/utils/file.go index 6d558431..b738ec3a 100644 --- a/utils/file.go +++ b/utils/file.go @@ -20,7 +20,7 @@ import ( // StaleTempAge is the age threshold for removing stale temp files during GC. const StaleTempAge = time.Hour -// SameFilesystem reports whether a and b live on the same filesystem (matching st_dev), so a rename or hardlink between them can succeed rather than failing with EXDEV. +// SameFilesystem reports whether a and b share a st_dev, so rename or hardlink avoids EXDEV. func SameFilesystem(a, b string) (bool, error) { sa, err := os.Stat(a) if err != nil { @@ -88,10 +88,7 @@ func ScanFileStems(dir, suffix string) ([]string, error) { // ScanSubdirs returns the names of all immediate subdirectories of dir. func ScanSubdirs(dir string) ([]string, error) { return scanDir(dir, func(e os.DirEntry) (string, bool) { - if e.IsDir() { - return e.Name(), true - } - return "", false + return e.Name(), e.IsDir() }) } diff --git a/utils/http.go b/utils/http.go index 9b2f9e2a..6f2cd3e6 100644 --- a/utils/http.go +++ b/utils/http.go @@ -45,7 +45,7 @@ func NewSocketHTTPClientWithTimeout(socketPath string, timeout time.Duration) *h } } -// DoAPI sends a request, validates status, returns body. url must be fully-formed. +// DoAPI sends a request and returns the body when the status is expectedStatus. func DoAPI(ctx context.Context, hc *http.Client, method, url string, body []byte, expectedStatus int) ([]byte, error) { var reqBody io.Reader if body != nil { diff --git a/utils/process.go b/utils/process.go index a729afd6..3bbd60d8 100644 --- a/utils/process.go +++ b/utils/process.go @@ -13,7 +13,7 @@ import ( const killWaitTimeout = 5 * time.Second -// WritePIDFile writes pid to path atomically, so a concurrent ReadPIDFile never observes a truncated or empty file mid-write. +// WritePIDFile writes pid to path atomically. func WritePIDFile(path string, pid int) error { return AtomicWriteFile(path, []byte(strconv.Itoa(pid)+"\n"), 0o600, NoSync) } @@ -40,7 +40,7 @@ func IsProcessAlive(pid int) bool { return err == nil || errors.Is(err, syscall.EPERM) } -// VerifyProcessCmdline matches pid against a whole expectArg argument in /proc//cmdline; falls back to IsProcessAlive on non-Linux or read errors. +// VerifyProcessCmdline matches pid against a whole expectArg argument in /proc//cmdline. func VerifyProcessCmdline(pid int, expectArg string) bool { if pid <= 0 { return false diff --git a/utils/qcow2.go b/utils/qcow2.go index 49184e27..0a513b69 100644 --- a/utils/qcow2.go +++ b/utils/qcow2.go @@ -18,7 +18,7 @@ type Qcow2Header struct { BackingFile string } -// ReadQcow2Header parses the leading 32 bytes of path and the backing file name; ok=false when the file is not qcow2 (wrong magic or shorter than the header). +// ReadQcow2Header parses the qcow2 header and backing file name; ok is false when path is not qcow2. func ReadQcow2Header(path string) (Qcow2Header, bool, error) { f, err := os.Open(path) //nolint:gosec // cocoon-managed or caller-validated path if err != nil { diff --git a/utils/qemuimg.go b/utils/qemuimg.go index ae9da97b..7335a237 100644 --- a/utils/qemuimg.go +++ b/utils/qemuimg.go @@ -7,7 +7,7 @@ import ( "strings" ) -// RunQemuImg shells out to qemu-img; non-zero exit returns wrapped combined output. Use exec.CommandContext directly when stdout matters. +// RunQemuImg shells out to qemu-img; non-zero exit returns wrapped combined output. func RunQemuImg(ctx context.Context, args ...string) error { out, err := exec.CommandContext(ctx, "qemu-img", args...).CombinedOutput() //nolint:gosec if err != nil { diff --git a/utils/reflink_linux.go b/utils/reflink_linux.go index 23fb335c..b375f2d2 100644 --- a/utils/reflink_linux.go +++ b/utils/reflink_linux.go @@ -15,16 +15,16 @@ import ( "golang.org/x/sys/unix" ) -// noReflink remembers the filesystems whose FICLONE answered "not supported", so later copies on them skip the create/ioctl/unlink round trip. +// noReflink holds filesystems whose FICLONE is unsupported, so later copies skip the probe. var noReflink sync.Map // ReflinkCopy copies a single file, preferring FICLONE (O(1) CoW on btrfs/xfs/bcachefs) and falling back to SparseCopy on any error. -func ReflinkCopy(ctx context.Context, dst, src string, sync SyncMode) error { +func ReflinkCopy(ctx context.Context, dst, src string, mode SyncMode) error { fs, known := fsID(filepath.Dir(dst)) if _, unsupported := noReflink.Load(fs); known && unsupported { - return SparseCopy(dst, src, sync) + return SparseCopy(dst, src, mode) } - err := tryFiclone(dst, src, sync) + err := tryFiclone(dst, src, mode) if err == nil { return nil } @@ -33,7 +33,7 @@ func ReflinkCopy(ctx context.Context, dst, src string, sync SyncMode) error { log.WithFunc("utils.ReflinkCopy").Warnf(ctx, "reflink unsupported on the filesystem holding %s (%v); clones copy their disks in full", filepath.Dir(dst), err) } } - return SparseCopy(dst, src, sync) + return SparseCopy(dst, src, mode) } // CheckReflink reports whether the filesystem holding dir supports FICLONE. @@ -49,13 +49,13 @@ func CheckReflink(dir string) error { return tryFiclone(dst, src.Name(), NoSync) } -func tryFiclone(dst, src string, sync SyncMode) error { +func tryFiclone(dst, src string, mode SyncMode) error { return CopyWithCleanup(dst, src, func(srcFile, dstFile *os.File) error { if err := unix.IoctlFileClone(int(dstFile.Fd()), int(srcFile.Fd())); err != nil { return fmt.Errorf("ficlone: %w", err) } // FICLONE only shares extents — still honor Sync, or the fast path silently drops durability. - if sync == Sync { + if mode == Sync { if err := dstFile.Sync(); err != nil { return fmt.Errorf("sync dst: %w", err) } diff --git a/utils/reflink_other.go b/utils/reflink_other.go index 29dfdbb8..823eac12 100644 --- a/utils/reflink_other.go +++ b/utils/reflink_other.go @@ -7,9 +7,9 @@ import ( "errors" ) -// ReflinkCopy copies a single file. On non-Linux, falls back to SparseCopy. -func ReflinkCopy(_ context.Context, dst, src string, sync SyncMode) error { - return SparseCopy(dst, src, sync) +// ReflinkCopy falls back to SparseCopy. +func ReflinkCopy(_ context.Context, dst, src string, mode SyncMode) error { + return SparseCopy(dst, src, mode) } // CheckReflink needs FICLONE, so it fails off Linux. diff --git a/utils/sparse_linux.go b/utils/sparse_linux.go index a328e4aa..7fdb192d 100644 --- a/utils/sparse_linux.go +++ b/utils/sparse_linux.go @@ -13,7 +13,7 @@ import ( ) // SparseCopy copies src to dst preserving sparsity via SEEK_HOLE/SEEK_DATA; dst is truncated to src size and only data segments are written. -func SparseCopy(dst, src string, sync SyncMode) error { +func SparseCopy(dst, src string, mode SyncMode) error { return CopyWithCleanup(dst, src, func(srcFile, dstFile *os.File) error { fi, err := srcFile.Stat() if err != nil { @@ -40,7 +40,7 @@ func SparseCopy(dst, src string, sync SyncMode) error { } } - if !sync { + if !mode { return nil } if err := dstFile.Sync(); err != nil { diff --git a/utils/sparse_other.go b/utils/sparse_other.go index 23fd46e1..62c58e2f 100644 --- a/utils/sparse_other.go +++ b/utils/sparse_other.go @@ -8,13 +8,13 @@ import ( "os" ) -// SparseCopy copies src to dst. On non-Linux platforms, sparsity is not preserved. -func SparseCopy(dst, src string, sync SyncMode) error { +// SparseCopy copies src to dst without preserving sparsity. +func SparseCopy(dst, src string, mode SyncMode) error { return CopyWithCleanup(dst, src, func(srcFile, dstFile *os.File) error { if _, err := io.Copy(dstFile, srcFile); err != nil { return err } - if !sync { + if !mode { return nil } return dstFile.Sync() diff --git a/utils/stream.go b/utils/stream.go index 253225d9..60386c70 100644 --- a/utils/stream.go +++ b/utils/stream.go @@ -52,7 +52,7 @@ func TarDirStream(dir string, cleanup func()) io.ReadCloser { }) } -// PeekReader peeks up to n bytes + returns a reader that re-emits them then the rest; short read at EOF is not an error. +// PeekReader returns up to n head bytes and a reader that replays them before the rest; a short read at EOF is not an error. func PeekReader(r io.Reader, n int) ([]byte, io.Reader, error) { head := make([]byte, n) actual, err := io.ReadFull(r, head) diff --git a/utils/tar.go b/utils/tar.go index 82a7744a..87ee8525 100644 --- a/utils/tar.go +++ b/utils/tar.go @@ -57,7 +57,7 @@ func ListRegularFiles(dir string) ([]string, error) { return names, nil } -// ExtractTar extracts flat tar entries into dir. It never fsyncs — callers needing durability follow with SyncTree. +// ExtractTar extracts flat tar entries into dir without fsync; callers needing durability follow with SyncTree. func ExtractTar(dir string, r io.Reader) error { tr := tar.NewReader(r) for { @@ -119,7 +119,7 @@ func extractFileSparse(path string, r io.Reader, perm os.FileMode, realSize, dat } sum += seg.Length } - // A tar rewritten by a third-party tool keeps the map but not the data it describes + // A tar rewritten by a third-party tool keeps the map but not the data it describes. if sum != dataSize { return fmt.Errorf("sparse map describes %d data bytes, entry holds %d", sum, dataSize) } @@ -200,17 +200,17 @@ func isAllZero(b []byte) bool { return true } -// tarFileMaybeSparse writes file as COCOON.sparse PAX when it has holes; falls back to a regular entry on empty files, unsupported FS, no holes, or an oversized segment map. +// tarFileMaybeSparse writes a COCOON.sparse PAX entry, or a plain entry when sparse cannot help. func tarFileMaybeSparse(tw *tar.Writer, path, nameInTar string) error { f, err := os.Open(path) //nolint:gosec if err != nil { - return fmt.Errorf("open %s: %w", path, err) + return err } defer f.Close() //nolint:errcheck fi, err := f.Stat() if err != nil { - return fmt.Errorf("stat %s: %w", path, err) + return err } size := fi.Size() From 33125b67af3f9bc6a8dbedb11fdba1391edad12c Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:35:16 +0800 Subject: [PATCH 11/25] docs: match the code, release v0.7.6, and keep one copy of each fact Pin the quick start to v0.7.6; correct the Windows clone NIC, Android SELinux, data-disk size and Debian SSH statements; document -o json on mutating vm verbs, hibernate's --name, device --id and the pull_conns cap; replace duplicated sections with links to the canonical copy and drop sections no supported setup can hit. --- README.md | 6 ++--- docs/cli.md | 20 +++++++--------- docs/devices.md | 5 ++++ docs/firecracker.md | 3 +-- docs/index.md | 4 ++-- docs/install.md | 12 +++++----- docs/known-issues.md | 57 ++++---------------------------------------- docs/os-image.md | 4 +--- docs/vm.md | 10 ++++---- docs/windows.md | 20 +++------------- 10 files changed, 39 insertions(+), 102 deletions(-) diff --git a/README.md b/README.md index 11f6ad41..aa8531ee 100644 --- a/README.md +++ b/README.md @@ -20,12 +20,12 @@ Lightweight MicroVM engine with dual hypervisor backends: [Cloud Hypervisor](htt ## Quick Start ```bash -# Install the binary (Linux amd64; arm64: cocoon_0.7.5_Linux_arm64.tar.gz) -curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.5/cocoon_0.7.5_Linux_x86_64.tar.gz +# Install the binary (Linux amd64; arm64: cocoon_0.7.6_Linux_arm64.tar.gz) +curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.6/cocoon_0.7.6_Linux_x86_64.tar.gz tar -xzf cocoon.tar.gz && install -m 0755 cocoon /usr/local/bin/ # One-time environment setup (installs CH, firmware, CNI plugins) -curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.5/doctor/check.sh +curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.6/doctor/check.sh install -m 0755 cocoon-check /usr/local/bin/ && sudo cocoon-check --upgrade # Pull an image and run a VM diff --git a/docs/cli.md b/docs/cli.md index d0ef7e24..4bc1c073 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -2,6 +2,8 @@ Every command and flag. Concept guides: [VM lifecycle](vm.md), [networking](networking.md), [snapshots](snapshots.md), [devices](devices.md), [GC](gc.md). +Guides that document specific verbs and flags: `vm stop`/`vm rm` in [Shutdown Behavior](vm.md#shutdown-behavior), `vm net` in [NIC hot-resize](networking.md#nic-hot-resize-cloud-hypervisor-or-firecracker-with---pci), `vm fs`/`device`/`disk` in [Runtime Device Attach](devices.md), `gc` flags in [GC](gc.md), and `daemon` flags in [Daemon](daemon.md#running-it). + ## CLI Commands ``` @@ -82,7 +84,7 @@ Config-file / env-only keys (no CLI flag): | Key | Env Variable | Default | Description | | ------------ | ------------------- | ------- | ---------------------------------------------------------------------- | -| `pull_conns` | `COCOON_PULL_CONNS` | `8` | Concurrent HTTP Range connections per cloud-image download (`image pull`); raise for fat pipes, lower to be gentle on the registry | +| `pull_conns` | `COCOON_PULL_CONNS` | `8` | Concurrent HTTP Range connections per cloud-image download (`image pull`); raise for fat pipes, lower to be gentle on the registry; capped at 64 | | `cgroup_parent` | `COCOON_CGROUP_PARENT` | `cocoon.slice` | cgroup v2 slice under `/sys/fs/cgroup` holding the per-VM CPU scopes; GC removes any empty `vm-.scope` under it that none of its own VMs owns, so co-hosted installations need distinct slices, as with `net_scope`; see [CPU Isolation](vm.md#cpu-isolation-cgroup-v2) | | `cgroup_cpus` | `COCOON_CGROUP_CPUS` | empty (all cores) | Host cpu list fencing the whole VM population (e.g. `0-14` reserves core 15 for the host); kernel cpu-list syntax | | `net_scope` | `COCOON_NET_SCOPE` | empty (legacy names) | Two alphanumerics keying this installation's host network families — bridge TAPs `-`, CNI netns `-` — so co-hosted installations never GC each other's; see [Host device namespaces](networking.md#host-device-namespaces) | @@ -107,6 +109,8 @@ spellings — `level`, `filename`, `usejson`, `maxsize`, `maxage`, `maxbackups` (not `use_json` / `max_age` / `max_backups`). A dotted key maps to its env variable by replacing `.` with `_`, hence `COCOON_LOG_MAXSIZE` for `log.maxsize`. +Mutating `vm` verbs (`create`, `run`, `clone`, `start`, `stop`, `rm`, `restore`, `reconcile-stale-create`, `net`, `fs`/`device`/`disk` `attach`/`detach`, `migrate capture`/`receive`) take `-o, --output json` for machine-readable output; `list` and `status` use `-o, --format`; `snapshot export -o` is the output file path. + ## VM Flags Applies to `cocoon vm create`, `cocoon vm run`, and `cocoon vm debug`: @@ -161,13 +165,7 @@ Applies to `cocoon vm clone`: | `--from-dir` | empty | Clone from a snapshot directory (must contain `snapshot.json`); mutually exclusive with positional `SNAPSHOT` | | `--data-disk` | empty (repeatable) | Create a fresh data disk for the clone and hot-add it after restore: `size=20G[,name=...][,fstype=ext4|none]` (CH, or FC `--pci` snapshots; names must not collide with disks inherited from the snapshot) | -CPU, memory, and storage all inherit from the snapshot — both hypervisors -restore the guest from the snapshot's binary device state, so those values -are fixed at snapshot time. NIC count inherits by default but `--nics N` -overrides it: Cloud Hypervisor hot-swaps the snapshot's NICs for a fresh set -right after restore, a Firecracker `--pci` clone restores the snapshot's NICs -and hot-plugs the delta afterwards; a Firecracker MMIO snapshot rejects the override (`--nics override on clone needs a --pci snapshot on Firecracker`). Use -`cocoon vm run` to create a fresh VM with different CPU/memory/storage. +CPU, memory, and storage are fixed at snapshot time; `--nics` overrides the NIC count. Backend rules in [clone constraints](snapshots.md#clone-constraints); use `cocoon vm run` for a different CPU/memory/storage shape. **Network backend** is decided per clone (the snapshot does not persist a bridge device). `--bridge` and `--network` are mutually exclusive: @@ -186,7 +184,7 @@ Applies to `cocoon vm restore`: | Flag | Default | Description | | ------------- | ------- | ------------------------------------------------------------------------------------------------------ | -| `--restore-mode` | `mmap` for plain private-anon snapshots, else `copy` | Memory restore mode: `copy`, `ondemand` (UFFD) or `mmap` (CoW map); CH only, `mmap` needs Cloud Hypervisor v54 or newer (`cocoon-check --upgrade` installs the cocoonstack fork build) and `ondemand` v52 or newer: v52–v53 reject a mode they lack, and builds before v52 ignore the field and restore by copy; hugepages/shared snapshots degrade `mmap` to `copy` with a warning | +| `--restore-mode` | `mmap` for plain private-anon snapshots, else `copy` | Memory restore mode; same values and version floors as [clone `--restore-mode`](#clone-flags) | | `--from-dir` | empty | Restore from a snapshot directory (must contain `snapshot.json`); mutually exclusive with positional `SNAPSHOT` | | `--force` | `false` | Skip the snapshot-belongs-to-VM check. Valid only with `--from-dir`; used on its own it is rejected with `--force only applies with --from-dir` | @@ -200,7 +198,7 @@ See [restore constraints](snapshots.md#restore-constraints). ### Snapshot Flags -Applies to `cocoon snapshot save`: +Applies to `cocoon snapshot save` and `cocoon vm hibernate`: | Flag | Default | Description | | --------------- | ------- | -------------------- | @@ -232,7 +230,7 @@ When FILE is omitted, data is read from stdin. This enables piping: `cocoon snap ### Direct Clone / Restore From a Directory -`vm clone --from-dir DIR` and `vm restore --from-dir DIR` accept a directory produced by `snapshot export --to-dir`. Do not unpack an `export -o file.tar` with a third-party tar and pass the result: cocoon stores sparse disks with private pax records that only its own extractor understands, so another tar rebuilds a short, content-shifted disk. To go through a tar, use `snapshot import`. The snapshot does not need to be in the local snapshot DB: +`vm clone --from-dir DIR` and `vm restore --from-dir DIR` accept a directory produced by `snapshot export --to-dir`. Do not unpack an `export -o file.tar` with a third-party tar and pass the result; go through `snapshot import` instead ([known issue](known-issues.md#--from-dir-and-snapshot-import-reject-a-snapshot-rebuilt-by-a-third-party-tar)). The snapshot does not need to be in the local snapshot DB: ```bash # Build a portable snapshot dir, ship it, clone from it: diff --git a/docs/devices.md b/docs/devices.md index 33e1876d..b226fc57 100644 --- a/docs/devices.md +++ b/docs/devices.md @@ -109,6 +109,11 @@ cocoon vm device attach my-vm --pci 01:00.0 --id mygpu cocoon vm device detach my-vm --id mygpu ``` +| Flag | Applies to | Description | +| ---- | ---------- | ----------- | +| `--pci` | `attach` (required) | Short BDF (`01:00.0`), full BDF (`0000:01:00.0`) or a sysfs path under `/sys/bus/pci/devices/` | +| `--id` | `attach` (optional), `detach` (required) | Device id; Cloud Hypervisor generates one when `attach` omits it, and `detach` takes the id `attach` returned. Must not start with `cocoon-` | + `cocoon vm inspect VM` includes an `attached_devices` field for running VMs that surfaces every attached vhost-user-fs share, VFIO device, and hot-attached disk, read live from CH `vm.info`. The field is omitted for stopped VMs and whenever the live attach set is empty. ## Firecracker (`--pci`) diff --git a/docs/firecracker.md b/docs/firecracker.md index e50fc7e1..274c3373 100644 --- a/docs/firecracker.md +++ b/docs/firecracker.md @@ -48,8 +48,7 @@ cocoon vm clone my-snap --name clone-vm - **Stop waits the full timeout**: FC guests without i8042 never answer CtrlAltDel, so `vm stop` waits out `stop_timeout_seconds` before SIGKILL - **Clone MTU must match**: a clone requires the target network's MTU to equal the snapshot's - **Raw disks only**: Firecracker uses raw virtio-blk without serial support; disks are referenced by device path (`/dev/vdX`) -- **Single-queue networking**: `NetworkConfig.NumQueues` is ignored -- **Snapshot portability requires same directory layout**: FC snapshots store absolute paths in the vmstate binary; cocoon bind-mounts the snapshot's drives into place inside a private mount namespace and re-anchors them after resume, so a differing `run_dir` fails outright (`unusable source ... is outside the managed run root`) rather than corrupting the clone, and the same OCI image must be pulled +- **Snapshot portability requires same directory layout**: FC snapshots store absolute host paths, so cross-host import needs the same `root_dir`/`run_dir` and OCI image; see [known issues](known-issues.md#firecracker-snapshot-portability) - **Console via PTY relay**: a background relay process bridges FC's serial (stdin/stdout) to `console.sock` ### OCI Image Compatibility diff --git a/docs/index.md b/docs/index.md index 0c93389c..58918597 100644 --- a/docs/index.md +++ b/docs/index.md @@ -58,14 +58,14 @@ cocoon CLI ──► images: OCI (EROFS layers, direct boot) | cloudimg (qcow2, - **Copy-on-write clone restore** — Cloud Hypervisor clones of plain private-anon snapshots default to `mmap` memory restore: no eager copy, page cache shared across sibling clones; hugepages/shared snapshots fall back to eager copy with a warning - **Hugepages** — opt-in via `vm create --hugepages` (Cloud Hypervisor only); backs VM memory with hugetlbfs at the cost of the mmap restore fast path for that VM's snapshots (never supported on Firecracker, whose snapshots cannot restore from hugetlbfs) - **Mergeable memory** — opt-in `--mergeable` (Cloud Hypervisor only) madvises guest memory `MADV_MERGEABLE` so host KSM can dedup identical pages across VMs; excludes `--hugepages`/`--shared-memory` -- **Memory balloon** — 25% of memory returned via virtio-balloon (deflate-on-OOM, free-page reporting) when memory >= 256 MiB; `--no-balloon` opts out at create and the setting follows the snapshot +- **Memory balloon** — 25% of memory returned via virtio-balloon (deflate-on-OOM, free-page reporting); `--no-balloon` opts out at create and the setting follows the snapshot - **Graceful shutdown** — ACPI power-button for every Cloud Hypervisor VM with configurable timeout, fallback to SIGTERM → SIGKILL - **Interactive console** — `cocoon vm console` with bidirectional PTY relay, an SSH-style escape sequence (`^]` then `.` disconnects; `--escape-char` changes the first key), SIGWINCH propagation - **Snapshot & clone** — `cocoon snapshot save` captures a running VM's full state (memory, disks, config); `cocoon vm clone` restores it as a new VM with fresh network and identity; guest CPU, memory and storage inherit verbatim from the snapshot and so does the NIC count unless `--nics` overrides it, while host-side cgroup CPU policy comes from clone flags (see [CPU Isolation](vm.md#cpu-isolation-cgroup-v2)) - **Snapshot export & import** — `cocoon snapshot export` packages a snapshot into a portable `.tar` archive (`.tar.gz` with `--gzip`, sparse-aware pax headers); `cocoon snapshot import` restores it on another host or cluster; supports piping via stdout/stdin for direct host-to-host transfer; `--to-dir` writes a directory form (with `snapshot.json` envelope) for NFS / rsync-friendly handoff - **Clone / restore from a directory** — `cocoon vm clone --from-dir DIR` and `cocoon vm restore --from-dir DIR` consume any directory containing a `snapshot.json` envelope without first registering the snapshot in the local DB; the dir is treated as read-only so multi-VM golden-image use cases work without copying - **Live status monitoring** — `cocoon vm status` watches VM state changes in real time via fsnotify, with refresh mode (top-like) and event-stream mode (append-only, for scripting and vk-cocoon integration) -- **Docker-like CLI** — `create`, `run`, `start`, `stop`, `list`, `inspect`, `console`, `rm`, `debug`, `clone`, `status` +- **Docker-like CLI** — `create`, `run`, `start`, `stop`, `list`, `inspect`, `console`, `exec`, `rm`, `clone`, `status` - **Structured logging** — configurable log level (`--log-level`); logs go to stderr until `log.filename` is set, which is also what enables rotation via `log.maxsize` / `log.maxage` / `log.maxbackups` (500 MB / 28 days / 3 files by default) - **Debug command** — `cocoon vm debug` generates a copy-pasteable `cloud-hypervisor` command for manual debugging - **Firecracker backend** — `--fc` flag selects Firecracker for OCI images: ~125ms boot, <5 MiB overhead, minimal attack surface (no UEFI, no qcow2, no Windows) diff --git a/docs/install.md b/docs/install.md index 1f9fe34c..e5d2a1cd 100644 --- a/docs/install.md +++ b/docs/install.md @@ -6,7 +6,7 @@ Requirements, install paths, the doctor script, and a first VM. - Linux with KVM (x86_64 or aarch64) - Root access (sudo) -- [Cloud Hypervisor](https://github.com/cloud-hypervisor/cloud-hypervisor) v54 or newer. `cocoon-check --upgrade` installs the [cocoonstack fork](https://github.com/cocoonstack/cloud-hypervisor/tree/dev) `dev` release build (upstream main plus diff snapshots and guest shadow-stack save/restore, built with the `fw_cfg` feature that aarch64 OCI boot needs), verified against the release checksums; v53.0 and older have no CopyOnWrite memory restore, so the default clone/restore mode (`mmap`) is rejected and `cocoon-check` reports it +- [Cloud Hypervisor](https://github.com/cloud-hypervisor/cloud-hypervisor) v54 or newer. `cocoon-check --upgrade` installs the [cocoonstack fork](https://github.com/cocoonstack/cloud-hypervisor/tree/dev) `dev` release build (upstream main plus diff snapshots and guest shadow-stack save/restore, built with the `fw_cfg` feature that aarch64 OCI boot needs), verified against the release checksums; v52–v53 reject the default clone/restore mode (`mmap`) and builds before v52 silently restore by copy; `cocoon-check` reports either - [Firecracker](https://github.com/firecracker-microvm/firecracker) v1.16.1 or newer (optional, for the `--fc` backend). `cocoon-check --upgrade` installs the [cocoonstack fork](https://github.com/cocoonstack/firecracker/tree/dev) `dev` release build (upstream main plus release CI), verified against the release checksums — the build the `--pci` hot-plug and NIC MTU paths are validated on. `vm clone` needs >= v1.16 for the vsock override, and v1.16.0 permanently breaks guest vsock after any restore, so v1.16.1 is the effective floor (see [known issues](known-issues.md)) - `qemu-img` (from qemu-utils, for cloud images) - `mkfs.erofs` from erofs-utils **>= 1.8** (for OCI images; 1.7.x tar mode @@ -24,10 +24,10 @@ Download pre-built binaries from [GitHub Releases](https://github.com/cocoonstac ```bash # Linux amd64 -curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.5/cocoon_0.7.5_Linux_x86_64.tar.gz +curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.6/cocoon_0.7.6_Linux_x86_64.tar.gz # Linux arm64 -curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.5/cocoon_0.7.5_Linux_arm64.tar.gz +curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.6/cocoon_0.7.6_Linux_arm64.tar.gz tar -xzf cocoon.tar.gz install -m 0755 cocoon /usr/local/bin/ @@ -54,7 +54,7 @@ Cocoon ships a diagnostic script that checks your environment and can auto-insta ```bash # Get script -curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.5/doctor/check.sh +curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.6/doctor/check.sh install -m 0755 cocoon-check /usr/local/bin/ # Check only — reports PASS/FAIL for each requirement @@ -71,12 +71,12 @@ cocoon-check --upgrade ``` The `--upgrade` flag downloads and installs: -- Cloud Hypervisor from the cocoonstack fork `dev` release (checksum-verified) and upstream ch-remote (static binaries) +- Cloud Hypervisor from the cocoonstack fork `dev` release (checksum-verified) - Firecracker from the cocoonstack fork `dev` release (checksum-verified) - CLOUDHV.fd firmware: the cocoonstack firmware fork `dev` build on x86_64 (checksum-verified), EDK2 `CLOUDHV_EFI.fd` from cloud-hypervisor/edk2 on aarch64 (verified against the digest pinned in the script) - CNI plugins (bridge, host-local, loopback, etc.) -Release tags and versions are overridable through `CH_REF`, `CH_REMOTE_VERSION`, `FC_REF`, `FW_REF`, `EDK2_REF` (with its `EDK2_SHA256`) and `CNI_VERSION` (see `cocoon-check --help`). +Release tags and versions are overridable through `CH_REF`, `FC_REF`, `FW_REF`, `EDK2_REF` (with its `EDK2_SHA256`) and `CNI_VERSION` (see `cocoon-check --help`). ## Quick Start diff --git a/docs/known-issues.md b/docs/known-issues.md index 61af075a..31f5b4e8 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -13,10 +13,6 @@ Limitations, workarounds, and upstream tracking. Fixed entries are kept for oper **Mitigation**: run the post-clone guest setup commands printed by `cocoon vm clone` (see [Post-Clone Guest Setup](snapshots.md#post-clone-guest-setup)). For cloudimg VMs they re-run `cloud-init`; for OCI VMs they replace the generated `/etc/systemd/network/10-*.network` files with per-MAC units for the new IP and restart `systemd-networkd`; on Firecracker they set the new MAC first. The clone answers on its allocated IP a few seconds after they run. -## Clone and restore resources are fixed at snapshot time - -`cocoon vm clone` inherits CPU, memory, and storage from the snapshot — none of these can be grown at clone time on either backend. NIC count inherits by default; Cloud Hypervisor clones may override it with `--nics N` (cocoon hot-swaps the snapshot's NICs for a fresh set after restore). MMIO Firecracker clones must keep the snapshot's NIC topology — FC's `network_overrides` only retargets existing interfaces, so `--nics` is rejected there (a `--pci` snapshot restores its NICs and hot-plugs the delta). On every Firecracker clone, MMIO and `--pci` alike, the target network's MTU must equal the snapshot's because the guest keeps the advertised MTU. `cocoon vm restore` is more restrictive: CPU, memory, and storage come from the snapshot (the persisted record is realigned to match), and NIC count must already match the target VM (mismatches are rejected, since restore reuses the existing network namespace). Use `cocoon vm run` to create a fresh VM with different sizing. - ## OCI VM multi-NIC kernel IP limitation OCI VMs use the kernel `ip=` boot parameter for network configuration. While multiple `ip=` parameters can be specified, the Linux kernel only reliably configures **one interface** via this mechanism — subsequent `ip=` parameters may be silently ignored or produce inconsistent results depending on kernel version. @@ -48,7 +44,7 @@ This is by design: clone restores the VM's exact state including all account set ## `vm exec` requires vsock — legacy VMs excluded -`cocoon vm exec` dials the cocoon-agent inside the guest over hybrid vsock. Two caveats: +`cocoon vm exec` dials the cocoon-agent inside the guest over hybrid vsock. Three caveats: - **Legacy VMs (created before vsock support landed)** have no vsock UDS bound. `vm inspect` omits `vsock_socket` and `vm exec` returns `vsock not configured for this VM (recreate the VM to enable agent exec)`. Recreate the VM to gain exec capability. - **FC clone vsock requires FC ≥ v1.16** (the `vsock_override` field on `PUT /snapshot/load` was merged post-v1.15). Older FC rejects the field with `unknown field vsock_override`. Cocoon omits the field on same-VM restore, so FC < v1.16 still runs and restores fine — only `vm clone` needs the field. @@ -137,34 +133,6 @@ The rules an earlier `ipMasq` network left carry its name in their comment, for iptables-save -t nat | grep -F 'name: \"cocoon\"' ``` -## Windows VM requires Cloud Hypervisor v50.2 - -**Status: FIXED** in our fork and upstream. - -Cloud Hypervisor v51.x had a regression ([#7849](https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7849)) that caused Windows to BSOD (`DRIVER_IRQL_NOT_LESS_OR_EQUAL` in `viostor.sys`) when DISCARD/WRITE_ZEROES features were advertised with default-zero config values, violating virtio spec v1.2. - -**Fix**: the DISCARD fix is included in our Cloud Hypervisor fork ([cocoonstack/cloud-hypervisor `dev` branch](https://github.com/cocoonstack/cloud-hypervisor/tree/dev)). Upstream has also merged it ([PR #7936](https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7936)). Cloud Hypervisor **v52** and newer work correctly with Windows VMs. - -**Previous recommendation** (no longer needed): use Cloud Hypervisor v50.2 for Windows VMs. - -## Windows VM requires virtio-win 0.1.240 - -**Status: FIXED** upstream since Cloud Hypervisor v52. - -virtio-win 0.1.271+ network drivers were incompatible with Cloud Hypervisor due to incomplete virtio-net control queue implementation ([#7925](https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7925)). CH only handled `CTRL_MQ` and `CTRL_GUEST_OFFLOADS`; all other commands (`CTRL_RX`, `CTRL_MAC`, `CTRL_VLAN`, `CTRL_ANNOUNCE`) returned `VIRTIO_NET_ERR`. - -| Version | Behavior on VIRTIO_NET_ERR | -|----------|----------------------------------------------------------| -| 0.1.240 | Tolerates error, continues working | -| 0.1.271 | May silently fail, NIC unusable | -| 0.1.285+ | Fail-fast: NdisMRemoveMiniport(), Problem Code 43 | - -0.1.285 introduced commit `50e7db9` ("indicate driver error on unexpected CX behavior") with zero-tolerance on control queue errors. Root cause was a CH bug — the correct fix is to return `VIRTIO_NET_OK` for unsupported commands and to report the correct `used_len`. - -**Fix**: the ctrl_queue command tolerance (from [@liuw](https://github.com/liuw)) plus the `used_len` fix merged upstream in [PR #7953](https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7953) and ship in Cloud Hypervisor v52 and newer, including the [cocoonstack fork](https://github.com/cocoonstack/cloud-hypervisor/tree/dev) build. virtio-win **0.1.285** now works. - -**Previous recommendation** (no longer needed): use virtio-win 0.1.240 for Windows VMs on Cloud Hypervisor. - ## Windows VM does not respond to ACPI power-button **Status: FIXED** in our firmware fork. @@ -231,14 +199,7 @@ Firecracker does not support virtio-blk serial numbers. Cocoon's OCI init script ## Cloudimg post-restart snapshot includes orphan `cidata.img` -When a cloudimg VM is snapshotted **after** it has been stopped and started at least once, the resulting snapshot tar still contains a `cidata.img` file even though the snapshot's `cocoon.json` sidecar and CH `config.json` no longer reference cidata. This is because `hypervisor/cloudhypervisor/snapshot.go` copies `cidata.img` whenever the source VM's `runDir` has it (cloudimg + non-Windows), regardless of whether `activeDisks(rec)` would have re-attached it. - -**Consequence**: ~1 MB of dead weight inside the tar. No correctness impact: - -- Clone from such a snapshot regenerates a fresh `cidata.img` via `ensureCloneCidata` and overwrites the orphan with the clone's identity / network config. -- Restore leaves the orphan in place: `cleanSnapshotFiles` removes only the memory ranges, `config.json`, `state.json`, `cocoon.json` and `data-*.raw`, so the staged `cidata.img` is merged back over the VM's own copy — harmless, since restore targets the same VM. - -This predates the data-disk feature; the cocoon.json sidecar just makes the asymmetry visible. We are not patching the copy logic because the orphan is harmless and the cidata disk is small (sparse-copied, ~1 MB). +A cloudimg VM snapshotted after a stop/start carries an unreferenced ~1 MB sparse `cidata.img`; clone overwrites it and restore keeps it. Harmless. ## Firecracker clone guest MAC address @@ -254,10 +215,6 @@ On CNI plugins with strict per-veth MAC enforcement (Cilium eBPF, Calico eBPF), `cocoon vm fs attach` only works on CH VMs that were created with `--shared-memory`. CH's `memory shared=on` is fixed at VM creation: backend processes (e.g. virtiofsd) need to mmap guest memory via the negotiated memfd, and the memory model cannot be flipped on a running VM. If `--shared-memory` was omitted at create time, the only path is to recreate the VM. Cocoon's preflight reads `vm.info` and surfaces a clear error rather than letting CH return a vague rejection. -## Snapshotting a VM with attached vhost-user-fs / VFIO / hot-attached disks is refused - -Cloud Hypervisor captures a snapshot of a VM that holds a vhost-user-fs share or a VFIO PCI passthrough device, but the result cannot be restored: it names a backend socket that is gone (restore fails after 60s) or hangs against a fresh one. Cocoon refuses the capture itself — the pre-flight reads the live `vm.info` before pausing the guest and fails with `hot-attached vhost-user-fs "": detach before snapshot or hibernate` (or `hot-attached device "": ...`, `hot-attached disk "": ...`) before anything is captured or recorded. `cocoon vm fs detach` / `cocoon vm device detach` / `cocoon vm disk detach` first to clear runtime devices, then snapshot. - ## A killed capture leaves the guest paused `snapshot save` and `vm hibernate` pause the guest for the capture. A canceled or timed-out capture attempts to resume the guest with a separate 10 minute timeout. If the cocoon process dies or that resume fails, the guest can remain paused: the VM record reads `running`, the guest is frozen, and `vm exec` hangs. SIGKILL does this. SIGPIPE does too: when the process that reads cocoon's stderr exits, cocoon dies on its next log line, and for a hibernate that line comes after the snapshot persists and before the VMM stops. @@ -276,10 +233,6 @@ On v0.6.7 and older every `snapshot save` leaves that zero-byte file behind and `cocoon vm clone --from-dir`, `cocoon vm restore --from-dir` and `cocoon snapshot import` compare `cow.raw`'s on-disk size with the size recorded in `snapshot.json`. cocoon's export tar stores sparse disks with private pax records that only its own extractor understands; a generic `tar -x` drops them and rebuilds a short, shifted disk that would restore into silent corruption. The snapshot is refused with `cow.raw in is N bytes but the envelope records M`. Use `snapshot export --to-dir` for the directory form, or `snapshot import` on the tar exactly as cocoon wrote it. -## Runtime attached devices do not survive VM stop / clone / restore - -Attaches via `cocoon vm fs attach`, `cocoon vm device attach` and `cocoon vm disk attach` are runtime-only — they live in the VMM process state and are never written into the VM record, sidecar, or snapshot. After `vm stop`, `vm clone`, or `vm restore`, the user must re-run the attach commands. `vm inspect` reflects the live VMM state for running VMs and omits `attached_devices` for stopped VMs. This is by design: cocoon does not own the backend (virtiofsd, the vfio-pci binding, an external disk file) and cannot guarantee the resource still exists across host events. - ## virtiofsd is a single-shot daemon Upstream virtiofsd serves exactly one vhost-user client and exits when that client disconnects. Consequence: after `cocoon vm fs detach`, the daemon is gone — a follow-up `cocoon vm fs attach` against the same socket path will hang or time out until a fresh `virtiofsd` instance is launched. The same applies after `cocoon vm stop` (CH closes the socket on shutdown). Scripts that cycle attach/detach should respawn virtiofsd between calls. This is a virtiofsd behavior, not a cocoon limitation. @@ -311,10 +264,8 @@ Firecracker VMs created with `--pci` resize through the same command. After remo ## NIC hot-remove during clone hot-swap cannot wait for guest B0EJ -Plain `cocoon vm net --nics N` polls CH's `device_tree` after `vm.remove-device` until the guest ACKs B0EJ via ACPI/SCI (typically < 1 s on Linux, 10–20 s on Windows). The clone path's `hotSwapNets` runs while the VM is paused, so the guest cannot process SCI — eject stays pending until resume. The clone path therefore returns without waiting and adds the fresh NICs against half-removed device-tree state. This is the long-standing CH limitation cocoon was designed around; the new wait only applies to the running-VM resize. +Plain `cocoon vm net --nics N` polls CH's `device_tree` after `vm.remove-device` until the guest ACKs B0EJ via ACPI/SCI (typically < 1 s on Linux, 10–20 s on Windows). The clone path's `hotSwapNets` runs while the VM is paused, so the guest cannot process SCI — eject stays pending until resume. The clone path therefore returns without waiting and adds the fresh NICs against half-removed device-tree state. This is the long-standing CH limitation cocoon was designed around; the wait only applies to the running-VM resize. ## Android cocoon-agent service may be blocked by SELinux -`os-image/android/{14.0,15.0,15.0-gms}` install the cocoon-agent binary at `/system/bin/cocoon-agent` and register it via `/system/etc/init/cocoon-agent.rc`. Android's SELinux policies don't ship with a domain for cocoon-agent, so the service may run in `init`'s domain or be denied outright depending on the redroid build. - -If `cocoon vm exec` against an Android VM returns `dial agent: ...`, check `logcat | grep -i avc` inside the guest. The fix is build-time — adjust the Android sepolicy to grant the new binary network/socket permissions — and is out of scope for the Dockerfile. +`os-image/android/{14.0,15.0,15.0-gms}` install the cocoon-agent binary at `/system/bin/cocoon-agent` and register it via `/system/etc/init/cocoon-agent.rc`. The official images boot with `androidboot.selinux=permissive` (`os-image/android/init-wrapper.sh`), so the service runs and SELinux denials are only logged. On an enforcing build the policy has no domain for cocoon-agent and may deny it outright: if `cocoon vm exec` returns `dial agent: ...`, check `logcat | grep -i avc` inside the guest and grant the binary network/socket permissions in the Android sepolicy at build time. diff --git a/docs/os-image.md b/docs/os-image.md index e20b6fc6..e05a88a3 100644 --- a/docs/os-image.md +++ b/docs/os-image.md @@ -30,7 +30,7 @@ checksum-verified agent. ### Android (Redroid) -The three bare `android/` tags run Android via [Redroid](https://github.com/remote-android/redroid-doc) directly as PID 1 in the VM — no Ubuntu/systemd layer — and are `linux/amd64` only; the 16.0-gms-h264 image is Ubuntu-based, builds under `ubuntu/` on the default multi-arch matrix, and behaves like the Ubuntu images below. +The three bare `android/` tags run Android via [Redroid](https://github.com/remote-android/redroid-doc) directly as PID 1 in the VM — no Ubuntu/systemd layer — and are `linux/amd64` only; the 16.0-gms-h264 image is Ubuntu-based, builds under `ubuntu/` on the default multi-arch matrix, and behaves like the Ubuntu images above. | Image | Tag | IMAGE_NAME | |-------|-----|------------| @@ -41,8 +41,6 @@ The three bare `android/` tags run Android via [Redroid](https://github.com/remo Access via `adb connect :5555` or `scrcpy -s :5555 --no-audio`. -The `16.0-gms-h264` image is Ubuntu+Redroid-based (built from `os-image/ubuntu/22.04-redroid-16.0-gms-h264/`), unlike the bare `os-image/android/` family above. - The `15.0-gms` variant adds Google Play services (GmsCore, Play Store, GSF) and `libndk_translation` so arm64-only APKs run on the x86_64 host. It runs ordinary arm64 apps (verified with Meituan). It does **not** run apps whose diff --git a/docs/vm.md b/docs/vm.md index 0d0c46a2..124c4ffe 100644 --- a/docs/vm.md +++ b/docs/vm.md @@ -104,7 +104,7 @@ With `cgroup_cpus=0-14`, the reserved core 15 has no VM competition and acts as Firecracker attaches virtio devices over MMIO by default (and adds `pci=off` to the guest command line itself). `--pci` on `vm run`/`vm create` starts Firecracker with `--enable-pci`, so the guest enumerates virtio-pci devices instead; this is the prerequisite for Firecracker device hot-plug (Developer Preview upstream). The transport is fixed for the VM's lifetime and travels with its snapshots: clones and restores relaunch on the transport the snapshot was taken with, and a snapshot cannot move between transports. The guest kernel needs `CONFIG_PCI`, `CONFIG_VIRTIO_PCI` and `CONFIG_ACPI`; distro kernels have both transports, while the sandbox guest kernel is built without `CONFIG_VIRTIO_MMIO`, so sandbox images can never use the MMIO transport (booting them on Firecracker at all is not validated yet). Cloud Hypervisor is always virtio-pci, so `--pci` is rejected there. -With `--pci`, `vm net`, `vm disk attach`/`detach`, clone `--nics` and clone `--data-disk` work as on Cloud Hypervisor, with one difference: a Firecracker build that plugs devices onto the root bus does not notify the guest, so cocoon prints the guest-side step after each change (a build that places devices behind PCIe root ports, which cocoon requests for every `--pci` VM, needs no step; `echo 1 > /sys/bus/pci/rescan` after a plug, `echo 1 > /sys/.../device/../remove` for the stale node after an unplug; `--output json` returns them as `hints`). Unmount or down a device inside the guest before detaching it. Hot-attached disks show up as `/dev/vdX` (Firecracker has no virtio-blk serial) and, as on Cloud Hypervisor, block snapshot and hibernate until detached. +With `--pci`, `vm net`, `vm disk attach`/`detach`, clone `--nics` and clone `--data-disk` work as on Cloud Hypervisor; on a Firecracker build that plugs devices onto the root bus cocoon prints the guest-side rescan/remove step after each change (see [Firecracker `--pci`](devices.md#firecracker---pci)). Unmount or down a device inside the guest before detaching it. Hot-attached disks show up as `/dev/vdX` (Firecracker has no virtio-blk serial) and, as on Cloud Hypervisor, block snapshot and hibernate until detached. ## Cloud-init & First Boot @@ -117,7 +117,7 @@ Cloudimg VMs receive a NoCloud cidata disk (FAT12 with `CIDATA` volume label) co The cidata disk is **automatically excluded on subsequent boots** — after the first successful start, the VM record is marked as `first_booted` and the cidata disk is no longer attached, preventing cloud-init from re-running. -Note: `--user`/`--password` only apply to **cloudimg** VMs (cloud-init). OCI VM images bake credentials at build time — every official `os-image/ubuntu/*` image ships `openssh-server` enabled with `PermitRootLogin yes` and the default `root:cocoon` credentials. Host-to-guest control plane operations (kubectl exec, kubectl logs) prefer cocoon-agent over vsock; SSH stays available as the human-on-keyboard path. +Note: `--user`/`--password` only apply to **cloudimg** VMs (cloud-init). OCI VM images bake credentials at build time — every official Ubuntu and Debian image ships `openssh-server` enabled with `PermitRootLogin yes` and the default `root:cocoon` credentials. Host-to-guest control plane operations (kubectl exec, kubectl logs) prefer cocoon-agent over vsock; SSH stays available as the human-on-keyboard path. ## Data Disks @@ -140,9 +140,9 @@ cocoon vm run --data-disk size=20G,name=raw,fstype=none | Key | Default | Notes | | ---------- | ---------------- | ----- | -| `size` | required | Minimum 16 MiB; goes through `units.RAMInBytes` so `512M`, `2G`, etc. all work | +| `size` | required | Minimum 16 MiB; same spellings as `--memory` (`20G`, `20GiB`, `20Gi`, all binary) | | `name` | `dataN` (auto) | 1-20 chars, `[a-z][a-z0-9_-]{0,19}`, `cocoon-` prefix reserved; auto-numbered names skip any explicit one already taken | -| `fstype` | `ext4` | `ext4` (cocoon mkfs's it) or `none` (guest must format); xfs is not supported in Phase 1 | +| `fstype` | `ext4` | `ext4` (cocoon mkfs's it) or `none` (guest must format); xfs is not supported | | `mount` | `/mnt/` | Cloudimg+CH only — emitted as a cloud-init `mounts:` row using `/dev/disk/by-id/virtio-`. Pass `mount=` (empty) to skip auto-mount even when fstype is ext4. `fstype=none` requires `mount=`/empty | | `directio` | `auto` | `on` forces `direct=on`, `off` forces page-cache, `auto` inherits VM-level `--no-direct-io`. CH only — FC has no DirectIO knob and logs a warn | @@ -157,7 +157,7 @@ cocoon vm run --data-disk size=20G,name=raw,fstype=none ### Snapshot/Clone/Restore -Phase 1 inherits data disks 1:1: snapshot reflinks each `data-.raw` into the snapshot tar, clone re-creates them under the new VM's runDir (and regenerates cidata so cloud-init re-mounts on the new identity), and restore rolls all data disks back to the snapshot timepoint along with the rootfs and memory state. Cloud Hypervisor clones can additionally CREATE fresh data disks at clone time via `--data-disk` (hot-added after restore — the snapshot's device tree itself cannot grow); clone-created disks are hot-added after cidata is regenerated, so that clone does not auto-mount them — `mount=` has no effect on it; mount them inside the guest. A clone of that clone inherits the disk like any other: its regenerated cidata lists the mount, which applies when the [post-clone guest setup](snapshots.md#post-clone-guest-setup) commands run. Removing inherited disks at clone time is not supported, and Firecracker clones accept `--data-disk` only from a `--pci` snapshot (MMIO cannot hot-plug). +Data disks are inherited 1:1: snapshot reflinks each `data-.raw` into the snapshot tar, clone re-creates them under the new VM's runDir (and regenerates cidata so cloud-init re-mounts on the new identity), and restore rolls all data disks back to the snapshot timepoint along with the rootfs and memory state. Cloud Hypervisor clones can additionally CREATE fresh data disks at clone time via `--data-disk` (hot-added after restore — the snapshot's device tree itself cannot grow); clone-created disks are hot-added after cidata is regenerated, so that clone does not auto-mount them — `mount=` has no effect on it; mount them inside the guest. A clone of that clone inherits the disk like any other: its regenerated cidata lists the mount, which applies when the [post-clone guest setup](snapshots.md#post-clone-guest-setup) commands run. Removing inherited disks at clone time is not supported, and Firecracker clones accept `--data-disk` only from a `--pci` snapshot (MMIO cannot hot-plug). Restore preflight verifies sidecar integrity, file presence (vmstate, memory, COW, every `data-*.raw` — presence, not size; `--from-dir` and `snapshot import` separately check the COW size against the envelope), per-index Path/RO agreement between the sidecar and CH config.json, Role/Serial agreement between the sidecar and the VM record, and on Firecracker that every COW and data disk is recorded at this VM's own path, all **before** killing the running VM, so a malformed or imported snapshot fails fast and leaves the live VM untouched. diff --git a/docs/windows.md b/docs/windows.md index cf8f6bef..77a5c1bf 100644 --- a/docs/windows.md +++ b/docs/windows.md @@ -25,25 +25,11 @@ The `--windows` flag: ### Image -Pre-built images and build automation are maintained in [cocoonstack/windows](https://github.com/cocoonstack/windows). - -The Windows image is published as an **OCI artifact** (split qcow2 parts pushed via ORAS), not a runnable OCI container image — use `oras pull` (not `cocoon image pull` or `docker pull`). - -```bash -# 1. Pull split parts via oras (https://oras.land) -oras pull ghcr.io/cocoonstack/windows/win11:25h2 - -# 2. Verify the split parts -sha256sum -c SHA256SUMS - -# 3. Import the parts into Cocoon in order -cocoon image import win11-25h2 windows-11-25h2.qcow2.*.qcow2.part -``` - +Pre-built images are published as split OCI artifacts and imported with `oras pull` + `cocoon image import`; see [OS images](os-image.md#windows) for the pull steps and [cocoonstack/windows](https://github.com/cocoonstack/windows) for build automation. ### Post-Clone Networking -- **DHCP networks**: no action needed, Windows DHCP client auto-configures -- **Static IP**: configure via SAC serial console (`cocoon vm console`) +- **DHCP networks**: the clone comes up on hot-swapped NICs with new MACs; if a NIC is left without IPv4, run the `Get-PnpDevice` rebind that `cocoon vm clone` prints +- **Static IP**: after the rebind, run the printed `netsh interface ipv4 set address ...` inside the guest (SAC serial console via `cocoon vm console`) For more details, see the [Cloud Hypervisor Windows documentation](https://github.com/cloud-hypervisor/cloud-hypervisor/blob/main/docs/windows.md). From eaec44ed298332613d720e01e3a35caf9a343e86 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:35:16 +0800 Subject: [PATCH 12/25] chore: tooling and doctor scripts match the code goreleaser archives use ids; fmt-check applies the local import prefix like fmt and CI; make test uses the CI timeout; .gitignore no longer hides cmd/storebench; cocoon-check stops requiring ch-remote, which cocoon never runs; vm-inspect reads the netns from the record so scoped installs work; meta-upgrade.py exits with main's status and is ruff-formatted. --- .gitignore | 11 +++------ .goreleaser.yml | 6 ++--- Makefile | 4 +-- doctor/check.sh | 26 +++---------------- doctor/vm-inspect.sh | 14 ++++++----- scripts/meta-upgrade.py | 55 +++++++++++++++++++++++++++++------------ 6 files changed, 58 insertions(+), 58 deletions(-) diff --git a/.gitignore b/.gitignore index 6f6839ee..59277fc3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,4 @@ -# Internal review documents (contain sensitive information) -docs/P0-RESOLVED.md -docs/P0-VERIFICATION-SUMMARY.md -docs/P1-TODO.md - # Build artifacts /cocoon cocoon-linux @@ -40,7 +35,7 @@ coverage/ .testcache/ # Local dev environment (macOS) -dev/ +/dev/ # Implementation plan (internal) TODO* @@ -50,8 +45,8 @@ tmp/* .tmp/* .claude/* -cocoon-test +/cocoon-test # Local scratch / test scripts (not for upstream) /scratch/ -storebench +/storebench diff --git a/.goreleaser.yml b/.goreleaser.yml index 7ea0aaa6..30c37009 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -38,7 +38,7 @@ builds: archives: - id: release - builds: + ids: - cocoon-linux files: - LICENSE* @@ -53,7 +53,7 @@ archives: {{- if .Arm }}v{{ .Arm }}{{ end -}} - id: debug - builds: + ids: - cocoon-debug files: - LICENSE* @@ -69,7 +69,7 @@ archives: # Unversioned alias so releases/latest/download/cocoon_Linux_x86_64.tar.gz resolves. - id: release-latest - builds: + ids: - cocoon-linux files: - LICENSE* diff --git a/Makefile b/Makefile index 8b4a4bd8..ea97f68b 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ build: ## Build cocoon binary # --- Testing --- test: vet ## Run tests with race detection and coverage - go test -race -timeout 120s -count=1 -cover -coverprofile=coverage.out ./... + go test -race -timeout 20m -count=1 -cover -coverprofile=coverage.out ./... coverage: test ## Generate and display coverage report go tool cover -func=coverage.out @@ -93,7 +93,7 @@ fmt: gofumpt goimports ## Format code with gofumpt and goimports fmt-check: gofumpt goimports ## Check formatting (fails if files need formatting) @test -z "$$($(GOFMT) -extra -l .)" || { echo "Files need formatting (gofumpt):"; $(GOFMT) -extra -l .; exit 1; } - @test -z "$$($(GOIMPORTS) -l .)" || { echo "Files need formatting (goimports):"; $(GOIMPORTS) -l .; exit 1; } + @test -z "$$($(GOIMPORTS) -l --local 'github.com/cocoonstack/cocoon' .)" || { echo "Files need formatting (goimports):"; $(GOIMPORTS) -l --local 'github.com/cocoonstack/cocoon' .; exit 1; } # --- Maintenance --- diff --git a/doctor/check.sh b/doctor/check.sh index 9593f15a..028dc611 100755 --- a/doctor/check.sh +++ b/doctor/check.sh @@ -21,10 +21,9 @@ COCOON_META_BACKEND="${COCOON_META_BACKEND:-}" # Dependency versions. cloud-hypervisor, Firecracker and the x86_64 firmware come from the # cocoonstack fork release tags (rolling dev builds verified against their SHA256SUMS); -# ch-remote, the aarch64 firmware and the CNI plugins come from upstream releases. +# the aarch64 firmware and the CNI plugins come from upstream releases. # The edk2 release publishes no SHA256SUMS, so EDK2_SHA256 pins the aarch64 firmware digest. CH_REF="${CH_REF:-dev}" -CH_REMOTE_VERSION="${CH_REMOTE_VERSION:-v53.0}" CH_MIN_MAJOR=54 FC_REF="${FC_REF:-dev}" FW_REF="${FW_REF:-dev}" @@ -38,8 +37,8 @@ FW_RELEASE_BASE="https://github.com/cocoonstack/rust-hypervisor-firmware/release # Architecture detection ARCH=$(uname -m) case "$ARCH" in - x86_64) GO_ARCH="amd64"; CH_SUFFIX="" ;; - aarch64) GO_ARCH="arm64"; CH_SUFFIX="-aarch64" ;; + x86_64) GO_ARCH="amd64" ;; + aarch64) GO_ARCH="arm64" ;; *) echo "Unsupported architecture: $ARCH"; exit 1 ;; esac @@ -65,7 +64,6 @@ Options: --fix Attempt to fix detected issues (dirs, sysctl, iptables, CNI config) --upgrade Fix issues and install/upgrade dependencies: cloud-hypervisor cocoonstack fork release ${CH_REF} - ch-remote ${CH_REMOTE_VERSION} firecracker cocoonstack fork release ${FC_REF} firmware hypervisor-fw cocoonstack fork release ${FW_REF} (x86_64), EDK2 ${EDK2_REF} (aarch64) CNI plugins ${CNI_VERSION} @@ -73,7 +71,6 @@ Options: Environment variables: CH_REF cocoonstack/cloud-hypervisor release tag (default: ${CH_REF}) - CH_REMOTE_VERSION upstream ch-remote version (default: ${CH_REMOTE_VERSION}) FC_REF cocoonstack/firecracker release tag (default: ${FC_REF}) FW_REF cocoonstack/rust-hypervisor-firmware tag (default: ${FW_REF}) EDK2_REF cloud-hypervisor/edk2 tag, aarch64 only (default: ${EDK2_REF}) @@ -110,8 +107,6 @@ generate_cni_conflist() { # Extract gateway: replace last octet of network address with .1 local network_part network_part=$(echo "$subnet" | cut -d/ -f1) - local prefix_len - prefix_len=$(echo "$subnet" | cut -d/ -f2) # Simple gateway: network address with last octet = 1 local gateway gateway=$(echo "$network_part" | awk -F. '{printf "%s.%s.%s.1", $1, $2, $3}') @@ -209,7 +204,6 @@ check_binary() { local ver="" case "$name" in cloud-hypervisor) ver=$("$name" --version 2>/dev/null | head -1) || true ;; - ch-remote) ver=$("$name" --version 2>/dev/null | head -1) || true ;; firecracker) ver=$("$name" --version 2>/dev/null | head -1) || true ;; qemu-img) ver=$("$name" --version 2>/dev/null | head -1) || true ;; mkfs.ext4) ver=$("$name" -V 2>&1 | head -1) || true ;; @@ -248,7 +242,6 @@ if [ "$ARCH" = "aarch64" ] && ch_features=$(cloud-hypervisor -v --version 2>/dev fail "cloud-hypervisor lacks fw_cfg — arm64 OCI boot hands the kernel to EDK2 over fw_cfg; run --upgrade" fi fi -check_binary ch-remote # Firecracker is optional — only needed for --fc backend. if command -v firecracker &>/dev/null; then check_binary firecracker @@ -510,19 +503,6 @@ if $UPGRADE; then fail "failed to download or verify cloud-hypervisor from ${ch_url}" fi - # -- ch-remote ---------------------------------------------------------- - header "Install ch-remote ${CH_REMOTE_VERSION}" - - chr_url="https://github.com/cloud-hypervisor/cloud-hypervisor/releases/download/${CH_REMOTE_VERSION}/ch-remote-static${CH_SUFFIX}" - chr_dest="/usr/local/bin/ch-remote" - info "downloading ${chr_url}" - if curl -fsSL -o "${tmpdir}/ch-remote" "$chr_url"; then - install -m 0755 "${tmpdir}/ch-remote" "$chr_dest" - fixed "ch-remote ${CH_REMOTE_VERSION} -> ${chr_dest}" - else - fail "failed to download ch-remote from ${chr_url}" - fi - # -- firecracker -------------------------------------------------------- header "Install firecracker (cocoonstack fork ${FC_REF})" diff --git a/doctor/vm-inspect.sh b/doctor/vm-inspect.sh index 6214b980..c0ebe4c3 100755 --- a/doctor/vm-inspect.sh +++ b/doctor/vm-inspect.sh @@ -8,14 +8,13 @@ # Usage: # ./doctor/vm-inspect.sh # -# Requirements: cocoon, jq, ip, tc, nsenter (standard on any Linux with iproute2). +# Requirements: cocoon, jq, ip, tc, curl (ip and tc ship with iproute2). set -uo pipefail # --------------------------------------------------------------------------- # Configuration # --------------------------------------------------------------------------- -COCOON_ROOT_DIR="${COCOON_ROOT_DIR:-/var/lib/cocoon}" COCOON_RUN_DIR="${COCOON_RUN_DIR:-/var/lib/cocoon/run}" COCOON_BIN="${COCOON_BIN:-cocoon}" @@ -247,8 +246,8 @@ fi # --------------------------------------------------------------------------- header "Network Chain (per-NIC)" -NETNS_NAME="cocoon-${VM_ID}" -NETNS_PATH="/var/run/netns/${NETNS_NAME}" +NETNS_PATH=$(echo "$VM_REC" | jq -r '.netns_path // .network_configs[0].netns_path // empty') +NETNS_NAME="${NETNS_PATH##*/}" # Read network configs from VM record NIC_COUNT=$(echo "$VM_REC" | jq '.network_configs | length') @@ -257,11 +256,14 @@ if [ "$NIC_COUNT" -eq 0 ]; then info "No NICs configured" else kv "NIC count" "$NIC_COUNT" - kv "Netns" "$NETNS_NAME" - if [ ! -e "$NETNS_PATH" ]; then + if [ -z "$NETNS_PATH" ]; then + info "No network namespace recorded (backend without netns); skipping veth/tc checks" + elif [ ! -e "$NETNS_PATH" ]; then + kv "Netns" "$NETNS_NAME" fail "Network namespace missing: $NETNS_PATH" else + kv "Netns" "$NETNS_NAME" pass "Network namespace exists" for i in $(seq 0 $((NIC_COUNT - 1))); do diff --git a/scripts/meta-upgrade.py b/scripts/meta-upgrade.py index 4256e497..f338c068 100755 --- a/scripts/meta-upgrade.py +++ b/scripts/meta-upgrade.py @@ -14,23 +14,26 @@ usage: meta-upgrade.py [--root-dir /var/lib/cocoon] """ +from __future__ import annotations + import argparse import fcntl import json import os import sqlite3 import sys +from typing import Any APPLICATION_ID = 0x434F434E USER_VERSION = 2 BACKENDS = ("cloudhypervisor", "firecracker") -def placement(record): +def placement(record: dict[str, Any]) -> list[int] | None: return record.get("queue_cpus") or record.get("cpuset") or None -def upgrade_sqlite(db_path): +def upgrade_sqlite(db_path: str) -> None: if os.path.exists(os.path.join(os.path.dirname(db_path), "meta-convert.manifest")): sys.exit(f"{db_path}: a meta convert is in flight; finish it first") db = sqlite3.connect(db_path, timeout=5, isolation_level=None) @@ -39,18 +42,25 @@ def upgrade_sqlite(db_path): sys.exit(f"{db_path}: application_id {app_id:#x} is not a cocoon meta store") version = db.execute("PRAGMA user_version").fetchone()[0] if version > USER_VERSION: - sys.exit(f"{db_path}: schema version {version} is newer than this script ({USER_VERSION})") + sys.exit( + f"{db_path}: schema version {version} is newer than this script ({USER_VERSION})" + ) db.execute("BEGIN IMMEDIATE") for backend in BACKENDS: ns = f"vms_{backend}" table = f'"{ns}__placements"' - db.execute(f"CREATE TABLE IF NOT EXISTS {table} (id TEXT NOT NULL PRIMARY KEY, data TEXT NOT NULL)") + db.execute( + f"CREATE TABLE IF NOT EXISTS {table} (id TEXT NOT NULL PRIMARY KEY, data TEXT NOT NULL)" + ) rows = 0 - for vm_id, data in db.execute(f'SELECT id, data FROM "{ns}__records"').fetchall(): + for vm_id, data in db.execute( + f'SELECT id, data FROM "{ns}__records"' + ).fetchall(): cpus = placement(json.loads(data)) if cpus: db.execute( - f"INSERT OR REPLACE INTO {table} (id, data) VALUES (?, ?)", (vm_id, json.dumps(cpus).encode()) + f"INSERT OR REPLACE INTO {table} (id, data) VALUES (?, ?)", + (vm_id, json.dumps(cpus).encode()), ) rows += 1 print(f"{ns}: {rows} placement rows") @@ -60,7 +70,7 @@ def upgrade_sqlite(db_path): print(f"{db_path}: schema generation {version} -> {USER_VERSION}") -def write_file(path, data): +def write_file(path: str, data: str) -> None: tmp = path + ".upgrade" with open(tmp, "w") as f: f.write(data) @@ -69,7 +79,7 @@ def write_file(path, data): os.rename(tmp, path) -def upgrade_json(root): +def upgrade_json(root: str) -> None: found = 0 for backend in BACKENDS: db_dir = os.path.join(root, backend, "db") @@ -81,7 +91,11 @@ def upgrade_json(root): fcntl.flock(lock, fcntl.LOCK_EX) with open(path) as f: doc = json.load(f) - rows = {vm_id: cpus for vm_id, rec in doc.get("vms", {}).items() if (cpus := placement(rec))} + rows = { + vm_id: cpus + for vm_id, rec in doc.get("vms", {}).items() + if (cpus := placement(rec)) + } if rows == doc.get("placements", {}): print(f"{path}: {len(rows)} placement rows already present") continue @@ -94,19 +108,28 @@ def upgrade_json(root): os.close(dir_fd) print(f"{path}: {len(rows)} placement rows") if not found: - sys.exit(f"{root}: no meta store found (neither meta/meta.db nor a VM namespace file)") - - -def main(): - parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) - parser.add_argument("--root-dir", default="/var/lib/cocoon", help="cocoon root_dir (default: %(default)s)") + sys.exit( + f"{root}: no meta store found (neither meta/meta.db nor a VM namespace file)" + ) + + +def main() -> int: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + parser.add_argument( + "--root-dir", + default="/var/lib/cocoon", + help="cocoon root_dir (default: %(default)s)", + ) root = parser.parse_args().root_dir db_path = os.path.join(root, "meta", "meta.db") if os.path.exists(db_path): upgrade_sqlite(db_path) else: upgrade_json(root) + return 0 if __name__ == "__main__": - main() + sys.exit(main()) From 60896a441ea08f1c4af77b500ce363074da62f05 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 19:47:15 +0800 Subject: [PATCH 13/25] fix(scripts): meta-upgrade placement() returns the cpu list string --- scripts/meta-upgrade.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/meta-upgrade.py b/scripts/meta-upgrade.py index f338c068..972b06c8 100755 --- a/scripts/meta-upgrade.py +++ b/scripts/meta-upgrade.py @@ -29,7 +29,7 @@ BACKENDS = ("cloudhypervisor", "firecracker") -def placement(record: dict[str, Any]) -> list[int] | None: +def placement(record: dict[str, Any]) -> str | None: return record.get("queue_cpus") or record.get("cpuset") or None From 679883bdb3beb76484737ee3508cc879c23c52f3 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:18:08 +0800 Subject: [PATCH 14/25] fix(convert): refuse to convert while a daemon is running The daemon keeps its store open across the cutover, and checkQuiesced only probes the namespace locks for an instant, which an idle daemon does not hold. After json to sqlite the json engine reads the retired files as empty, so the next --gc-interval sweep treats every VM as an orphan: it kills the VMM and deletes the COW disk. After sqlite to json it keeps reading the renamed database, so VMs created later die the same way. Convert now holds the daemon's single-instance lock for the whole run, resume included. Hot-path cost is zero. --- cmd/meta/convert/convert.go | 29 +++++++++++++++++++++++++---- cmd/meta/convert/convert_test.go | 32 +++++++++++++++++++++++++++++++- cmd/meta/handler.go | 10 ++++++---- daemon/daemon.go | 11 ++++++++--- 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/cmd/meta/convert/convert.go b/cmd/meta/convert/convert.go index 6179685a..ff23d345 100644 --- a/cmd/meta/convert/convert.go +++ b/cmd/meta/convert/convert.go @@ -30,10 +30,11 @@ var testCrashStep func(step string) error // Spec carries both engines' declarations from the composition root. type Spec struct { - MetaRoot string // manifest directory (parent of the sqlite DB) - DBPath string - Decls []metasqlite.Namespace - JSON []metajson.Namespace + MetaRoot string // manifest directory (parent of the sqlite DB) + DBPath string + DaemonLock string + Decls []metasqlite.Namespace + JSON []metajson.Namespace } // Manifest is the crash-recovery authority for one conversion run. @@ -56,6 +57,11 @@ func Run(ctx context.Context, spec Spec, target string) error { if target != config.MetaBackendSQLite && target != config.MetaBackendJSON { return fmt.Errorf("unknown target engine %q", target) } + unlock, err := lockOutDaemon(ctx, spec.DaemonLock) + if err != nil { + return err + } + defer unlock() m, err := loadManifest(spec.MetaRoot) if err != nil { return err @@ -74,6 +80,21 @@ func Run(ctx context.Context, spec Spec, target string) error { return finishManifest(spec.MetaRoot) } +func lockOutDaemon(ctx context.Context, path string) (func(), error) { + if err := utils.EnsureDirs(filepath.Dir(path)); err != nil { + return nil, err + } + l := flock.New(path) + ok, err := l.TryLock(ctx) + if err != nil { + return nil, err + } + if !ok { + return nil, errors.New("a cocoon daemon is running; stop it before converting") + } + return func() { _ = l.Unlock(ctx) }, nil +} + func crashStep(step string) error { if testCrashStep == nil { return nil diff --git a/cmd/meta/convert/convert_test.go b/cmd/meta/convert/convert_test.go index 7941bd03..33269eb0 100644 --- a/cmd/meta/convert/convert_test.go +++ b/cmd/meta/convert/convert_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/cocoonstack/cocoon/lock/flock" "github.com/cocoonstack/cocoon/meta" metajson "github.com/cocoonstack/cocoon/meta/json" metasqlite "github.com/cocoonstack/cocoon/meta/sqlite" @@ -228,6 +229,31 @@ func TestConvertRefusesUnsupportedFS(t *testing.T) { } } +func TestConvertRefusesARunningDaemon(t *testing.T) { + spec := testSpec(t, "vms") + seedJSON(t, spec, "vms") + if err := os.MkdirAll(filepath.Dir(spec.DaemonLock), 0o750); err != nil { + t.Fatal(err) + } + daemon := flock.New(spec.DaemonLock) + if ok, err := daemon.TryLock(t.Context()); err != nil || !ok { + t.Fatalf("hold daemon lock: ok=%v err=%v", ok, err) + } + err := Run(t.Context(), spec, "sqlite") + if err == nil || !strings.Contains(err.Error(), "daemon is running") { + t.Fatalf("want daemon refusal, got %v", err) + } + if !utils.FileExists(spec.JSON[0].FilePath) || utils.FileExists(spec.DBPath) { + t.Fatal("the refused convert touched the stores") + } + if err := daemon.Unlock(t.Context()); err != nil { + t.Fatal(err) + } + if err := Run(t.Context(), spec, "sqlite"); err != nil { + t.Fatalf("convert after the daemon stopped: %v", err) + } +} + func TestCrashRerunMatrix(t *testing.T) { errCrash := errors.New("injected crash") forward := []string{"manifest-saved", "target-opened", "ns-copied", "ns-marked", "ns-done", "source-renamed"} @@ -463,7 +489,11 @@ func TestConvertRerunsAfterCrashedTargetInit(t *testing.T) { func testSpec(t *testing.T, nss ...string) Spec { t.Helper() root := t.TempDir() - spec := Spec{MetaRoot: filepath.Join(root, "meta"), DBPath: filepath.Join(root, "meta", metasqlite.DBFileName)} + spec := Spec{ + MetaRoot: filepath.Join(root, "meta"), + DBPath: filepath.Join(root, "meta", metasqlite.DBFileName), + DaemonLock: filepath.Join(root, "locks", "daemon.lock"), + } for _, ns := range nss { spec.Decls = append(spec.Decls, metasqlite.Namespace{Name: ns, Tables: testTables}) spec.JSON = append(spec.JSON, metajson.Namespace{ diff --git a/cmd/meta/handler.go b/cmd/meta/handler.go index 76a74ee2..c2ee7004 100644 --- a/cmd/meta/handler.go +++ b/cmd/meta/handler.go @@ -10,6 +10,7 @@ import ( cmdcore "github.com/cocoonstack/cocoon/cmd/core" "github.com/cocoonstack/cocoon/cmd/meta/convert" "github.com/cocoonstack/cocoon/config" + "github.com/cocoonstack/cocoon/daemon" metasqlite "github.com/cocoonstack/cocoon/meta/sqlite" ) @@ -39,10 +40,11 @@ func (h Handler) Convert(cmd *cobra.Command, _ []string) error { target := cmp.Or(conf.MetaBackend, config.MetaBackendSQLite) dbPath := cmdcore.MetaDBPath(conf) spec := convert.Spec{ - MetaRoot: filepath.Dir(dbPath), - DBPath: dbPath, - Decls: cmdcore.MetaNamespaces(), - JSON: cmdcore.MetaJSONNamespaces(conf), + MetaRoot: filepath.Dir(dbPath), + DBPath: dbPath, + DaemonLock: daemon.LockPath(conf.RootDir), + Decls: cmdcore.MetaNamespaces(), + JSON: cmdcore.MetaJSONNamespaces(conf), } if err := convert.Run(ctx, spec, target); err != nil { return err diff --git a/daemon/daemon.go b/daemon/daemon.go index d945e397..ab3c59a1 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -141,11 +141,11 @@ func (d *Daemon) Run(ctx context.Context) error { } func (d *Daemon) lockInstance(ctx context.Context) (func(), error) { - dir := filepath.Join(d.conf.RootDir, "locks") - if err := utils.EnsureDirs(dir); err != nil { + path := LockPath(d.conf.RootDir) + if err := utils.EnsureDirs(filepath.Dir(path)); err != nil { return nil, err } - l := flock.New(filepath.Join(dir, lockFileName)) + l := flock.New(path) ok, err := l.TryLock(ctx) if err != nil { return nil, fmt.Errorf("acquire daemon lock: %w", err) @@ -189,6 +189,11 @@ func (d *Daemon) runGC(ctx context.Context) { }() } +// LockPath is the single-instance lock a running daemon holds. +func LockPath(rootDir string) string { + return filepath.Join(rootDir, "locks", lockFileName) +} + func drain(ch <-chan struct{}) { for { select { From b77c198f95ad0c2fd1a2003b518d64a25fe17e38 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:18:36 +0800 Subject: [PATCH 15/25] fix(convert): check the target filesystem before the manifest is durable A json root on NFS, CIFS or FUSE cannot host sqlite, yet the CLI suggests meta convert on every json start. The filesystem refusal came after the manifest was saved, so every later verb refused with 'a conversion is in flight' and the rerun hit the same refusal; only deleting the manifest by hand got out. --- cmd/meta/convert/convert.go | 13 +++++++++++++ cmd/meta/convert/convert_test.go | 3 +++ meta/sqlite/fscheck.go | 4 ++-- meta/sqlite/init.go | 2 +- meta/sqlite/store.go | 2 +- 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/cmd/meta/convert/convert.go b/cmd/meta/convert/convert.go index ff23d345..fb73e990 100644 --- a/cmd/meta/convert/convert.go +++ b/cmd/meta/convert/convert.go @@ -115,6 +115,9 @@ func convertAll(ctx context.Context, spec Spec, target string, m *Manifest) erro return qerr } if m == nil { + if err := checkTargetFS(spec, target); err != nil { + return err + } if m, err = newManifest(ctx, spec, target, src); err != nil { return err } @@ -149,6 +152,16 @@ func convertAll(ctx context.Context, spec Spec, target string, m *Manifest) erro return nil } +func checkTargetFS(spec Spec, target string) error { + if target != config.MetaBackendSQLite { + return nil + } + if err := utils.EnsureDirs(filepath.Dir(spec.DBPath)); err != nil { + return err + } + return metasqlite.CheckFS(spec.DBPath) +} + func newManifest(ctx context.Context, spec Spec, target string, src meta.Store) (*Manifest, error) { m := &Manifest{Target: target, Namespaces: map[string]*NSRecord{}, StartedAt: time.Now()} for _, ns := range spec.Decls { diff --git a/cmd/meta/convert/convert_test.go b/cmd/meta/convert/convert_test.go index 33269eb0..b24b8b59 100644 --- a/cmd/meta/convert/convert_test.go +++ b/cmd/meta/convert/convert_test.go @@ -227,6 +227,9 @@ func TestConvertRefusesUnsupportedFS(t *testing.T) { if utils.FileExists(spec.DBPath) { t.Fatal("convert created target WAL state on refused filesystem") } + if err := metasqlite.RefuseManifest(spec.DBPath); err != nil { + t.Fatalf("the refused convert locked the root: %v", err) + } } func TestConvertRefusesARunningDaemon(t *testing.T) { diff --git a/meta/sqlite/fscheck.go b/meta/sqlite/fscheck.go index debac74e..76091352 100644 --- a/meta/sqlite/fscheck.go +++ b/meta/sqlite/fscheck.go @@ -7,8 +7,8 @@ import ( "github.com/cocoonstack/cocoon/meta" ) -// checkFS refuses unsupported filesystems before any WAL work (§4); the env seam lets every entry path (open, init, convert) be asserted in tests. -func checkFS(dbPath string) error { +// CheckFS refuses unsupported filesystems before any WAL work (§4); the env seam lets every entry path (open, init, convert) be asserted in tests. +func CheckFS(dbPath string) error { if name := os.Getenv("COCOON_TEST_UNSUPPORTED_FS"); name != "" { return fsRefusal(dbPath, name) } diff --git a/meta/sqlite/init.go b/meta/sqlite/init.go index 65d8d710..c4b98287 100644 --- a/meta/sqlite/init.go +++ b/meta/sqlite/init.go @@ -53,7 +53,7 @@ func initStore(ctx context.Context, dbPath string, namespaces []Namespace) (err if merr := utils.EnsureDirs(filepath.Dir(dbPath)); merr != nil { return merr } - if ferr := checkFS(dbPath); ferr != nil { + if ferr := CheckFS(dbPath); ferr != nil { return ferr } if utils.FileExists(dbPath) { diff --git a/meta/sqlite/store.go b/meta/sqlite/store.go index 0557e70b..5ca4344f 100644 --- a/meta/sqlite/store.go +++ b/meta/sqlite/store.go @@ -86,7 +86,7 @@ func openStore(dbPath string, namespaces []Namespace) (*Store, error) { if !utils.FileExists(dbPath) { return nil, fmt.Errorf("no sqlite store at %s: run `cocoon meta init` or `cocoon meta convert`", dbPath) } - if err := checkFS(dbPath); err != nil { + if err := CheckFS(dbPath); err != nil { return nil, err } s := &Store{path: dbPath, nss: map[string]Namespace{}} From 731bdd847d390938928d751f10d71e930e7a4788 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:19:37 +0800 Subject: [PATCH 16/25] fix(hypervisor): a restore's kill schedules the quiesce it needs 3ff73bb8 set the flag at placement, but a restore of a running VM has already committed Stopped by then. A SIGKILL during apply left Stopped with no flag and the plumbing up: the daemon skips that record and vm stop finds nothing to do, so only another restore or vm rm cleared it. The commit that erases the Running signal now sets the flag. The test also pins placement before network recovery for a stopped origin. --- hypervisor/restore.go | 1 + hypervisor/restore_test.go | 59 +++++++++++++++++++++++++++++++++++++ hypervisor/teardown_test.go | 8 +++-- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/hypervisor/restore.go b/hypervisor/restore.go index dbfef397..46d31b69 100644 --- a/hypervisor/restore.go +++ b/hypervisor/restore.go @@ -213,6 +213,7 @@ func (b *Backend) emitRestoreComputeStop(ctx context.Context, vmID string, oldSh } markTransition(r, types.VMStateStopped, types.TransitionRestore, now) r.StoppedAt = &now + r.QuiescePending = needsQuiesce(r) closed = true return t.Put(vmID, r) }); err != nil { diff --git a/hypervisor/restore_test.go b/hypervisor/restore_test.go index 88cf9eac..27e64723 100644 --- a/hypervisor/restore_test.go +++ b/hypervisor/restore_test.go @@ -152,6 +152,65 @@ func TestRunningRestoreFailureSchedulesNetworkConvergence(t *testing.T) { } } +func TestRestoreKeepsAQuiescePendingFromTheKillToTheVMM(t *testing.T) { + tests := []struct { + name string + origin types.VMState + pendingInApply bool + }{ + {"running origin", types.VMStateRunning, true}, + {"stopped origin", types.VMStateStopped, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, _ := newMeteringTestBackend(t) + ctx := t.Context() + const id = "vm-restore-window" + seedRunningVM(t, b, id, 1, 1<<30, 10<<30) + if err := b.dbUpdate(ctx, func(idx *VMIndex) error { + idx.VMs[id].RunDir = shortTempDir(t) + idx.VMs[id].NetSetup = types.NetSetup{ + NetBackend: types.BackendCNI, + NetworkConfigs: []*types.NetworkConfig{{}}, + } + if tt.origin == types.VMStateStopped { + idx.VMs[id].State = tt.origin + idx.VMs[id].StoppedAt = new(timeNow()) + } + return nil + }); err != nil { + t.Fatalf("seed network: %v", err) + } + pending := func() bool { return recordOf(t, b, id).QuiescePending } + b.SetNetwork(stubNetwork{recover: func(context.Context, *types.VM) error { + if !pending() { + t.Error("a kill during network recovery leaves no quiesce pending") + } + return nil + }}) + + _, err := b.DirectRestoreSequence(ctx, id, DirectRestoreSpec{ + VMCfg: &types.VMConfig{CPU: 1, Memory: 1 << 30, Storage: 10 << 30}, + SrcDir: t.TempDir(), + Preflight: func(string, *VMRecord) error { return nil }, + Kill: func(context.Context, string, *VMRecord) error { return nil }, + Populate: func(*VMRecord, string) error { + if got := pending(); got != tt.pendingInApply { + t.Errorf("pending during populate = %v, want %v", got, tt.pendingInApply) + } + return nil + }, + AfterExtract: func(context.Context, string, *types.VMConfig, *VMRecord) (*types.VM, error) { + return nil, errors.New("stop after the window") + }, + }) + if err == nil { + t.Fatal("expected the AfterExtract failure") + } + }) + } +} + func TestRestorePartialMergeQuarantinesEvenStoppedOrigin(t *testing.T) { b, _ := newMeteringTestBackend(t) const id = "vm-merge-fail" diff --git a/hypervisor/teardown_test.go b/hypervisor/teardown_test.go index 06e11d11..38e9eeea 100644 --- a/hypervisor/teardown_test.go +++ b/hypervisor/teardown_test.go @@ -327,12 +327,16 @@ func TestPrepareStartRefusesMidDeleting(t *testing.T) { } type stubNetwork struct { + recover func(context.Context, *types.VM) error quiesce func(context.Context, *types.VM) error cleanup func(context.Context, string) error } -func (s stubNetwork) Recover(context.Context, *types.VM) error { - return nil +func (s stubNetwork) Recover(ctx context.Context, vm *types.VM) error { + if s.recover == nil { + return nil + } + return s.recover(ctx, vm) } func (s stubNetwork) Quiesce(ctx context.Context, vm *types.VM) error { From 0d0e5013716949606c6fe65b3226c3025cc12827 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:20:04 +0800 Subject: [PATCH 17/25] fix(daemon): a VM that cannot converge no longer wakes its own daemon RecoverTombstone commits a fresh lease before its teardown fails. That commit fires the meta event, so a persistent failure such as a removed CNI conflist ran a full pass about four times a second instead of every five seconds: a /proc walk, several durable commits per stuck VM and plugin execs, all contending with CLI writers. After a degraded pass the loop leaves the next pass to the ticker. --- daemon/daemon.go | 9 ++++-- daemon/reconcile.go | 5 ++-- daemon/reconcile_test.go | 63 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index ab3c59a1..2bcf689b 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -110,7 +110,7 @@ func (d *Daemon) Run(ctx context.Context) error { gcTick := d.gcTick() logger.Infof(ctx, "supervising %d backend(s), reconcile every %s", len(d.order), d.conf.ReconcileInterval) - d.reconcile(ctx) + stuck := d.reconcile(ctx) for { select { case <-ctx.Done(): @@ -123,13 +123,16 @@ func (d *Daemon) Run(ctx context.Context) error { logger.Info(ctx, "meta change events restored") } } - d.reconcile(ctx) + stuck = d.reconcile(ctx) case <-events: + if stuck { + continue + } if !d.settle(ctx) { return nil } drain(events) - d.reconcile(ctx) + stuck = d.reconcile(ctx) case ev := <-d.watcher.exits: d.handleExit(ctx, ev) // The convergence just written wakes this daemon's own subscription; its pass already ran. diff --git a/daemon/reconcile.go b/daemon/reconcile.go index dcebef80..634f3873 100644 --- a/daemon/reconcile.go +++ b/daemon/reconcile.go @@ -12,8 +12,8 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -// reconcile runs one idempotent pass over every backend and republishes the API snapshot. -func (d *Daemon) reconcile(ctx context.Context) { +// reconcile runs one idempotent pass over every backend, republishes the API snapshot, and reports whether a VM could not converge. +func (d *Daemon) reconcile(ctx context.Context) bool { var all []VMStatus healthy, degraded := true, 0 for _, b := range d.order { @@ -27,6 +27,7 @@ func (d *Daemon) reconcile(ctx context.Context) { all = append(all, st...) } d.state.publish(all, healthy, degraded, time.Now()) + return degraded > 0 } // reconcileBackend returns the backend's statuses, how many VMs the pass could not converge, and any scan failure; a busy ops lock is not a failure. diff --git a/daemon/reconcile_test.go b/daemon/reconcile_test.go index 5be8e369..745eb76f 100644 --- a/daemon/reconcile_test.go +++ b/daemon/reconcile_test.go @@ -4,12 +4,17 @@ import ( "context" "errors" "maps" + "path/filepath" "slices" "sync" + "sync/atomic" "testing" "time" "github.com/cocoonstack/cocoon/hypervisor" + "github.com/cocoonstack/cocoon/meta" + metasqlite "github.com/cocoonstack/cocoon/meta/sqlite" + "github.com/cocoonstack/cocoon/meta/tombstone" "github.com/cocoonstack/cocoon/types" "github.com/cocoonstack/cocoon/utils" ) @@ -180,6 +185,41 @@ func TestHandleExitIgnoresDeletedRecord(t *testing.T) { } } +func TestFailingRollForwardDoesNotWakeItsOwnDaemon(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 1500*time.Millisecond) + defer cancel() + const ns = "vms_fake" + decl := metasqlite.Namespace{Name: ns, Tables: []string{"records", tombstone.TableName}} + dir := t.TempDir() + db := filepath.Join(dir, "meta", metasqlite.DBFileName) + if err := metasqlite.Init(ctx, db, decl); err != nil { + t.Fatal(err) + } + store, err := metasqlite.Open(db, decl) + if err != nil { + t.Fatal(err) + } + defer store.Close() + ts := tombstone.NewTable(ns) + if err := store.Update(ctx, meta.Scope{Write: ns}, meta.CommitDurable, func(w meta.Writer) error { + lease, err := ts.Lease(ctx, w, "vm1", tombstone.Payload{Kind: tombstone.KindRecord, Mode: tombstone.ModeAggregate, Cleanup: []byte(`{}`)}) + if err != nil { + return err + } + return ts.MarkDeleting(ctx, w, "vm1", lease) + }); err != nil { + t.Fatal(err) + } + f := &committingSupervisor{fakeSupervisor: newFake().put(runningRec("vm1", 1)), store: store, ns: ns} + f.tombstoned["vm1"] = struct{}{} + if err := New(Config{RootDir: dir, ReconcileInterval: time.Hour}, store, []Supervisor{f}).Run(ctx); err != nil { + t.Fatal(err) + } + if got := f.passes.Load(); got != 1 { + t.Fatalf("passes = %d with an hourly ticker, want 1", got) + } +} + func TestPassCountsDegradedVMsWithoutGoingUnready(t *testing.T) { tests := []struct { name string @@ -371,6 +411,29 @@ func newFake() *fakeSupervisor { } } +type committingSupervisor struct { + *fakeSupervisor + store meta.Store + ns string + passes atomic.Int64 +} + +func (s *committingSupervisor) ScanSupervision(ctx context.Context) (hypervisor.SupervisionScan, error) { + s.passes.Add(1) + return s.fakeSupervisor.ScanSupervision(ctx) +} + +func (s *committingSupervisor) RecoverTombstone(ctx context.Context, id string) (bool, error) { + var cl struct{} + if err := s.store.Update(ctx, meta.Scope{Write: s.ns}, meta.CommitDurable, func(w meta.Writer) error { + _, err := tombstone.NewTable(s.ns).Recover(ctx, w, id, &cl) + return err + }); err != nil { + return false, err + } + return false, errors.New("cni down") +} + func newTestDaemon(t *testing.T, f *fakeSupervisor) *Daemon { t.Helper() return New(Config{RootDir: t.TempDir()}, nil, []Supervisor{f}) From ba3f00578951f2154d9d10e7f0d2af73fccccc6f Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:20:04 +0800 Subject: [PATCH 18/25] fix(vm): migrate receive --id refuses a busy id The reclaim's busy outcome was discarded, so the receive blocked on the lock and then adopted the placeholder: over a deleting tombstone the next roll-forward destroyed the received VM, and over a concurrent receive it removed the directories under that receive's VMM. --- cmd/vm/migrate.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/vm/migrate.go b/cmd/vm/migrate.go index 854208d4..17e9f41c 100644 --- a/cmd/vm/migrate.go +++ b/cmd/vm/migrate.go @@ -48,8 +48,12 @@ func (h Handler) MigrateReceive(cmd *cobra.Command, _ []string) error { } // A receive killed mid-flight leaves its placeholder and maybe a VMM under this ID. if id != "" { - if _, err = hyper.ReconcileStaleCreate(ctx, id); err != nil { - return fmt.Errorf("reclaim interrupted receive: %w", err) + outcome, reclaimErr := hyper.ReconcileStaleCreate(ctx, id) + if reclaimErr != nil { + return fmt.Errorf("reclaim interrupted receive: %w", reclaimErr) + } + if outcome == hypervisor.StaleCreateBusy { + return fmt.Errorf("vm %s is busy: another operation owns it", id) } } cs, err := h.prepareClone(ctx, cmd, conf, hyper, cfg) From b8571e3bcac750e40c5cd60adac0cac646594b62 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:20:04 +0800 Subject: [PATCH 19/25] docs: convert needs the daemon stopped, a dead target VMM cold-boots Also correct the QuiescePending comment and the daemon event note after the flag started covering launches, and name the way out in meta-upgrade's in-flight refusal. --- docs/cli.md | 2 +- docs/daemon.md | 2 +- docs/snapshots.md | 2 +- hypervisor/db.go | 2 +- scripts/meta-upgrade.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 4bc1c073..6bb066b3 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -65,7 +65,7 @@ cocoon `daemon` is optional: every other command works standalone with no daemon running. See [Daemon](daemon.md). -The meta engine is selected by `meta_backend` in the config; unset auto-resolves — an existing store binds its engine (legacy json roots keep json), fresh roots get `sqlite` and bootstrap themselves. `meta convert` always converts TO the configured `meta_backend` (unset = sqlite). Run `scripts/meta-upgrade.py --root-dir ` (in the release archive next to the binary) once after a binary swap that raises the schema generation: it creates the tables the new generation declares, writes the placement rows for VMs the previous binary launched, and is safe to rerun. A sqlite store from an older generation is refused by name until then; a json root has no generation stamp, so it is not refused, but its running VMs stay out of the placement tally until the script has run. +The meta engine is selected by `meta_backend` in the config; unset auto-resolves — an existing store binds its engine (legacy json roots keep json), fresh roots get `sqlite` and bootstrap themselves. `meta convert` always converts TO the configured `meta_backend` (unset = sqlite). Stop the daemon and any `cocoon gc` timer first: convert refuses while the daemon runs, and it needs the store otherwise idle. Run `scripts/meta-upgrade.py --root-dir ` (in the release archive next to the binary) once after a binary swap that raises the schema generation: it creates the tables the new generation declares, writes the placement rows for VMs the previous binary launched, and is safe to rerun. A sqlite store from an older generation is refused by name until then; a json root has no generation stamp, so it is not refused, but its running VMs stay out of the placement tally until the script has run. ## Global Flags diff --git a/docs/daemon.md b/docs/daemon.md index a1648af3..fdb8bf2f 100644 --- a/docs/daemon.md +++ b/docs/daemon.md @@ -162,4 +162,4 @@ The stream is a hint, not a log: it replays nothing and a slow reader loses events. After a reconnect, compare each VM's `transition_generation` against the last one you handled — a generation you have not seen whose state is `stopped` and whose reason is `unexpected-exit` is a crash you missed. A `MODIFIED` can -repeat a generation: the quiesce-pending flag is cleared without bumping it. +repeat a generation: the quiesce-pending flag changes without bumping it. diff --git a/docs/snapshots.md b/docs/snapshots.md index 1ebf8daf..c94b0059 100644 --- a/docs/snapshots.md +++ b/docs/snapshots.md @@ -187,7 +187,7 @@ Rules for the caller: - A VM cloned or received seconds ago may still eject its old NICs. If that happens during a series, the next delta fails; start again. - Without `--id` the receive generates an ID. With `--id`, a retry after a killed receive reclaims what it left. A retry after a finished receive fails with `id collision`; `vm inspect` shows the VM. Only `vm migrate receive` reads a series directory: `vm clone --from-dir` would pair the last memory with the baseline disks. -The received VM gets fresh NICs like a clone, with new MACs, so the guest needs the [post-clone network setup](#post-clone-guest-setup). No reseed runs: the guest state is the same one, not a copy. `--restore-mode ondemand` is rejected by the hypervisor for a series with deltas. Metering opens the new intervals with reason `migrate`. +The received VM gets fresh NICs like a clone, with new MACs, so the guest needs the [post-clone network setup](#post-clone-guest-setup). No reseed runs: the guest state is the same one, not a copy. If the target VMM dies before `vm start`, the start cold-boots from the disk and still succeeds, so check the guest before removing the source. `--restore-mode ondemand` is rejected by the hypervisor for a series with deltas. Metering opens the new intervals with reason `migrate`. A qcow2 overlay is open when captured, so its dirty bit is set and the target VMM rebuilds the refcounts when it opens the file. The overlay hash therefore differs after the receive; compare it before the VMM opens it. diff --git a/hypervisor/db.go b/hypervisor/db.go index f23d91b9..3e110fbf 100644 --- a/hypervisor/db.go +++ b/hypervisor/db.go @@ -18,6 +18,6 @@ type VMRecord struct { // Quarantine names why start must refuse; only a successful restore clears it. Quarantine string `json:"quarantine,omitempty"` - // QuiescePending survives a crash between the stop transition and the network Quiesce, so a later pass retries it. + // QuiescePending marks plumbing a stop or a killed launch may have left up, so a later pass brings it down. QuiescePending bool `json:"quiesce_pending,omitzero"` } diff --git a/scripts/meta-upgrade.py b/scripts/meta-upgrade.py index 972b06c8..f7a77cd5 100755 --- a/scripts/meta-upgrade.py +++ b/scripts/meta-upgrade.py @@ -35,7 +35,7 @@ def placement(record: dict[str, Any]) -> str | None: def upgrade_sqlite(db_path: str) -> None: if os.path.exists(os.path.join(os.path.dirname(db_path), "meta-convert.manifest")): - sys.exit(f"{db_path}: a meta convert is in flight; finish it first") + sys.exit(f"{db_path}: a meta convert is in flight; finish it with the binary that started it") db = sqlite3.connect(db_path, timeout=5, isolation_level=None) app_id = db.execute("PRAGMA application_id").fetchone()[0] if app_id != APPLICATION_ID: From 600fe888a3bc94c3174509c555390e82ee132781 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 20:20:29 +0800 Subject: [PATCH 20/25] review: convert's filesystem check does not shadow err; ruff format --- cmd/meta/convert/convert.go | 4 ++-- scripts/meta-upgrade.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/meta/convert/convert.go b/cmd/meta/convert/convert.go index fb73e990..ae21c9df 100644 --- a/cmd/meta/convert/convert.go +++ b/cmd/meta/convert/convert.go @@ -115,8 +115,8 @@ func convertAll(ctx context.Context, spec Spec, target string, m *Manifest) erro return qerr } if m == nil { - if err := checkTargetFS(spec, target); err != nil { - return err + if ferr := checkTargetFS(spec, target); ferr != nil { + return ferr } if m, err = newManifest(ctx, spec, target, src); err != nil { return err diff --git a/scripts/meta-upgrade.py b/scripts/meta-upgrade.py index f7a77cd5..9cb74dde 100755 --- a/scripts/meta-upgrade.py +++ b/scripts/meta-upgrade.py @@ -35,7 +35,9 @@ def placement(record: dict[str, Any]) -> str | None: def upgrade_sqlite(db_path: str) -> None: if os.path.exists(os.path.join(os.path.dirname(db_path), "meta-convert.manifest")): - sys.exit(f"{db_path}: a meta convert is in flight; finish it with the binary that started it") + sys.exit( + f"{db_path}: a meta convert is in flight; finish it with the binary that started it" + ) db = sqlite3.connect(db_path, timeout=5, isolation_level=None) app_id = db.execute("PRAGMA application_id").fetchone()[0] if app_id != APPLICATION_ID: From 7f78a580e242b5a6b0a731e09baa1ceab6b4c2ef Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 21:07:08 +0800 Subject: [PATCH 21/25] fix(cloudhypervisor): a capture waits for the guest to eject a swapped NIC A clone hot-swaps its NICs while paused, and the old NIC stays in the device tree until the guest acks the eject. A snapshot taken before that ack saves the node without the pending-eject bit, so every VM cloned from it keeps a half-ejected NIC forever and passes it down the chain. On hardware, 4 of 5 zero-delay snapshots carried it, and one clone came up with no NIC in the guest while vm clone reported success. Pause now waits up to ejectWaitTimeout. On timeout it warns and goes on instead of refusing, because descendants of an already poisoned snapshot hold a node that never ejects and must stay capturable. A VM with nothing pending pays nothing: vm.info is already read here. --- docs/snapshots.md | 2 +- hypervisor/cloudhypervisor/snapshot.go | 33 +++++++++++++ hypervisor/cloudhypervisor/snapshot_test.go | 55 +++++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/docs/snapshots.md b/docs/snapshots.md index c94b0059..92b03029 100644 --- a/docs/snapshots.md +++ b/docs/snapshots.md @@ -184,7 +184,7 @@ Rules for the caller: - Copy every file a round lists, in the listed round order. `state.json` and `config.json` are replaced each round. A `.ref` file never leaves the source. A short or missing disk delta fails the receive. - A VM has one series. A new baseline ends the previous one and deletes its directory; the directory of a series nobody continues stays until `gc` removes it after 24 hours. A delta with an old series ID fails and leaves the guest alone. - A failed round ends the series, resumes the guest and deletes its directory; start again with a new baseline. A round after a killed capture fails the same way. `snapshot save`, removing a NIC, a device detach, a reboot or a stop ends the series too, and the next delta fails. -- A VM cloned or received seconds ago may still eject its old NICs. If that happens during a series, the next delta fails; start again. +- A VM cloned or received seconds ago may still eject its old NICs. A capture waits up to 30 seconds for that, then goes on with a warning; if an eject lands during a series, the next delta fails, so start again. - Without `--id` the receive generates an ID. With `--id`, a retry after a killed receive reclaims what it left. A retry after a finished receive fails with `id collision`; `vm inspect` shows the VM. Only `vm migrate receive` reads a series directory: `vm clone --from-dir` would pair the last memory with the baseline disks. The received VM gets fresh NICs like a clone, with new MACs, so the guest needs the [post-clone network setup](#post-clone-guest-setup). No reseed runs: the guest state is the same one, not a copy. If the target VMM dies before `vm start`, the start cold-boots from the disk and still succeeds, so check the guest before removing the source. `--restore-mode ondemand` is rejected by the hypervisor for a series with deltas. Metering opens the new intervals with reason `migrate`. diff --git a/hypervisor/cloudhypervisor/snapshot.go b/hypervisor/cloudhypervisor/snapshot.go index b7f9e145..f4e336cb 100644 --- a/hypervisor/cloudhypervisor/snapshot.go +++ b/hypervisor/cloudhypervisor/snapshot.go @@ -5,6 +5,8 @@ import ( "fmt" "net/http" "path/filepath" + "slices" + "strings" "time" "github.com/projecteru2/core/log" @@ -49,6 +51,9 @@ func (ch *CloudHypervisor) snapshotSpec(ctx context.Context) hypervisor.Snapshot if len(info.Config.Nets) != len(rec.NetworkConfigs) { return fmt.Errorf("vm %s has %d NICs but its record has %d; rerun vm net first", rec.ID, len(info.Config.Nets), len(rec.NetworkConfigs)) } + if err := waitPendingNICEjects(ctx, hc, rec.ID, info); err != nil { + return err + } if err := waitBalloonSettled(ctx, hc, rec.ID, info); err != nil { return err } @@ -120,6 +125,34 @@ func hotAttachedError(fs []chFs, devices []chDevice, disks []chDisk) error { return nil } +func waitPendingNICEjects(ctx context.Context, hc *http.Client, vmID string, info *chVMInfoResponse) error { + if !hasPendingNICEject(info) { + return nil + } + err := utils.WaitFor(ctx, ejectWaitTimeout, 100*time.Millisecond, func() (bool, error) { + cur, infoErr := getVMInfo(ctx, hc) + if infoErr != nil { + return false, infoErr + } + return !hasPendingNICEject(cur), nil + }) + if err == nil || ctx.Err() != nil { + return err + } + log.WithFunc("cloudhypervisor.waitPendingNICEjects").Warnf(ctx, "vm %s: capturing with a NIC the guest has not ejected: %v", vmID, err) + return nil +} + +func hasPendingNICEject(info *chVMInfoResponse) bool { + for id := range info.DeviceTree { + isNIC := strings.HasPrefix(id, "_net") || strings.HasPrefix(id, cocoonNetIDPrefix) + if isNIC && !slices.ContainsFunc(info.Config.Nets, func(n chNet) bool { return n.ID == id }) { + return true + } + } + return false +} + func waitBalloonSettled(ctx context.Context, hc *http.Client, vmID string, info *chVMInfoResponse) error { balloon := info.Config.Balloon if balloon == nil || info.State != chStateRunning { diff --git a/hypervisor/cloudhypervisor/snapshot_test.go b/hypervisor/cloudhypervisor/snapshot_test.go index 209fc2e4..6e037c60 100644 --- a/hypervisor/cloudhypervisor/snapshot_test.go +++ b/hypervisor/cloudhypervisor/snapshot_test.go @@ -1,6 +1,7 @@ package cloudhypervisor import ( + "encoding/json/jsontext" "encoding/json/v2" "errors" "net/http" @@ -85,6 +86,60 @@ func TestSnapshotPauseRefusesANICCountOffItsRecord(t *testing.T) { } } +func TestSnapshotPauseWaitsForAPendingNICEject(t *testing.T) { + tests := []struct { + name string + tree func(polls int) []string + wait time.Duration + }{ + {"nothing pending", func(int) []string { return []string{"cocoon-net-new"} }, 0}, + {"guest acks the eject", func(polls int) []string { + if polls < 4 { + return []string{"cocoon-net-new", "_net3"} + } + return []string{"cocoon-net-new"} + }, 300 * time.Millisecond}, + {"a node the guest never ejects stops at the cap", func(int) []string { return []string{"cocoon-net-new", "_net3"} }, ejectWaitTimeout}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + polls, pauses := 0, 0 + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/vm.info", func(w http.ResponseWriter, _ *http.Request) { + tree := map[string]jsontext.Value{} + for _, id := range tt.tree(polls) { + tree[id] = jsontext.Value(`{}`) + } + _ = json.MarshalWrite(w, chVMInfoResponse{ + State: chStateRunning, + Config: chVMConfig{Nets: []chNet{{ID: "cocoon-net-new"}}}, + DeviceTree: tree, + }) + polls++ + }) + mux.HandleFunc("/api/v1/vm.pause", func(w http.ResponseWriter, _ *http.Request) { + pauses++ + w.WriteHeader(http.StatusNoContent) + }) + hc := &http.Client{Transport: handlerTransport{mux}} + rec := &hypervisor.VMRecord{VM: types.VM{NetSetup: types.NetSetup{NetworkConfigs: []*types.NetworkConfig{{}}}}} + start := time.Now() + + if err := (&CloudHypervisor{}).snapshotSpec(t.Context()).Pause(rec, hc); err != nil { + t.Fatalf("Pause: %v", err) + } + if got := time.Since(start); got != tt.wait { + t.Errorf("paused after %s, want %s", got, tt.wait) + } + if pauses != 1 { + t.Errorf("pauses = %d, want 1", pauses) + } + }) + }) + } +} + func TestSnapshotPauseWaitsForBalloonToSettle(t *testing.T) { const ( mem = int64(8 << 30) From 6c464dbd815b70565a560801e9638890f185b956 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 21:08:05 +0800 Subject: [PATCH 22/25] test: use a promoted field in the NIC eject test's record literal --- hypervisor/cloudhypervisor/snapshot_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hypervisor/cloudhypervisor/snapshot_test.go b/hypervisor/cloudhypervisor/snapshot_test.go index 6e037c60..1a8ec4b9 100644 --- a/hypervisor/cloudhypervisor/snapshot_test.go +++ b/hypervisor/cloudhypervisor/snapshot_test.go @@ -123,7 +123,7 @@ func TestSnapshotPauseWaitsForAPendingNICEject(t *testing.T) { w.WriteHeader(http.StatusNoContent) }) hc := &http.Client{Transport: handlerTransport{mux}} - rec := &hypervisor.VMRecord{VM: types.VM{NetSetup: types.NetSetup{NetworkConfigs: []*types.NetworkConfig{{}}}}} + rec := &hypervisor.VMRecord{NetworkConfigs: []*types.NetworkConfig{{}}} start := time.Now() if err := (&CloudHypervisor{}).snapshotSpec(t.Context()).Pause(rec, hc); err != nil { From d7146d58f21b09710d44df12bc23d527894759d6 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 21:26:53 +0800 Subject: [PATCH 23/25] fix(doctor): the generated conflist does not leak a masquerade rule per VM cocoon-check --subnet wrote ipMasq true, the setting known-issues says leaks a POSTROUTING rule and a CNI chain for every VM, because the bridge plugin reads the addresses to remove off an interface cocoon has flushed. Generate ipMasq false and masquerade the subnet on the host once. --- doctor/check.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doctor/check.sh b/doctor/check.sh index 028dc611..0fe02475 100755 --- a/doctor/check.sh +++ b/doctor/check.sh @@ -130,7 +130,7 @@ generate_cni_conflist() { "bridge": "cni0", "mtu": ${host_mtu}, "isGateway": true, - "ipMasq": true, + "ipMasq": false, "hairpinMode": true, "ipam": { "type": "host-local", @@ -145,7 +145,9 @@ generate_cni_conflist() { ] } CNIEOF - fixed "generated $CNI_CONFLIST (subnet: ${subnet})" + iptables -t nat -C POSTROUTING -s "$subnet" ! -o cni0 -j MASQUERADE 2>/dev/null || + iptables -t nat -A POSTROUTING -s "$subnet" ! -o cni0 -j MASQUERADE + fixed "generated $CNI_CONFLIST (subnet: ${subnet}); persist the masquerade rule for ${subnet} with your firewall tooling" } # --------------------------------------------------------------------------- From bf83dbe4e2a99bd4bbcd45a7f392138a34f2d2af Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 21:51:14 +0800 Subject: [PATCH 24/25] fix(hypervisor): restore recovers the network before it places the VM again 3ff73bb8 moved placement first so the quiesce flag preceded the un-quiesce. Measured across commits on an idle node-pinned host, that order alone adds 6-7ms to every restore (p50 130 to 136, n=60, flag kept and order reverted measures 129.7), and hibernate wake is a latency path. A running origin already has the flag from the kill's commit, so this only reopens a 1-2ms gap for a stopped origin between un-quiesce and the placement commit. The window up to the VMM exec stays covered. --- hypervisor/restore.go | 8 ++++---- hypervisor/restore_test.go | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/hypervisor/restore.go b/hypervisor/restore.go index 46d31b69..df8585a7 100644 --- a/hypervisor/restore.go +++ b/hypervisor/restore.go @@ -141,15 +141,15 @@ func (b *Backend) restoreCore(ctx context.Context, run restoreRun) (*types.VM, e if err := run.apply(run.rec); err != nil { return err } + // Inside the ops lock, like StartSequence: a hibernate resume after a host reboot finds no plumbing left to un-quiesce. + if err := b.RecoverNetwork(ctx, run.rec); err != nil { + return fmt.Errorf("recover network: %w", err) + } placed, err := b.placeRecord(ctx, run.vmID, &run.vmCfg.Config) if err != nil { return fmt.Errorf("place VM: %w", err) } run.rec.CPUSet, run.rec.QueueCPUs = placed.CPUSet, placed.QueueCPUs - // Inside the ops lock, like StartSequence: a hibernate resume after a host reboot finds no plumbing left to un-quiesce. - if err := b.RecoverNetwork(ctx, run.rec); err != nil { - return fmt.Errorf("recover network: %w", err) - } var afterErr error result, afterErr = run.afterExtract(ctx, run.vmID, run.vmCfg, run.rec) return afterErr diff --git a/hypervisor/restore_test.go b/hypervisor/restore_test.go index 27e64723..8ef8ab39 100644 --- a/hypervisor/restore_test.go +++ b/hypervisor/restore_test.go @@ -152,7 +152,7 @@ func TestRunningRestoreFailureSchedulesNetworkConvergence(t *testing.T) { } } -func TestRestoreKeepsAQuiescePendingFromTheKillToTheVMM(t *testing.T) { +func TestRestoreHasAQuiescePendingBeforeTheVMM(t *testing.T) { tests := []struct { name string origin types.VMState @@ -183,8 +183,8 @@ func TestRestoreKeepsAQuiescePendingFromTheKillToTheVMM(t *testing.T) { } pending := func() bool { return recordOf(t, b, id).QuiescePending } b.SetNetwork(stubNetwork{recover: func(context.Context, *types.VM) error { - if !pending() { - t.Error("a kill during network recovery leaves no quiesce pending") + if got := pending(); got != tt.pendingInApply { + t.Errorf("pending during network recovery = %v, want %v", got, tt.pendingInApply) } return nil }}) @@ -201,6 +201,9 @@ func TestRestoreKeepsAQuiescePendingFromTheKillToTheVMM(t *testing.T) { return nil }, AfterExtract: func(context.Context, string, *types.VMConfig, *VMRecord) (*types.VM, error) { + if !pending() { + t.Error("a kill before the VMM runs leaves no quiesce pending") + } return nil, errors.New("stop after the window") }, }) From 7e20c1e489d6d7792a57bf47c227afebdfc57f0e Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 4 Nov 2026 22:09:41 +0800 Subject: [PATCH 25/25] fix(doctor): check the generated network's masquerade rule on every run d7146d58 added the rule once at generation and printed FIXED even when iptables failed. A later run skipped generation, and a reboot dropped the rule, so guests lost outbound NAT with no report. It is now checked like the FORWARD rules: reported on every run and added by --fix, so a rerun converges. --- doctor/check.sh | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doctor/check.sh b/doctor/check.sh index 0fe02475..9ccae019 100755 --- a/doctor/check.sh +++ b/doctor/check.sh @@ -145,9 +145,7 @@ generate_cni_conflist() { ] } CNIEOF - iptables -t nat -C POSTROUTING -s "$subnet" ! -o cni0 -j MASQUERADE 2>/dev/null || - iptables -t nat -A POSTROUTING -s "$subnet" ! -o cni0 -j MASQUERADE - fixed "generated $CNI_CONFLIST (subnet: ${subnet}); persist the masquerade rule for ${subnet} with your firewall tooling" + fixed "generated $CNI_CONFLIST (subnet: ${subnet})" } # --------------------------------------------------------------------------- @@ -446,6 +444,12 @@ else fi fi +if grep -q '"ipMasq": false' "$CNI_CONFLIST" 2>/dev/null; then + masq_subnet=$(sed -n 's/.*"subnet": "\([^"]*\)".*/\1/p' "$CNI_CONFLIST" | head -1) + check_iptables_rule "nat POSTROUTING MASQUERADE ${masq_subnet}" \ + POSTROUTING -t nat -s "$masq_subnet" ! -o cni0 -j MASQUERADE +fi + # --------------------------------------------------------------------------- # 8. CNI plugins # ---------------------------------------------------------------------------