From c91ee0396ce872fe9e12c79250e6ce5b549c501e Mon Sep 17 00:00:00 2001 From: CMGS Date: Sat, 11 Jul 2026 14:17:13 +0800 Subject: [PATCH] review: enforce one-line comment budget; sweep modern-Go stragglers Apply the hard comment budget (one-line godoc, one-line WHY) across 45 files: condense multi-line justification narratives, drop step-narration comments. Replace three hand-rolled patterns with slices.ContainsFunc, max(), and a folded name-collection loop. No behavior change. --- cmd/vm/exec.go | 4 +--- cmd/vm/hibernate.go | 3 +-- cmd/vm/lifecycle.go | 11 +++-------- cmd/vm/reseed.go | 16 ++++----------- hypervisor/cloudhypervisor/clone.go | 10 +++------- hypervisor/cloudhypervisor/console.go | 3 +-- hypervisor/cloudhypervisor/extend.go | 26 ++++++------------------- hypervisor/cloudhypervisor/netresize.go | 3 +-- hypervisor/cloudhypervisor/restore.go | 3 +-- hypervisor/cloudhypervisor/utils.go | 3 +-- hypervisor/create.go | 3 +-- hypervisor/db.go | 5 ++--- hypervisor/firecracker/clone.go | 8 ++------ hypervisor/firecracker/cowlock.go | 8 ++------ hypervisor/firecracker/create.go | 10 +++------- hypervisor/firecracker/relay.go | 6 ++---- hypervisor/firecracker/restore.go | 3 +-- hypervisor/firecracker/snapshot.go | 3 +-- hypervisor/firecracker/start.go | 3 +-- hypervisor/gc.go | 6 ++---- hypervisor/inspect.go | 3 +-- hypervisor/restore.go | 20 ++++++------------- hypervisor/snapshot.go | 3 +-- hypervisor/stop.go | 13 +++---------- hypervisor/utils.go | 4 +--- images/cloudimg/import.go | 3 --- images/cloudimg/pull.go | 23 +++++++--------------- images/gc.go | 3 +-- images/index.go | 5 +---- images/oci/erofs.go | 15 ++++---------- images/oci/process.go | 11 +++-------- metadata/metadata.go | 3 +-- network/bridge/bridge_linux.go | 5 +---- network/bridge/gc_linux.go | 3 +-- network/bridge/gc_other.go | 3 +-- network/cni/lifecycle.go | 6 ++---- network/network.go | 3 +-- network/tap_linux.go | 6 ++---- network/utils.go | 4 +--- snapshot/localfile/gc.go | 3 +-- utils/atomic.go | 6 ++---- utils/file.go | 13 ++++--------- utils/httprange.go | 10 ++-------- utils/sparse_linux.go | 3 +-- utils/tar_sparse_linux.go | 3 +-- 45 files changed, 88 insertions(+), 223 deletions(-) diff --git a/cmd/vm/exec.go b/cmd/vm/exec.go index 9aa3a8a7..049d4cfe 100644 --- a/cmd/vm/exec.go +++ b/cmd/vm/exec.go @@ -58,9 +58,7 @@ func (h Handler) Exec(cmd *cobra.Command, args []string) error { } defer conn.Close() //nolint:errcheck - // Stdin is opt-in (docker semantics): a nil reader makes client.Run close - // the child's stdin immediately, so scripted callers feeding a shell over - // stdin don't have the rest of their script swallowed by the pump. + // Stdin is opt-in (docker semantics): a nil reader closes the child's stdin immediately so scripted callers don't have the rest of their script swallowed by the pump. var stdin io.Reader if interactive, _ := cmd.Flags().GetBool("interactive"); interactive { stdin = os.Stdin diff --git a/cmd/vm/hibernate.go b/cmd/vm/hibernate.go index 6709e139..37a7062c 100644 --- a/cmd/vm/hibernate.go +++ b/cmd/vm/hibernate.go @@ -12,8 +12,7 @@ import ( "github.com/cocoonstack/cocoon/types" ) -// Hibernate atomically snapshots a running VM and stops it; the snapshot -// point and the stop coincide, so `vm restore` resumes with nothing lost. +// Hibernate atomically snapshots a running VM and stops it; the snapshot point and the stop coincide, so `vm restore` resumes with nothing lost. func (h Handler) Hibernate(cmd *cobra.Command, args []string) error { ctx, conf, err := h.Init(cmd) if err != nil { diff --git a/cmd/vm/lifecycle.go b/cmd/vm/lifecycle.go index 1330b2e5..14ad0dc8 100644 --- a/cmd/vm/lifecycle.go +++ b/cmd/vm/lifecycle.go @@ -292,17 +292,14 @@ func (h Handler) recoverNetwork(ctx context.Context, conf *config.Config, hyper logger.Warnf(ctx, "recover network for VM %s: %v (start will fail)", vm.ID, recoverErr) } } - // Ops lock serializes the verify-and-rebuild: two concurrent starts would - // otherwise both see missing plumbing, double-ADD, and the loser's - // rollback DEL would tear down the winner's fresh network. + // Ops lock serializes verify-and-rebuild: two concurrent starts would both see missing plumbing, double-ADD, and the loser's rollback DEL would tear down the winner's network. if locker, ok := hyper.(hypervisor.Reserver); ok { unlock, lockErr := locker.LockVMOps(ctx, vm.ID) if lockErr != nil { logger.Warnf(ctx, "skip network recovery for VM %s: ops lock: %v", vm.ID, lockErr) continue } - // Recheck under the lock and rebuild from the fresh record: the - // pre-lock List snapshot may predate a completed rm or net resize. + // Recheck under the lock from the fresh record: the pre-lock List snapshot may predate a completed rm or net resize. if fresh, inspectErr := hyper.Inspect(ctx, vm.ID); inspectErr == nil { recoverOne(fresh) } @@ -313,9 +310,7 @@ func (h Handler) recoverNetwork(ctx context.Context, conf *config.Config, hyper } } -// applyStopFlags maps --force (-1 = immediate kill, docker-style; #82: FC -// guests without i8042 never answer CtrlAltDel) and --timeout onto the stop -// window for stop and rm; rm has no --timeout flag, that read no-ops. +// applyStopFlags maps --force (-1 = immediate kill; #82: FC guests without i8042 never answer CtrlAltDel) and --timeout onto the stop window; rm has no --timeout flag, that read no-ops. func applyStopFlags(conf *config.Config, cmd *cobra.Command) { force, _ := cmd.Flags().GetBool("force") timeout, _ := cmd.Flags().GetInt("timeout") diff --git a/cmd/vm/reseed.go b/cmd/vm/reseed.go index bb226337..4255cc91 100644 --- a/cmd/vm/reseed.go +++ b/cmd/vm/reseed.go @@ -35,17 +35,12 @@ func (h Handler) Reseed(cmd *cobra.Command, args []string) error { return reseedVM(ctx, vm, regenMachineID) } -// reseedAfterResume re-inspects then fires the best-effort reseed. Clone/Restore return a -// *types.VM built in-process that never passed through ToVM, so its VsockSocket is zero; -// pairing refresh with signal keeps a caller from silently no-op-ing on a stale value. +// reseedAfterResume re-inspects then fires the best-effort reseed: Clone/Restore return an in-process *types.VM whose VsockSocket is zero, and a stale value would silently no-op. func (h Handler) reseedAfterResume(ctx context.Context, hyper hypervisor.Hypervisor, vm *types.VM, regenMachineID bool) { signalReseed(ctx, refreshVM(ctx, hyper, vm), regenMachineID) } -// reseedVM pushes fresh entropy and a CRNG reseed order over vsock. Only a failed -// dial is retried — the guest agent re-listens shortly after a clone/restore resume; -// once a live agent answers, its reply (success, version-skew rejection, or failure) -// is final, so an old agent isn't billed the whole retry budget. +// reseedVM pushes fresh entropy and a CRNG reseed order over vsock; only a failed dial retries (the agent re-listens shortly after resume) — a live agent's reply is final. func reseedVM(ctx context.Context, vm *types.VM, regenMachineID bool) error { if vm.VsockSocket == "" { return fmt.Errorf("reseed: %w (recreate the VM to enable agent reseed)", ErrVsockNotConfigured) @@ -82,9 +77,7 @@ func reseedVM(ctx context.Context, vm *types.VM, regenMachineID bool) error { return fmt.Errorf("reseed: dial agent: %w", dialErr) } -// signalReseed is the non-fatal wrapper for clone/restore paths: it reports whether a reseed -// was attempted (false = skipped for a Windows guest or a legacy VM without vsock) and never -// fails the calling command on agent version skew. +// signalReseed is the non-fatal clone/restore wrapper: reports whether a reseed was attempted (false = Windows or no vsock) and never fails the calling command. func signalReseed(ctx context.Context, vm *types.VM, regenMachineID bool) bool { logger := log.WithFunc("cmd.vm.reseed") if vm.Config.Windows { @@ -102,8 +95,7 @@ func signalReseed(ctx context.Context, vm *types.VM, regenMachineID bool) bool { return true } -// refreshVM re-inspects to recover runtime-only fields (VsockSocket, PID, SocketPath) that -// ToVM sets but Clone/Restore's in-process return value lacks; keeps the original on error. +// refreshVM re-inspects to recover runtime-only fields Clone/Restore's in-process return value lacks; keeps the original on error. func refreshVM(ctx context.Context, hyper hypervisor.Hypervisor, vm *types.VM) *types.VM { info, err := hyper.Inspect(ctx, vm.ID) if err != nil { diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index b9c197b6..c69337e6 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -70,8 +70,7 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v if err != nil { return nil, err } - // The patch list is taken before the new --data-disk configs are appended: - // they are not in the snapshot's device tree and are hot-added post-restore. + // The patch list predates the new --data-disk configs: they are not in the snapshot's device tree and are hot-added post-restore. patchStorageConfigs := restorePatchStorageConfigs(storageConfigs, directBoot, vmCfg.Windows, hadCidataInSnapshot) newDataDisks, err := ch.prepareCloneDataDisks(ctx, vmID, vmCfg, storageConfigs) if err != nil { @@ -166,8 +165,7 @@ func (ch *CloudHypervisor) restoreAndResumeClone( } if !opts.directBoot && !opts.vmCfg.Windows && !opts.hadCidataInSnapshot { - // Select by role, not position: new --data-disk configs are appended after - // the cidata entry, so the last element is not necessarily cidata. + // Select by role, not position: new --data-disk configs land after the cidata entry, so the last element is not necessarily cidata. i := slices.IndexFunc(opts.storageConfigs, hasCidataRole) if i < 0 { return fmt.Errorf("vm.add-disk (cidata): missing storage config") @@ -188,9 +186,7 @@ func (ch *CloudHypervisor) restoreAndResumeClone( return nil } -// prepareCloneDataDisks creates the --data-disk files requested for a clone; -// a name colliding with an inherited disk's serial would overwrite its -// backing file in the clone run dir, hence the pre-create scan. +// prepareCloneDataDisks creates the clone's --data-disk files; the pre-create scan exists because a name colliding with an inherited serial would overwrite its backing file. func (ch *CloudHypervisor) prepareCloneDataDisks(ctx context.Context, vmID string, vmCfg *types.VMConfig, existing []*types.StorageConfig) ([]*types.StorageConfig, error) { if len(vmCfg.DataDisks) == 0 { return nil, nil diff --git a/hypervisor/cloudhypervisor/console.go b/hypervisor/cloudhypervisor/console.go index 8517e907..29d381b7 100644 --- a/hypervisor/cloudhypervisor/console.go +++ b/hypervisor/cloudhypervisor/console.go @@ -12,8 +12,7 @@ import ( "github.com/cocoonstack/cocoon/hypervisor" ) -// Console returns a bidirectional stream to the VM console: console.sock (UEFI) or the CH-allocated PTY (OCI). -// Caller closes the returned ReadWriteCloser. +// Console returns a caller-closed bidirectional stream to the VM console: console.sock (UEFI) or the CH-allocated PTY (OCI). func (ch *CloudHypervisor) Console(ctx context.Context, ref string) (io.ReadWriteCloser, error) { id, rec, err := ch.ResolveAndLoad(ctx, ref) if err != nil { diff --git a/hypervisor/cloudhypervisor/extend.go b/hypervisor/cloudhypervisor/extend.go index 4fbb4d1d..ccc07b23 100644 --- a/hypervisor/cloudhypervisor/extend.go +++ b/hypervisor/cloudhypervisor/extend.go @@ -41,9 +41,7 @@ func (ch *CloudHypervisor) DiskAttach(ctx context.Context, vmRef string, spec di return "", err } id := disk.DeriveID(spec.Name) - // The CH fork refuses disks without an explicit image_type; DirectIO/queue - // semantics must match create-path data disks, so the disk is built from - // the record reloaded under the ops lock (restore rewrites Config in there). + // 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). makeBody := func(rec *hypervisor.VMRecord) any { d := storageConfigToDisk(&types.StorageConfig{ Role: types.StorageRoleData, Path: path, Serial: spec.Name, RO: spec.ReadOnly, DirectIO: spec.DirectIO, @@ -56,8 +54,7 @@ func (ch *CloudHypervisor) DiskAttach(ctx context.Context, vmRef string, spec di if ex.ID == id { return fmt.Errorf("disk name %q already attached", spec.Name) } - // Record data disks carry CH auto ids — match serials too, else two - // devices race for one /dev/disk/by-id/virtio-. + // Record data disks carry CH auto ids — match serials too, else two devices race for one /dev/disk/by-id/virtio-. if ex.Serial == spec.Name { return fmt.Errorf("disk serial %q already used by disk %q", spec.Name, ex.ID) } @@ -199,11 +196,7 @@ func (ch *CloudHypervisor) DeviceList(ctx context.Context, vmRef string) ([]vfio }) } -// resolveExternalVolume canonicalizes path (EvalSymlinks also asserts existence) -// and refuses anything inside a cocoon-managed root: vm rm / GC delete those -// trees, breaking the never-deletes contract, and a symlink must not smuggle a -// managed path past the check. Returns the resolved path so the duplicate -// precheck and CH both see one canonical name per volume. +// 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. func (ch *CloudHypervisor) resolveExternalVolume(path string) (string, error) { resolved, err := filepath.EvalSymlinks(path) if err != nil { @@ -233,11 +226,7 @@ func (ch *CloudHypervisor) inspectRunning(ctx context.Context, vmRef string) (*h return hc, info, nil } -// lockedDeviceOp serializes device-set mutations per VM across processes and -// hands back the record and a vm.info snapshot taken UNDER the lock, so -// precheck-then-call is atomic against concurrent attach/detach and the -// record's Config can't be a pre-restore vintage (restore rewrites it while -// holding this lock). The flock dies with the process, so no stale locks. +// lockedDeviceOp serializes device-set mutations per VM and returns the record plus a vm.info snapshot taken under the lock, making precheck-then-call atomic against concurrent attach/detach/restore. func (ch *CloudHypervisor) lockedDeviceOp(ctx context.Context, vmRef string) (*http.Client, hypervisor.VMRecord, *chVMInfoResponse, func(), error) { hc, vmID, _, err := ch.runningVMClientWithRecord(ctx, vmRef) if err != nil { @@ -320,9 +309,7 @@ func (ch *CloudHypervisor) detachWith( if err := removeDeviceVM(ctx, hc, deviceID); err != nil { return fmt.Errorf("vm.remove-device %s: %w", deviceID, err) } - // Block until the guest acks the ACPI eject (B0EJ): only then has CH freed - // the slot, the id, and the backing file — a caller reusing either right - // after detach must not race a still-live device (Windows can take 10-20s). + // Block until the guest acks the ACPI eject (B0EJ): only then has CH freed the slot, id, and backing file (Windows can take 10-20s). if err := waitDeviceEjected(ctx, hc, deviceID); err != nil { return fmt.Errorf("device %s removal initiated but the guest has not ejected it: %w", deviceID, err) } @@ -349,8 +336,7 @@ func (ch *CloudHypervisor) runningVMClientWithRecord(ctx context.Context, vmRef return utils.NewSocketHTTPClient(sockPath), vmID, rec, nil } -// ensureNotPaused refuses device-set mutations while a capture window is open -// (snapshot/hibernate/fork): mutating mid-capture would desync config and memory. +// ensureNotPaused refuses device-set mutations while a capture window is open — mutating mid-capture would desync config and memory. func ensureNotPaused(info *chVMInfoResponse) error { if info.State == chStatePaused { return fmt.Errorf("vm is paused (snapshot or hibernate in flight); retry after it completes") diff --git a/hypervisor/cloudhypervisor/netresize.go b/hypervisor/cloudhypervisor/netresize.go index d9852f1d..33d5c6c5 100644 --- a/hypervisor/cloudhypervisor/netresize.go +++ b/hypervisor/cloudhypervisor/netresize.go @@ -33,8 +33,7 @@ func (ch *CloudHypervisor) NetResize(ctx context.Context, vmRef string, spec net return netresize.Result{}, err } defer unlock() - // Reload under the lock: a resize that won the lock first may have - // changed the NIC set after this one loaded the record. + // Reload under the lock: a resize that won the lock first may have changed the NIC set after this one loaded the record. if rec, err = ch.LoadRecord(ctx, vmID); err != nil { return netresize.Result{}, err } diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index 9b5217ef..78b6a637 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -22,8 +22,7 @@ func (ch *CloudHypervisor) Restore(ctx context.Context, vmRef string, vmCfg *typ SourceSnapshotID: sourceSnapshotID, Preflight: ch.preflightRestore, Kill: ch.killForRestore, - // Same sweep as DirectRestore's Populate: stale snapshot files (data-*.raw, - // memory ranges) from a previous incarnation must not survive the merge. + // Same sweep as DirectRestore's Populate: stale snapshot files from a previous incarnation must not survive the merge. BeforeMerge: func(rec *hypervisor.VMRecord) error { return cleanSnapshotFiles(rec.RunDir) }, diff --git a/hypervisor/cloudhypervisor/utils.go b/hypervisor/cloudhypervisor/utils.go index 3ddd0c6e..8f308c67 100644 --- a/hypervisor/cloudhypervisor/utils.go +++ b/hypervisor/cloudhypervisor/utils.go @@ -29,8 +29,7 @@ const ( chAPIBase = "http://localhost/api/v1/" - // chMemoryRestoreOnDemand uses userfaultfd (UFFD) to lazily page in - // guest memory from the snapshot file, avoiding a full upfront copy. + // chMemoryRestoreOnDemand lazily pages in guest memory via userfaultfd instead of a full upfront copy. chMemoryRestoreOnDemand chMemoryRestoreMode = "OnDemand" ) diff --git a/hypervisor/create.go b/hypervisor/create.go index 2ca94d9a..2448cfb8 100644 --- a/hypervisor/create.go +++ b/hypervisor/create.go @@ -134,8 +134,7 @@ func (b *Backend) reservePlaceholder(ctx context.Context, id string, vmCfg *type logDir = b.Conf.VMLogDir(id) cleanup = func() { - // Record first: dir removal deletes the held ops.lock inode, and a - // concurrent rm on the recreated file must not find a live placeholder. + // Record first: dir removal deletes the held ops.lock inode, and a concurrent rm on the recreated file must not find a live placeholder. b.RollbackCreate(ctx, id, vmCfg.Name) _ = RemoveVMDirs(runDir, logDir) } diff --git a/hypervisor/db.go b/hypervisor/db.go index fb2b27cf..e5feb517 100644 --- a/hypervisor/db.go +++ b/hypervisor/db.go @@ -7,8 +7,7 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -// VMRecord is the persisted record for a single VM. -// JSON tags live on the embedded types.VM — duplicating them here would shadow the promoted fields. +// VMRecord is the persisted record for a single VM; JSON tags live on the embedded types.VM (duplicates would shadow the promoted fields). type VMRecord struct { types.VM @@ -27,7 +26,7 @@ type VMRecord struct { type VMIndex struct { VMs map[string]*VMRecord `json:"vms"` Names map[string]string `json:"names"` // name → VM ID - // OrphanDirs are migrated VM dirs whose delete removed the record but failed the dir removal; GC retries them (the orphan scan only covers the configured roots). + // OrphanDirs are migrated VM dirs whose delete removed the record but failed the dir removal; GC retries them since the orphan scan only covers configured roots. OrphanDirs []string `json:"orphan_dirs,omitempty"` } diff --git a/hypervisor/firecracker/clone.go b/hypervisor/firecracker/clone.go index 019892c5..399507f4 100644 --- a/hypervisor/firecracker/clone.go +++ b/hypervisor/firecracker/clone.go @@ -133,9 +133,7 @@ func (fc *Firecracker) restoreAndResumeClone( if err = resumeVM(ctx, hc); err != nil { return fmt.Errorf("resume: %w", err) } - // Re-anchor redirected drives at the clone's own paths: the loaded - // vmstate still names the source's, and any future snapshot of this VM - // would embed those dangling paths — breaking its restore (hibernate). + // Re-anchor redirected drives at the clone's own paths: the loaded vmstate still names the source's, and a future snapshot would embed those dangling paths, breaking its restore. for _, i := range redirectedDriveIndices(srcConfigs, dstConfigs) { if err = patchDrivePath(ctx, hc, fmt.Sprintf(driveIDFmt, i), dstConfigs[i].Path); err != nil { return fmt.Errorf("re-anchor drive %d: %w", i, err) @@ -179,9 +177,7 @@ func rebuildCloneStorage(meta *hypervisor.SnapshotMeta, cowPath string) ([]*type return configs, nil } -// redirectedDriveIndices lists the drives whose source and clone paths -// differ: exactly the set createDriveRedirects symlinks and the re-anchor -// loop patches — the two must never diverge, so both derive from here. +// redirectedDriveIndices lists drives whose source and clone paths differ — the one source of truth for both createDriveRedirects and the re-anchor loop, which must never diverge. func redirectedDriveIndices(srcConfigs, dstConfigs []*types.StorageConfig) []int { var indices []int for i, src := range srcConfigs { diff --git a/hypervisor/firecracker/cowlock.go b/hypervisor/firecracker/cowlock.go index 5a29d263..b5b415fa 100644 --- a/hypervisor/firecracker/cowlock.go +++ b/hypervisor/firecracker/cowlock.go @@ -8,10 +8,7 @@ import ( "github.com/cocoonstack/cocoon/types" ) -// withSourceWritableDisksLocked locks the source VM's writable disks (COW + data) in sorted order so concurrent clones can't deadlock. -// Each acquire runs recoverStaleBackup to finish any interrupted prior swap. -// Lock files sit next to the disks and die with the VM dir; the fresh-inode -// window after a concurrent rm is closed by ensureSourceAlive under the lock. +// withSourceWritableDisksLocked locks the source's writable disks in sorted order (deadlock-free), self-healing interrupted swaps on acquire; the fresh-inode window after a concurrent rm is closed by ensureSourceAlive under the lock. func (*Firecracker) withSourceWritableDisksLocked(ctx context.Context, configs []*types.StorageConfig, fn func() error) error { paths := make([]string, 0, len(configs)) for _, sc := range configs { @@ -38,8 +35,7 @@ func withCOWPathLocked(ctx context.Context, cowPath string, fn func() error) err if lockErr := l.Lock(ctx); lockErr != nil { return lockErr } - // Do NOT remove the lock file after unlock — flock synchronizes on - // the inode, not the pathname. + // Do NOT remove the lock file after unlock — flock synchronizes on the inode, not the pathname. defer func() { _ = l.Unlock(ctx) }() return fn() diff --git a/hypervisor/firecracker/create.go b/hypervisor/firecracker/create.go index 310f7f75..87640ca0 100644 --- a/hypervisor/firecracker/create.go +++ b/hypervisor/firecracker/create.go @@ -144,7 +144,9 @@ func decompressKernel(data []byte) ([]byte, error) { {"gzip", []byte{0x1f, 0x8b, 0x08}, decompressGzip}, } + var tried []string for _, f := range formats { + tried = append(tried, f.name) offset := bytes.Index(data, f.magic) if offset < 0 { continue @@ -155,11 +157,6 @@ func decompressKernel(data []byte) ([]byte, error) { } return decompressed, nil } - - tried := make([]string, len(formats)) - for i, f := range formats { - tried[i] = f.name - } return nil, fmt.Errorf("no supported compression format found (tried %s)", strings.Join(tried, ", ")) } @@ -169,8 +166,7 @@ func decompressZstd(data []byte) ([]byte, error) { return nil, fmt.Errorf("new zstd reader: %w", err) } defer dec.Close() - // DecodeAll may error on trailing data after the first frame; any prefix - // already in out is valid — caller validates via ELF magic. + // DecodeAll may error on trailing data after the first frame; any decoded prefix is valid — caller validates via ELF magic. out, err := dec.DecodeAll(data, nil) if len(out) == 0 { return nil, fmt.Errorf("zstd decode: %w", err) diff --git a/hypervisor/firecracker/relay.go b/hypervisor/firecracker/relay.go index 94b56a0b..8a00c061 100644 --- a/hypervisor/firecracker/relay.go +++ b/hypervisor/firecracker/relay.go @@ -26,8 +26,7 @@ const ( relayBufSize = 4096 ) -// broadcaster fans out PTY master reads to the active console session. -// Only one session is active at a time; the subscriber is swapped atomically. +// broadcaster fans out PTY master reads to the single active console session, swapped atomically. type broadcaster struct { master io.Reader mu sync.Mutex @@ -64,8 +63,7 @@ func IsRelayMode() bool { return os.Getenv(relayEnvKey) == "1" } -// RunRelay runs the console relay loop. Inherits fd 3 (PTY master), fd 4 (console.sock listener), $_COCOON_FC_PID. -// A single persistent goroutine reads the PTY and broadcasts to the active session so disconnects don't strand readers. +// RunRelay runs the console relay loop over inherited fd 3 (PTY master), fd 4 (console.sock listener), and $_COCOON_FC_PID; one persistent PTY reader broadcasts so disconnects don't strand readers. func RunRelay(ctx context.Context) { master := os.NewFile(relayMasterFD, "pty-master") defer master.Close() //nolint:errcheck diff --git a/hypervisor/firecracker/restore.go b/hypervisor/firecracker/restore.go index 72919043..5089f20b 100644 --- a/hypervisor/firecracker/restore.go +++ b/hypervisor/firecracker/restore.go @@ -24,8 +24,7 @@ func (fc *Firecracker) Restore(ctx context.Context, vmRef string, vmCfg *types.V Wrap: func(rec *hypervisor.VMRecord, inner func() error) error { return fc.wrapSourceLocked(ctx, rec, inner) }, - // Same sweep as DirectRestore's Populate: stale vmstate/mem/data-*.raw - // from a previous incarnation must not survive the merge. + // Same sweep as DirectRestore's Populate: stale snapshot files from a previous incarnation must not survive the merge. BeforeMerge: func(rec *hypervisor.VMRecord) error { return cleanSnapshotFiles(rec.RunDir) }, diff --git a/hypervisor/firecracker/snapshot.go b/hypervisor/firecracker/snapshot.go index 9fd86b6c..7fa27660 100644 --- a/hypervisor/firecracker/snapshot.go +++ b/hypervisor/firecracker/snapshot.go @@ -35,8 +35,7 @@ func (fc *Firecracker) snapshotSpec(ctx context.Context) hypervisor.SnapshotSpec return hypervisor.SnapshotSpec{ Pause: func(_ *hypervisor.VMRecord, hc *http.Client) error { return pauseVM(ctx, hc) }, Resume: func(_ *hypervisor.VMRecord, hc *http.Client) error { return resumeVM(context.WithoutCancel(ctx), hc) }, - // createSnapshotFC builds its own client with VMMemTransferTimeout - // (multi-GiB memory transfer); it cannot share hc. + // createSnapshotFC builds its own client with VMMemTransferTimeout (multi-GiB memory transfer); it cannot share hc. Capture: func(rec *hypervisor.VMRecord, _ *http.Client, tmpDir string) error { sockPath := hypervisor.SocketPath(rec.RunDir) if err := createSnapshotFC(ctx, sockPath, tmpDir); err != nil { diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index 352b2c25..26383fc9 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -171,8 +171,7 @@ func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMReco go func() { _ = fcCmd.Wait() - // If relay failed, master fd was kept open to preserve ttyS0. - // Close it now that FC has exited to avoid permanent fd leak. + // A failed relay kept master open to preserve ttyS0; close it now that FC exited or the fd leaks forever. if relayErr != nil { _ = master.Close() } diff --git a/hypervisor/gc.go b/hypervisor/gc.go index 7a61788d..f4d8b925 100644 --- a/hypervisor/gc.go +++ b/hypervisor/gc.go @@ -135,8 +135,7 @@ func (b *Backend) gcCollect(ctx context.Context, ids []string, snap VMGCSnapshot logger := log.WithFunc("gc." + b.Typ) errs := b.sweepStaleCaptureDirs(ctx, snap.sweepDirs(b.Conf.RunDir())) errs = append(errs, b.sweepOrphanDirs(ctx, snap.orphanDirs)...) - // Only fully-reclaimed ids lose their DB record: unrecording a skipped VM - // would strand a live VMM/dirs with no owner and let network GC tear it down. + // Only fully-reclaimed ids lose their DB record: unrecording a skipped VM would strand a live VMM/dirs with no owner and let network GC tear it down. safeToUnrecord := make([]string, 0, len(ids)) for _, id := range ids { runDir, logDir := b.Conf.VMRunDir(id), b.Conf.VMLogDir(id) @@ -146,8 +145,7 @@ func (b *Backend) gcCollect(ctx context.Context, ids []string, snap VMGCSnapshot } return nil }) - // Ops lock excludes in-flight owners: a create pre-locks and mkdirs the run - // dir before its DB record lands, so an unlocked "orphan" may be seconds old. + // 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. ok := b.withOpsTryLock(ctx, runDir, func() { // Fail closed: deleting sockets/disks under a still-live VMM corrupts it. if err := b.ensureOrphanVMMDead(ctx, runDir); err != nil { diff --git a/hypervisor/inspect.go b/hypervisor/inspect.go index 4477b866..93a99fd8 100644 --- a/hypervisor/inspect.go +++ b/hypervisor/inspect.go @@ -107,8 +107,7 @@ func (b *Backend) UpdateRecord(ctx context.Context, vmID string, mutate func(*VM }) } -// SetRunningSockets fills a running VM's live sockets (CH API socket, vsock UDS -// when bound) from runDir — for clone/restore records that skip ToVM. +// SetRunningSockets fills a running VM's live sockets (API socket, bound vsock UDS) from runDir — for clone/restore records that skip ToVM. func SetRunningSockets(info *types.VM, runDir string) { info.SocketPath = SocketPath(runDir) if p := VsockSockPath(runDir); isVsockBound(p) { diff --git a/hypervisor/restore.go b/hypervisor/restore.go index 63e1e71c..3b39a38b 100644 --- a/hypervisor/restore.go +++ b/hypervisor/restore.go @@ -19,8 +19,7 @@ import ( func (b *Backend) KillForRestore(ctx context.Context, vmID string, rec *VMRecord, terminate func(pid int) error, runtimeFiles []string) error { killErr := b.WithRunningVM(ctx, rec, terminate) if killErr != nil && !errors.Is(killErr, ErrNotRunning) { - // A stopped origin has no live VMM, so a transient liveness-scan error - // here must not brick the wake — nothing was mutated, retry converges. + // A stopped origin has no live VMM: a transient liveness-scan error must not brick the wake — nothing was mutated, retry converges. b.FailRestore(ctx, vmID, rec.State) return fmt.Errorf("stop running VM: %w", killErr) } @@ -28,10 +27,7 @@ func (b *Backend) KillForRestore(ctx context.Context, vmID string, rec *VMRecord return nil } -// FailRestore marks the VM error after a restore-path failure; a stopped -// origin is spared so hibernate wake stays retryable. Run-dir-mutating steps -// (staged merge, direct populate) quarantine unconditionally at their own -// site; origin is the pre-kill state — the DB may read stopped after the kill. +// FailRestore marks the VM error after a restore failure; a stopped origin (the pre-kill state) is spared so hibernate wake stays retryable — run-dir-mutating steps quarantine at their own site. func (b *Backend) FailRestore(ctx context.Context, vmID string, origin types.VMState) { if origin == types.VMStateStopped { return @@ -110,15 +106,13 @@ func (b *Backend) RestoreSequence(ctx context.Context, vmRef string, spec Restor } if spec.BeforeMerge != nil { if err := spec.BeforeMerge(rec); err != nil { - // The sweep may have deleted some snapshot files already; a - // stopped origin would otherwise stay startable on mixed state. + // The sweep may already have deleted snapshot files; a stopped origin would otherwise stay startable on mixed state. b.QuarantineVM(ctx, vmID, "partial restore cleanup") 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 leaves mixed-vintage files in the run dir; quarantine regardless of origin so vm start cannot boot them. b.QuarantineVM(ctx, vmID, "partial snapshot merge") return fmt.Errorf("apply staged snapshot: %w", mergeErr) } @@ -160,8 +154,7 @@ func (b *Backend) DirectRestoreSequence(ctx context.Context, vmRef string, spec return err } 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 cleans then clones with no rollback; a partial run dir must quarantine regardless of origin, like the merge. b.QuarantineVM(ctx, vmID, "partial restore populate") return populateErr } @@ -191,8 +184,7 @@ func (b *Backend) prepareRestore(ctx context.Context, vmRef string) (string, *VM unlock() return "", nil, nil, err } - // Revalidate under the lock: 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, and preflight anchors the external trust set to it. vmID, rec, err := b.ResolveForRestore(ctx, vmID) if err != nil { return fail(err) diff --git a/hypervisor/snapshot.go b/hypervisor/snapshot.go index 123e8c36..2135c001 100644 --- a/hypervisor/snapshot.go +++ b/hypervisor/snapshot.go @@ -270,8 +270,7 @@ func IsUnderDir(path, dir string) bool { return strings.HasPrefix(cleaned, root+string(filepath.Separator)) } -// ValidateMetaPaths rejects dereferenced sidecar paths escaping cocoon-managed roots; an imported snapshot's cocoon.json is otherwise untrusted. -// Snapshot-resident disks (COW/cidata/data) are exempt: their bytes travel inside the snapshot and the recorded path is provenance only — restore and clone resolve them from the record or rewrite them — so a --run-dir migration must not reject the VM's own history. Layers and boot files are dereferenced from the blob store and stay strict. +// ValidateMetaPaths rejects dereferenced sidecar/boot paths escaping cocoon-managed roots (an imported cocoon.json is untrusted); snapshot-resident disks are exempt — their recorded path is provenance only, resolved from the record or rewritten. func ValidateMetaPaths(meta *SnapshotMeta, rootDir, runDir string) error { for i, sc := range meta.StorageConfigs { if sc == nil { diff --git a/hypervisor/stop.go b/hypervisor/stop.go index eea105e2..6a032d41 100644 --- a/hypervisor/stop.go +++ b/hypervisor/stop.go @@ -125,8 +125,7 @@ func (b *Backend) deleteOneLocked(ctx context.Context, id string, force bool, st return fmt.Errorf("refuse delete: api socket %s still responsive (suspected orphan vmm; kill the vmm process then retry)", sockPath) } for _, pid := range procScan.Find(sockPath) { - // procScan is a pre-stop snapshot: a just-force-stopped VM is already gone. - // Only a still-live match is a real orphan worth killing and logging. + // procScan predates the stop: only a still-live match is a real orphan worth killing. if !utils.IsProcessAlive(pid) { continue } @@ -139,15 +138,9 @@ func (b *Backend) deleteOneLocked(ctx context.Context, id string, force bool, st shape metering.Shape hadRunningInterval bool ) - // Dirs outside the configured roots (--run-dir migration) escape the GC - // orphan scan; persist a cleanup intent in the same transaction so a - // failed dir removal below stays reclaimable after the record is gone. + // Dirs outside the configured roots escape the GC orphan scan; persist a cleanup intent in the same transaction so a failed removal stays reclaimable. migrated := migratedDirs(rec, b.Conf.VMRunDir(id), b.Conf.VMLogDir(id)) - // Record goes first: dir removal deletes the ops.lock file, and a later - // locker on the recreated file is a fresh inode that does not contend — - // with the record already gone its resolve fails instead of reviving the - // VM. Capture in the same transaction so a concurrent UpdateStates can't - // shift the truth; a failed dir removal below leaves orphans for GC. + // Record first: dir removal deletes the ops.lock inode, and a fresh-inode locker must resolve a gone record instead of reviving the VM. if err := b.DB.Update(ctx, func(idx *VMIndex) error { r := idx.VMs[id] if r == nil { diff --git a/hypervisor/utils.go b/hypervisor/utils.go index 4ad23f03..2fd631a2 100644 --- a/hypervisor/utils.go +++ b/hypervisor/utils.go @@ -50,9 +50,7 @@ const ( // SnapshotFileKind classifies a snapshot file for CloneSnapshotFiles. type SnapshotFileKind int -// LockVMOps serializes mutating verbs on one VM across processes (#103): -// device attach/detach, net resize, snapshot, hibernate, restore, stop. -// The flock dies with the process, so a crashed holder never wedges the VM. +// LockVMOps serializes mutating verbs on one VM across processes (#103); the flock dies with the holder, so a crash never wedges the VM. func (b *Backend) LockVMOps(ctx context.Context, vmID string) (func(), error) { runDir := b.Conf.VMRunDir(vmID) // The record's persisted RunDir wins: after a --run-dir migration the paths differ and two lock files would let ops interleave. diff --git a/images/cloudimg/import.go b/images/cloudimg/import.go index 869ee3ab..80748103 100644 --- a/images/cloudimg/import.go +++ b/images/cloudimg/import.go @@ -33,7 +33,6 @@ func importQcow2File(ctx context.Context, conf *Config, store storage.Store[imag return fmt.Errorf("import %s: %w", filePath, err) } - // First pass: hash the source file. h := sha256.New() if _, err = io.Copy(h, srcFile); err != nil { return fmt.Errorf("hash %s: %w", filePath, err) @@ -41,7 +40,6 @@ func importQcow2File(ctx context.Context, conf *Config, store storage.Store[imag digestHex := hex.EncodeToString(h.Sum(nil)) logger.Debugf(ctx, "hashed %s -> sha256:%s", filePath, digestHex[:12]) - // Cached fast path: just add the ref. if utils.ValidFile(conf.BlobPath(digestHex)) { if err = commit(ctx, conf, store, name, tracker, "", digestHex); err != nil { return err @@ -50,7 +48,6 @@ func importQcow2File(ctx context.Context, conf *Config, store storage.Store[imag return nil } - // Second pass: copy into a cocoon temp file. if _, err = srcFile.Seek(0, io.SeekStart); err != nil { return fmt.Errorf("seek %s: %w", filePath, err) } diff --git a/images/cloudimg/pull.go b/images/cloudimg/pull.go index b0f4ccbc..16496964 100644 --- a/images/cloudimg/pull.go +++ b/images/cloudimg/pull.go @@ -32,8 +32,7 @@ const ( progressInterval = 1 << 20 ) -// progressCounter emits PhaseDownload events every ~1 MiB; mutex-guarded so it -// serves both the serial writer and parallel range workers. +// progressCounter emits PhaseDownload events every ~1 MiB; mutex-guarded so it serves both the serial writer and parallel range workers. type progressCounter struct { mu sync.Mutex written int64 @@ -51,8 +50,7 @@ func (pc *progressCounter) add(n int64) { } done := pc.written pc.mu.Unlock() - // Emit outside the lock: the tracker callback is user-supplied and must not - // serialize the range workers. + // Emit outside the lock: the tracker callback is user-supplied and must not serialize the range workers. if report { pc.tracker.OnEvent(cloudimgProgress.Event{ Phase: cloudimgProgress.PhaseDownload, @@ -84,7 +82,6 @@ type rangeProbe struct { func pull(ctx context.Context, conf *Config, store storage.Store[imageIndex], url string, force bool, tracker progress.Tracker) error { logger := log.WithFunc("cloudimg.pull") - // URL-level idempotency check (skipped when force is true). if !force { var skip bool if err := store.With(ctx, func(idx *imageIndex) error { @@ -137,8 +134,7 @@ func withDownload( return fn(tmpFile, tmpPath, digestHex) } -// downloadToFile probes whether url supports HTTP Range requests and downloads it into dst -// accordingly: pullConns concurrent range connections when supported, a single stream otherwise. +// downloadToFile downloads url into dst: pullConns concurrent Range connections when supported, a single stream otherwise. func downloadToFile(ctx context.Context, url string, dst *os.File, tracker progress.Tracker, pullConns int) (string, error) { logger := log.WithFunc("cloudimg.downloadToFile") client := &http.Client{Timeout: urlDownloadTimeout} @@ -162,8 +158,7 @@ func downloadToFile(ctx context.Context, url string, dst *os.File, tracker progr return hashDigest(dst) } -// downloadSerial streams url into dst over a single connection; used when the server doesn't -// support Range requests or doesn't report a usable size. +// downloadSerial streams url into dst over one connection when the server lacks Range support or a usable size. func downloadSerial(ctx context.Context, client *http.Client, url string, dst *os.File, tracker progress.Tracker) (string, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -206,9 +201,7 @@ func downloadSerial(ctx context.Context, client *http.Client, url string, dst *o return hex.EncodeToString(h.Sum(nil)), nil } -// probeRangeSupport issues a GET with Range: bytes=0-0; nil means no usable Range support -// (fall back to serial). The probe URL is resp.Request.URL (post-redirect) so each range -// request hits the resolved location without re-following the redirect chain. +// probeRangeSupport GETs Range: bytes=0-0 (nil = no usable Range support); the returned URL is post-redirect so range requests skip the redirect chain. func probeRangeSupport(ctx context.Context, client *http.Client, url string) (*rangeProbe, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -232,8 +225,7 @@ func probeRangeSupport(ctx context.Context, client *http.Client, url string) (*r return &rangeProbe{finalURL: resp.Request.URL.String(), size: size}, nil } -// downloadRangesParallel splits [0,size) into pullConns contiguous ranges and downloads them -// concurrently into disjoint offsets of dst. +// downloadRangesParallel splits [0,size) into pullConns contiguous ranges downloaded concurrently into disjoint offsets of dst. func downloadRangesParallel(ctx context.Context, client *http.Client, url string, dst *os.File, size int64, pullConns int, tracker progress.Tracker) error { if err := dst.Truncate(size); err != nil { return fmt.Errorf("truncate temp file: %w", err) @@ -275,8 +267,7 @@ func downloadRange(ctx context.Context, client *http.Client, url string, dst *os return utils.CopyRangeBody(resp, w, start, end) } -// hashDigest re-reads dst from disk: scattered parallel writes can't feed a streaming -// hasher, and hashing the file itself verifies what actually landed. +// hashDigest re-reads dst from disk: scattered parallel writes can't feed a streaming hasher, and hashing the file verifies what actually landed. func hashDigest(dst *os.File) (string, error) { if _, err := dst.Seek(0, io.SeekStart); err != nil { return "", fmt.Errorf("seek temp file: %w", err) diff --git a/images/gc.go b/images/gc.go index 037f101c..225dbc8e 100644 --- a/images/gc.go +++ b/images/gc.go @@ -82,8 +82,7 @@ func BuildGCModule[I any](cfg GCModuleConfig[I]) gc.Module[ImageGCSnapshot] { } } -// gcStaleTemp removes temp entries older than StaleTempAge; dirOnly=true skips files. -// .lock files are never removed — flock syncs on inode, so deleting one races with a current holder. +// gcStaleTemp removes temp entries older than StaleTempAge (dirOnly=true skips files); .lock files stay — flock syncs on inode, so deleting one races a current holder. func gcStaleTemp(ctx context.Context, dir string, dirOnly bool) []error { cutoff := time.Now().Add(-utils.StaleTempAge) return utils.RemoveMatching(ctx, dir, func(e os.DirEntry) bool { diff --git a/images/index.go b/images/index.go index 508595dd..21dd731c 100644 --- a/images/index.go +++ b/images/index.go @@ -48,10 +48,7 @@ func ReferencedDigests[E Entry](images map[string]*E) map[string]struct{} { return refs } -// LookupOne resolves id to a single entry via LookupRefs' matching rules (exact -// ref, normalizers, digest exact/prefix). Multiple refs are fine only while they -// name the same image (tag aliases of one digest); a prefix spanning distinct -// digests resolves to nothing rather than to map-iteration luck. +// 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. func LookupOne[E Entry](images map[string]*E, id string, normalizers ...func(string) (string, bool)) (string, *E, bool) { refs := LookupRefs(images, id, normalizers...) if len(refs) == 0 { diff --git a/images/oci/erofs.go b/images/oci/erofs.go index 96382939..414500b4 100644 --- a/images/oci/erofs.go +++ b/images/oci/erofs.go @@ -16,9 +16,7 @@ const ( erofsBlockSize = 4096 erofsCompression = "lz4hc" - // erofs-utils 1.7.x tar mode writes corrupt compressed clusters for some - // inputs and still exits 0; the corruption surfaces only as EUCLEAN at - // read time (#94). 1.8 is the first safe series. + // erofs-utils < 1.8 tar mode writes corrupt compressed clusters yet exits 0, surfacing only as EUCLEAN at read time (#94). erofsMinMajor = 1 erofsMinMinor = 8 ) @@ -61,9 +59,7 @@ func startErofsConversion(ctx context.Context, uuid, outputPath string) (cmd *ex return cmd, stdin, output, nil } -// runErofsConversion streams src into mkfs.erofs at outPath while scanning for boot files under scanDir. -// The scan→drain→close→wait order is load-bearing: mkfs.erofs (and any hasher upstream of src) must -// see the full stream before Wait, and stdin must close before Wait or mkfs.erofs blocks. +// runErofsConversion streams src into mkfs.erofs while scanning boot files; the scan→drain→close→wait order is load-bearing (full stream before Wait, stdin closed or mkfs.erofs blocks). func runErofsConversion(ctx context.Context, src io.Reader, scanDir, namePrefix, uuid, outPath string) (kernelPath, initrdPath string, err error) { cmd, stdin, output, err := startErofsConversion(ctx, uuid, outPath) if err != nil { @@ -88,9 +84,7 @@ func runErofsConversion(ctx context.Context, src io.Reader, scanDir, namePrefix, return kernelPath, initrdPath, nil } -// checkErofsVersion refuses conversion when mkfs.erofs predates the floor. -// Only success is cached: a transient probe failure or an in-place -// erofs-utils upgrade is re-probed on the next conversion. +// checkErofsVersion refuses conversion when mkfs.erofs predates the floor; only success is cached so transient probe failures re-probe on the next conversion. func checkErofsVersion(ctx context.Context) error { erofsCheckMu.Lock() defer erofsCheckMu.Unlock() @@ -108,8 +102,7 @@ func checkErofsVersion(ctx context.Context) error { return nil } -// erofsVersionAtLeast parses the leading "X.Y" from `mkfs.erofs --version` -// output ("mkfs.erofs (erofs-utils) 1.8.10") and compares it to the floor. +// erofsVersionAtLeast parses the leading "X.Y" from `mkfs.erofs --version` output and compares it to the floor. func erofsVersionAtLeast(output string) error { m := erofsVersionRe.FindStringSubmatch(output) if m == nil { diff --git a/images/oci/process.go b/images/oci/process.go index 64380502..fa7027a2 100644 --- a/images/oci/process.go +++ b/images/oci/process.go @@ -51,8 +51,7 @@ func processLayer(ctx context.Context, j layerJob) error { logger.Debugf(ctx, "Layer %d: sha256:%s -> erofs (single-pass)", j.idx, digestHex[:12]) - // Hoisted out of the retried region: a too-old mkfs.erofs is permanent, - // retrying it would burn attempts, backoff sleeps, and aborted blob GETs. + // Hoisted out of the retried region: a too-old mkfs.erofs is permanent — retrying burns attempts, backoff sleeps, and aborted blob GETs. if err = checkErofsVersion(ctx); err != nil { return err } @@ -81,9 +80,7 @@ func processLayer(ctx context.Context, j layerJob) error { return nil } -// convertLayer downloads and converts one layer into a fresh layerDir; the -// remote stream cannot resume, so each attempt redoes the whole layer and -// must not see a previous attempt's partial erofs or scanned boot files. +// convertLayer downloads and converts one layer into a fresh layerDir; the remote stream cannot resume, so each attempt redoes the layer and must not see a prior attempt's partial output. func convertLayer(ctx context.Context, j layerJob, layerDir, digestHex, layerUUID, erofsPath string) (kernelPath, initrdPath string, err error) { if err = os.RemoveAll(layerDir); err != nil { return "", "", fmt.Errorf("reset layer work dir: %w", err) @@ -118,9 +115,7 @@ func applyCachedLayerPaths(conf *Config, result *pullLayerResult, digestHex stri } } -// retryLayer runs fn up to attempts times, sleeping backoff between failures; -// every error retries — a mid-stream break is indistinguishable from bad input -// by the time mkfs or the boot scan reports it. +// retryLayer runs fn up to attempts times with backoff; every error retries — a mid-stream break is indistinguishable from bad input by the time mkfs reports it. func retryLayer(ctx context.Context, attempts int, backoff time.Duration, fn func() error) error { var err error for attempt := 1; ; attempt++ { diff --git a/metadata/metadata.go b/metadata/metadata.go index 71fbf174..3fb68ef7 100644 --- a/metadata/metadata.go +++ b/metadata/metadata.go @@ -78,8 +78,7 @@ write_files: {{- end}} `)) - // networkConfigTmpl renders cloud-init network-config (netplan v2 → systemd-networkd). - // Clone reinit fallback for netplan PERM-MAC mismatch is wired in via user-data write_files. + // networkConfigTmpl renders cloud-init network-config (netplan v2); the clone-reinit fallback for netplan PERM-MAC mismatch is wired via user-data write_files. networkConfigTmpl = template.Must(template.New("network-config").Parse(`version: 2 ethernets: {{- range $i, $n := .Networks}} diff --git a/network/bridge/bridge_linux.go b/network/bridge/bridge_linux.go index de61b44d..78b91f3a 100644 --- a/network/bridge/bridge_linux.go +++ b/network/bridge/bridge_linux.go @@ -101,10 +101,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: the DB says this index is free (caller holds the - // ops lock), so a same-name TAP is an interrupted-resize leftover and - // without the reclaim it wedges every retry here. Recovery specs keep - // the EEXIST failure — their slot is occupied, possibly by a live VMM's TAP. + // Fresh adds only: a same-name TAP is an interrupted-resize leftover that would wedge every retry; recovery specs keep the EEXIST failure (their slot may hold a live VMM's TAP). if spec.Existing == nil { if old, lErr := netlink.LinkByName(name); lErr == nil { if delErr := netlink.LinkDel(old); delErr != nil { diff --git a/network/bridge/gc_linux.go b/network/bridge/gc_linux.go index a87dccab..012a7019 100644 --- a/network/bridge/gc_linux.go +++ b/network/bridge/gc_linux.go @@ -25,8 +25,7 @@ type bridgeSnapshot struct { prefixes map[string]struct{} } -// GCModule returns a GC module that reclaims orphan bt* TAP devices. -// It does not require a Bridge instance — only rootDir for the lock file. +// GCModule returns a GC module reclaiming orphan bt* TAP devices; it needs no Bridge instance, only rootDir for the lock file. func GCModule(rootDir string) gc.Module[bridgeSnapshot] { lockPath := filepath.Join(rootDir, "bridge", "gc.lock") _ = utils.EnsureDirs(filepath.Dir(lockPath)) diff --git a/network/bridge/gc_other.go b/network/bridge/gc_other.go index 28f3f9d8..e1630253 100644 --- a/network/bridge/gc_other.go +++ b/network/bridge/gc_other.go @@ -16,8 +16,7 @@ type bridgeSnapshot struct{} func GCModule(rootDir string) gc.Module[bridgeSnapshot] { return gc.Module[bridgeSnapshot]{ Name: "bridge", - // /dev/null is world-writable and supports flock on all Unix platforms, - // so TryLock always succeeds without creating a real lock file. + // /dev/null supports flock everywhere, so TryLock always succeeds without creating a real lock file. Locker: flock.New("/dev/null"), ReadDB: func(_ context.Context) (bridgeSnapshot, error) { return bridgeSnapshot{}, nil diff --git a/network/cni/lifecycle.go b/network/cni/lifecycle.go index 9a64e1cc..7a2d0dde 100644 --- a/network/cni/lifecycle.go +++ b/network/cni/lifecycle.go @@ -179,8 +179,7 @@ func (c *CNI) Remove(ctx context.Context, vmID string, indices ...int) error { for _, i := range indices { wanted[fmt.Sprintf("eth%d", i)] = true } - // Pick every matching record, not one per ifname: a failed reclaim can leave - // duplicates, and DEL is idempotent — skipping one would strand it as a phantom. + // Pick every matching record, not one per ifname: a failed reclaim can leave duplicates, and DEL is idempotent — skipping one would strand a phantom. picked := make([]networkRecord, 0, len(indices)) found := make(map[string]bool, len(indices)) for _, r := range records { @@ -208,8 +207,7 @@ func (c *CNI) stageNICIntents(ctx context.Context, confList *libcni.NetworkConfi } ifName := fmt.Sprintf("eth%d", spec.Index) if rec, ok := stale[ifName]; ok { - // The index is reusable only after a full reclaim: proceeding would double-allocate - // on lenient IPAM plugins or bury the root cause under the ADD failure on strict ones. + // The index is reusable only after a full reclaim: proceeding would double-allocate on lenient IPAM plugins or bury the root cause on strict ones. if rcErr := c.reclaimStaleNIC(ctx, vmID, nsPath, rec); rcErr != nil { return nil, fmt.Errorf("reclaim stale NIC %s/%s: %w", vmID, ifName, rcErr) } diff --git a/network/network.go b/network/network.go index c19f45c6..196f7819 100644 --- a/network/network.go +++ b/network/network.go @@ -13,8 +13,7 @@ var ErrNotConfigured = errors.New("network provider not configured") // AddSpec is one NIC's add request; Existing != nil reuses MAC/IP for recovery. type AddSpec struct { Index int - // Queues is the TAP queue count; 0 derives NetNumQueues(vmCfg.CPU). Callers with a - // backend-specific size (FC opens the TAP single-queue) set it explicitly. + // Queues is the TAP queue count; 0 derives NetNumQueues(vmCfg.CPU), backend-specific callers (FC opens single-queue) set it explicitly. Queues int Existing *types.NetworkConfig } diff --git a/network/tap_linux.go b/network/tap_linux.go index 9b4e8fec..45f4831a 100644 --- a/network/tap_linux.go +++ b/network/tap_linux.go @@ -9,12 +9,10 @@ import ( ) const ( - // tapTxQueueLen absorbs traffic bursts (especially UDP) without - // dropping; the kernel default of 1000 is too small for VM workloads. + // tapTxQueueLen absorbs traffic bursts (especially UDP) without dropping; the kernel default of 1000 is too small for VM workloads. tapTxQueueLen = 10000 - // groMaxSize matches the maximum virtio-net segment size, allowing - // the kernel to aggregate inbound packets before CH reads them. + // groMaxSize matches the maximum virtio-net segment size so the kernel aggregates inbound packets before CH reads them. groMaxSize = 65536 ) diff --git a/network/utils.go b/network/utils.go index b6e9288c..939a2fcd 100644 --- a/network/utils.go +++ b/network/utils.go @@ -14,8 +14,7 @@ const ( NetQueueSize = 512 ) -// NetNumQueues returns the virtio-net queue count for the given CPU count. -// CH uses queue pairs (TX+RX), so the result is always even (≥ 2). +// NetNumQueues returns the virtio-net queue count for cpu; CH uses TX+RX pairs, so the result is always even (>= 2). func NetNumQueues(cpu int) int { if cpu <= 1 { return 2 //nolint:mnd @@ -24,7 +23,6 @@ func NetNumQueues(cpu int) int { } // ResolveQueueSize returns qs if non-zero, otherwise the default NetQueueSize. -// Negative values aren't reachable from validated callers. func ResolveQueueSize(qs int) int { return cmp.Or(qs, NetQueueSize) } diff --git a/snapshot/localfile/gc.go b/snapshot/localfile/gc.go index 19fca395..292d1853 100644 --- a/snapshot/localfile/gc.go +++ b/snapshot/localfile/gc.go @@ -169,8 +169,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker emits = append(emits, id) } } - // Emit only after the record deletion lands: a persistently failing DB - // would otherwise re-candidate these ids and double-close the interval. + // Emit only after the record deletion lands: a persistently failing DB would re-candidate these ids and double-close the interval. if err := cleanResolvedRecords(store, removed); err != nil { errs = append(errs, fmt.Errorf("clean DB records: %w", err)) } else { diff --git a/utils/atomic.go b/utils/atomic.go index f4d12c01..6ee6eb83 100644 --- a/utils/atomic.go +++ b/utils/atomic.go @@ -23,8 +23,7 @@ func AtomicWriteFile(path string, data []byte, perm os.FileMode) error { return atomicWriteFile(path, data, perm, true) } -// AtomicWriteFileNoSync is AtomicWriteFile without the fsyncs, for transient -// run-dir files regenerated on the next launch. +// AtomicWriteFileNoSync is AtomicWriteFile without fsyncs, for transient run-dir files regenerated on the next launch. func AtomicWriteFileNoSync(path string, data []byte, perm os.FileMode) error { return atomicWriteFile(path, data, perm, false) } @@ -34,8 +33,7 @@ func AtomicWriteJSON(path string, v any) error { return atomicWriteJSON(path, v, true) } -// AtomicWriteJSONNoSync marshals v and writes it atomically without fsync -// (see AtomicWriteFileNoSync). +// AtomicWriteJSONNoSync marshals v and writes it atomically without fsync (see AtomicWriteFileNoSync). func AtomicWriteJSONNoSync(path string, v any) error { return atomicWriteJSON(path, v, false) } diff --git a/utils/file.go b/utils/file.go index c88cf73f..df7cb9e1 100644 --- a/utils/file.go +++ b/utils/file.go @@ -8,6 +8,7 @@ import ( "io/fs" "os" "path/filepath" + "slices" "strings" "syscall" "time" @@ -127,16 +128,10 @@ func FilterUnreferenced(candidates []string, refs map[string]struct{}, exclude . if _, ok := refs[s]; ok { continue } - excluded := false - for _, ex := range exclude { - if _, ok := ex[s]; ok { - excluded = true - break - } - } - if !excluded { - out = append(out, s) + if slices.ContainsFunc(exclude, func(ex map[string]struct{}) bool { _, ok := ex[s]; return ok }) { + continue } + out = append(out, s) } return out } diff --git a/utils/httprange.go b/utils/httprange.go index fdf3db9d..4a380f01 100644 --- a/utils/httprange.go +++ b/utils/httprange.go @@ -9,9 +9,7 @@ import ( // SplitRanges divides [0,size) into up to n contiguous, inclusive-ended byte ranges. func SplitRanges(size int64, n int) [][2]int64 { - if n < 1 { - n = 1 - } + n = max(n, 1) chunk := (size + int64(n) - 1) / int64(n) ranges := make([][2]int64, 0, n) for start := int64(0); start < size; start += chunk { @@ -24,11 +22,7 @@ func SplitRanges(size int64, n int) [][2]int64 { return ranges } -// CopyRangeBody validates resp as the answer to a bytes=start-end Range request -// (206 status, matching Content-Range span, full-length body) and copies exactly -// the requested bytes into w. A mismatched span would land bytes at the wrong -// offsets; a short body would leave a zero hole — both would otherwise surface -// only when a later digest pass re-hashes the assembled file. +// CopyRangeBody validates resp against the requested bytes=start-end range (status, span, length) and copies it into w — a mismatch would otherwise surface only at the later digest pass. func CopyRangeBody(resp *http.Response, w io.Writer, start, end int64) error { if resp.StatusCode != http.StatusPartialContent { return fmt.Errorf("range %d-%d: unexpected status %s", start, end, resp.Status) diff --git a/utils/sparse_linux.go b/utils/sparse_linux.go index 9cb161ea..1a746667 100644 --- a/utils/sparse_linux.go +++ b/utils/sparse_linux.go @@ -15,8 +15,7 @@ const ( seekHole = 4 // SEEK_HOLE ) -// SparseCopy copies src to dst preserving sparsity via SEEK_HOLE/SEEK_DATA. -// dst is created as a new file (truncated to src size, then only data segments written). +// 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 { return copyWithCleanup(dst, src, func(srcFile, dstFile *os.File) error { fi, err := srcFile.Stat() diff --git a/utils/tar_sparse_linux.go b/utils/tar_sparse_linux.go index a3b61e5a..2f8ee848 100644 --- a/utils/tar_sparse_linux.go +++ b/utils/tar_sparse_linux.go @@ -14,8 +14,7 @@ import ( // Sparse-map JSON cap (margin under tar's 1MB PAX block); var so tests can override. var maxSparseMapJSONSize = 800 * 1024 -// tarFileMaybeSparse writes file as COCOON.sparse PAX when it has holes (SEEK_HOLE/SEEK_DATA). -// Falls back to a regular tar entry on empty files, unsupported FS, no holes, or oversized segment map. +// 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. func tarFileMaybeSparse(tw *tar.Writer, path, nameInTar string) error { f, err := os.Open(path) //nolint:gosec if err != nil {