review: one-line comment budget + modern-Go stragglers

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

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 {

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")

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 {

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

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 {

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-<name>.
+			// Record data disks carry CH auto ids — match serials too, else two devices race for one /dev/disk/by-id/virtio-<name>.
 			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")

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
 	}

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)
 		},

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"
 )
 

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)
 	}

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"`
 }
 

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 {

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()

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)