diff --git a/cgroup/cgroup.go b/cgroup/cgroup.go index ec0092c3..a79934f8 100644 --- a/cgroup/cgroup.go +++ b/cgroup/cgroup.go @@ -303,7 +303,7 @@ func placeScope(parentDir, dir, cpuset string) error { } } } - if err := checkSubset(cpuset, parentDir, "--cpuset-cpus"); err != nil { + if err := checkSubset(cpuset, parentDir, "cpuset_cpus placement"); err != nil { return err } return writeControl(dir, cpusetName, cpuset) diff --git a/cmd/core/gc.go b/cmd/core/gc.go index ae8503b5..0e814c71 100644 --- a/cmd/core/gc.go +++ b/cmd/core/gc.go @@ -7,7 +7,6 @@ import ( "github.com/cocoonstack/cocoon/gc" "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/lock/vmlock" - "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/network/bridge" "github.com/cocoonstack/cocoon/snapshot/localfile" ) @@ -40,7 +39,7 @@ func NewGCOrchestrator(ctx context.Context, conf *config.Config, snapOpts ...loc } gc.Register(o, hypervisor.CgroupGCModule(conf.CgroupParentDir())) netProvider.RegisterGC(o) - gc.Register(o, bridge.GCModule(network.BridgeTAPPrefix(conf.NetScope))) + gc.Register(o, bridge.GCModule(conf.BridgeTAPPrefix())) gc.Register(o, vmlock.GCModule(conf.RootDir)) snapBackend.RegisterGC(o) return o, nil diff --git a/cmd/core/init.go b/cmd/core/init.go index f43cdfd2..7bb2cdcb 100644 --- a/cmd/core/init.go +++ b/cmd/core/init.go @@ -172,7 +172,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; backends build lazily, GC-path only. -func pinnedElsewhere(conf *config.Config) func(context.Context) (map[string]struct{}, error) { +func pinnedElsewhere(conf *config.Config) imagebackend.PinRecheck { type pinner interface { PinnedBlobIDs(context.Context) (map[string]struct{}, error) } diff --git a/cmd/core/metastore_test.go b/cmd/core/metastore_test.go index 94f40501..486da822 100644 --- a/cmd/core/metastore_test.go +++ b/cmd/core/metastore_test.go @@ -10,8 +10,7 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -// A failed engine open must leave the interface nil so CloseMetaStore's nil -// check holds — a typed-nil store would panic at command teardown. +// A failed engine open must leave the interface nil so CloseMetaStore's nil check holds — a typed-nil store would panic at command teardown. func TestMetaStoreOpenErrorThenCloseNoPanic(t *testing.T) { resetMetaStore() conf := testConf(t) diff --git a/cmd/core/network.go b/cmd/core/network.go index b8a96159..9706610f 100644 --- a/cmd/core/network.go +++ b/cmd/core/network.go @@ -96,7 +96,7 @@ func (n *NetProviders) Quiesce(ctx context.Context, vm *types.VM) error { // A partial CNI failure leaves the tombstone for retry or GC to resume. func (n *NetProviders) Cleanup(ctx context.Context, vmID string) error { - bridgenet.CleanupTAPs(network.BridgeTAPPrefix(n.conf.NetScope), []string{vmID}) + bridgenet.CleanupTAPs(n.conf.BridgeTAPPrefix(), []string{vmID}) p, err := n.cniOnly() if err != nil { // Lazy CNI; OK to skip for bridge-only setups. diff --git a/cmd/core/utils.go b/cmd/core/utils.go index ff4d0bb7..319492f2 100644 --- a/cmd/core/utils.go +++ b/cmd/core/utils.go @@ -167,7 +167,7 @@ func EnsureSnapshotNameFree(ctx context.Context, snapBackend snapshot.Snapshot, if name == "" { return nil } - // The name index, not Inspect: a save killed mid-flight leaves a pending record still holding the name, which Inspect reports as not-found. Passing that preflight means the whole capture is written before the insert rejects it. + // 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. if nh, ok := snapBackend.(snapshot.NameHolder); ok { id, held, err := nh.NameOwner(ctx, name) if err != nil { @@ -311,7 +311,6 @@ func digestPullRef(image, digest, imageType string) string { if digest == "" || imageType != types.ImageTypeOCI { return image } - // OCI: convert "registry/repo:tag" → "registry/repo@sha256:..." ref, err := name.ParseReference(image) if err != nil { return image diff --git a/cmd/core/utils_test.go b/cmd/core/utils_test.go index 141a1cf1..b58152cc 100644 --- a/cmd/core/utils_test.go +++ b/cmd/core/utils_test.go @@ -200,9 +200,7 @@ func TestPersistSnapshotDirCleansCaptureOnDirectError(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 is a DirectCreator whose CreateFromDir always fails; the embedded interface panics on any other call, so the test proves the failure path alone. type directErrSnap struct { snapshot.Snapshot } diff --git a/cmd/core/vmconfig.go b/cmd/core/vmconfig.go index cf246f3d..b693d28d 100644 --- a/cmd/core/vmconfig.go +++ b/cmd/core/vmconfig.go @@ -38,10 +38,6 @@ func VMConfigFromFlags(cmd *cobra.Command, image string) (*types.VMConfig, error mergeable, _ := cmd.Flags().GetBool("mergeable") dataDiskRaw, _ := cmd.Flags().GetStringArray("data-disk") - if mergeable && (hugePages || sharedMemory) { - return nil, fmt.Errorf("--mergeable needs plain private memory; drop --hugepages/--shared-memory") - } - vmName = cmp.Or(vmName, sanitizeVMName(image)) memBytes, err := units.RAMInBytes(memStr) @@ -212,10 +208,7 @@ func sanitizeVMName(image string) string { n := strings.ReplaceAll(image, "/", "-") n = strings.ReplaceAll(n, ":", "-") n = "cocoon-" + n - if len(n) > 63 { - n = n[:63] - } - return n + return n[:min(len(n), 63)] } repo := strings.TrimPrefix(ref.Context().RepositoryStr(), "library/") @@ -226,10 +219,7 @@ func sanitizeVMName(image string) string { n += "-" + tag.TagStr() } - if len(n) > 63 { - n = n[:63] - } - return n + return n[:min(len(n), 63)] } // parseDataDiskFlags parses --data-disk values, normalizes defaults, and returns the spec list ready for hypervisor.PrepareDataDisks. diff --git a/cmd/meta/convert/convert_test.go b/cmd/meta/convert/convert_test.go index 518811fa..81c5c376 100644 --- a/cmd/meta/convert/convert_test.go +++ b/cmd/meta/convert/convert_test.go @@ -294,8 +294,7 @@ func TestDistinctGenerationsRoundTrip(t *testing.T) { t.Fatalf("convert back to json: %v", err) } - // Corrupt the new main: the served generation must be the imported one, - // never a stranded pre-conversion .prev (§9). + // Corrupt the new main: the served generation must be the imported one, never a stranded pre-conversion .prev (§9). main, err := os.ReadFile(spec.JSON[0].FilePath) if err != nil { t.Fatal(err) @@ -313,8 +312,7 @@ func TestDistinctGenerationsRoundTrip(t *testing.T) { } } -// TestUncheckpointedWALReverseConversion: a commit stranded in the WAL by a -// killed process must survive the checkpoint-then-aside reverse conversion (§9). +// TestUncheckpointedWALReverseConversion: a commit stranded in the WAL by a killed process must survive the checkpoint-then-aside reverse conversion (§9). func TestUncheckpointedWALReverseConversion(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -364,8 +362,7 @@ func TestUncheckpointedWALReverseConversion(t *testing.T) { } } -// TestWALWriterWorker commits one durable record, acks, and hangs until killed -// so the WAL is never checkpointed by a clean close. +// TestWALWriterWorker commits one durable record, acks, and hangs until killed so the WAL is never checkpointed by a clean close. func TestWALWriterWorker(t *testing.T) { db := os.Getenv("META_MP_DB") if db == "" { @@ -387,8 +384,7 @@ func TestWALWriterWorker(t *testing.T) { select {} //nolint:staticcheck // hang until SIGKILL keeps the WAL un-checkpointed } -// TestConvertSkipsNeverWrittenNamespace: a namespace whose lock dir was -// never created (subsystem never ran) must not fail the quiesce probe. +// TestConvertSkipsNeverWrittenNamespace: a namespace whose lock dir was never created (subsystem never ran) must not fail the quiesce probe. func TestConvertSkipsNeverWrittenNamespace(t *testing.T) { spec := testSpec(t, "vms") seedJSON(t, spec, "vms") diff --git a/cmd/vm/attach.go b/cmd/vm/attach.go index 5d158d36..16d03b8d 100644 --- a/cmd/vm/attach.go +++ b/cmd/vm/attach.go @@ -101,7 +101,6 @@ func resolveAttacher[A any](h Handler, cmd *cobra.Command, args []string, op str return ctx, conf, hyper, a, nil } -// classifyAttachErr surfaces ErrNotRunning more clearly than the generic wrap. func classifyAttachErr(err error) error { if errors.Is(err, hypervisor.ErrNotRunning) { return fmt.Errorf("vm is not running: %w", err) diff --git a/cmd/vm/debug.go b/cmd/vm/debug.go index 143e3f6a..a03b2a9f 100644 --- a/cmd/vm/debug.go +++ b/cmd/vm/debug.go @@ -1,6 +1,7 @@ package vm import ( + "cmp" "fmt" "os" "slices" @@ -147,7 +148,7 @@ func buildCHDebugSpec(cmd *cobra.Command, conf *config.Config, storageConfigs [] case balloon == 0: balloon = int(size >> 20) //nolint:mnd } - allowed := cgroup.EffectiveCPUs(vmCfg.CPUSetCPUs, conf.CgroupCPUs) + allowed := cgroup.EffectiveCPUs(vmCfg.CPUSetCPUs, conf.CgroupCPUFence()) return chDebugSpec{ Configs: storageConfigs, Boot: boot, @@ -166,9 +167,7 @@ func printCHDebug(s chDebugSpec) { noDirectIO := s.VMCfg.NoDirectIO if hypervisor.IsDirectBoot(s.Boot) { - if s.CowPath == "" { - s.CowPath = fmt.Sprintf("cow-%s.raw", s.VMCfg.Name) - } + s.CowPath = cmp.Or(s.CowPath, fmt.Sprintf("cow-%s.raw", s.VMCfg.Name)) debugConfigs := slices.Concat(s.Configs, []*types.StorageConfig{ {Path: s.CowPath, RO: false, Serial: hypervisor.CowSerial}, }) @@ -189,9 +188,7 @@ func printCHDebug(s chDebugSpec) { fmt.Print(" \\\n") fmt.Printf(" --cmdline \"%s\" \\\n", cmdline) } else { - if s.CowPath == "" { - s.CowPath = fmt.Sprintf("cow-%s.qcow2", s.VMCfg.Name) - } + s.CowPath = cmp.Or(s.CowPath, fmt.Sprintf("cow-%s.qcow2", s.VMCfg.Name)) basePath := s.Configs[0].Path fmt.Println("# Prepare COW overlay") fmt.Printf("qemu-img create -f qcow2 -F qcow2 -b %s %s\n", basePath, s.CowPath) diff --git a/cmd/vm/exec_test.go b/cmd/vm/exec_test.go index 7f60e38c..781fb91c 100644 --- a/cmd/vm/exec_test.go +++ b/cmd/vm/exec_test.go @@ -79,8 +79,7 @@ func TestReadHybridVsockReply(t *testing.T) { // TestDialHybridVsock_ConnectHandshake drives the CH/FC hybrid vsock dialect (CONNECT \n → OK \n) against an in-process listener. func TestDialHybridVsock_ConnectHandshake(t *testing.T) { - // macOS caps unix socket paths at ~104 bytes, so t.TempDir() (long - // /var/folders/... path) can overflow. Use os.CreateTemp + immediate unlink. + // macOS caps unix socket paths at ~104 bytes and t.TempDir() can overflow it, so use os.CreateTemp + immediate unlink. f, err := os.CreateTemp("", "vsock-*.uds") if err != nil { t.Fatalf("create temp: %v", err) diff --git a/cmd/vm/logstream.go b/cmd/vm/logstream.go index 518b8f9d..863353ec 100644 --- a/cmd/vm/logstream.go +++ b/cmd/vm/logstream.go @@ -3,8 +3,10 @@ package vm import ( "bytes" "context" + "errors" "fmt" "io" + "io/fs" "os" "github.com/cocoonstack/cocoon/utils" @@ -13,7 +15,7 @@ import ( func streamLog(ctx context.Context, path string, follow bool, tail int) error { f, err := os.Open(path) //nolint:gosec if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("open log %s: VM may not have been started yet", path) } return fmt.Errorf("open log: %w", err) diff --git a/cmd/vm/run.go b/cmd/vm/run.go index ef424e79..c0836474 100644 --- a/cmd/vm/run.go +++ b/cmd/vm/run.go @@ -1,6 +1,7 @@ package vm import ( + "cmp" "context" "fmt" "time" @@ -323,9 +324,7 @@ func (h Handler) prepareClone(ctx context.Context, cmd *cobra.Command, conf *con return cloneSetup{}, err } vmID := utils.GenerateID() - if vmCfg.Name == "" { - vmCfg.Name = "cocoon-clone-" + network.VMIDPrefix(vmID) - } + vmCfg.Name = cmp.Or(vmCfg.Name, "cocoon-clone-"+network.VMIDPrefix(vmID)) if err = vmCfg.Validate(); err != nil { return cloneSetup{}, err } diff --git a/cmd/vm/status.go b/cmd/vm/status.go index 13c93ec5..a2fcb646 100644 --- a/cmd/vm/status.go +++ b/cmd/vm/status.go @@ -20,6 +20,7 @@ import ( "github.com/cocoonstack/cocoon/config" "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/types" + "github.com/cocoonstack/cocoon/utils" ) type vmEvent struct { @@ -54,9 +55,7 @@ func (h Handler) Status(cmd *cobra.Command, args []string) error { ctx, conf := h.Init(cmd) interval, _ := cmd.Flags().GetInt("interval") - if interval <= 0 { - interval = 5 //nolint:mnd - } + interval = utils.OrDefault(interval, 5) //nolint:mnd eventMode, _ := cmd.Flags().GetBool("event") watchMode, _ := cmd.Flags().GetBool("watch") if eventMode && watchMode { diff --git a/config/config.go b/config/config.go index c046a1b6..802027cc 100644 --- a/config/config.go +++ b/config/config.go @@ -50,8 +50,7 @@ type Config struct { PoolSize int `json:"pool_size" mapstructure:"pool_size"` // PullConns: concurrent HTTP Range connections per cloud-image download; <=0 = 8. PullConns int `json:"pull_conns" mapstructure:"pull_conns"` - // MetaBackend selects the metadata engine: "json" or "sqlite"; empty - // auto-resolves (an existing store binds its engine, fresh roots get sqlite). + // MetaBackend selects the metadata engine: "json" or "sqlite"; empty auto-resolves (an existing store binds its engine, fresh roots get sqlite). MetaBackend string `json:"meta_backend,omitempty" mapstructure:"meta_backend"` // CNIConfDir: CNI plugin configuration dir. Default: /etc/cni/net.d. CNIConfDir string `json:"cni_conf_dir" mapstructure:"cni_conf_dir"` @@ -92,6 +91,12 @@ func (c *Config) EffectivePullConns() int { return min(utils.OrDefault(c.PullConns, defaultPullConns), maxPullConns) } +// BridgeTAPPrefix returns this installation's host-side bridge TAP name prefix. +func (c *Config) BridgeTAPPrefix() string { return network.BridgeTAPPrefix(c.NetScope) } + +// NetnsPrefix returns this installation's per-VM CNI netns name prefix. +func (c *Config) NetnsPrefix() string { return network.NetnsPrefix(c.NetScope) } + // CgroupParentDir returns the absolute cgroup v2 directory holding per-VM CPU scopes. func (c *Config) CgroupParentDir() string { return filepath.Join(cgroup.Root, cmp.Or(c.CgroupParent, cgroup.DefaultParent)) @@ -100,8 +105,7 @@ func (c *Config) CgroupParentDir() string { // CgroupCPUFence returns the configured fence cpu list (empty = all cores). func (c *Config) CgroupCPUFence() string { return c.CgroupCPUs } -// Validate checks that all config fields are within acceptable ranges. -// Should be called once at startup after unmarshalling. +// Validate checks that all config fields are within acceptable ranges; call once at startup after unmarshalling. func (c *Config) Validate() error { if c.RootDir == "" { return fmt.Errorf("root_dir must not be empty") diff --git a/daemon/reconcile_test.go b/daemon/reconcile_test.go index 5479cb9f..7872eef1 100644 --- a/daemon/reconcile_test.go +++ b/daemon/reconcile_test.go @@ -51,8 +51,7 @@ func TestReconcileSkipsInconclusiveLiveness(t *testing.T) { } } -// A tombstoned record is not supervised as a live VM, but the daemon starts -// deletes of its own, so it must finish one a crash left mid-protocol. +// A tombstoned record is not supervised as a live VM, but the daemon starts deletes of its own, so it must finish one a crash left mid-protocol. func TestReconcileResumesInterruptedDelete(t *testing.T) { f := newFake().put(runningRec("vm1", 1)) f.tombstoned["vm1"] = struct{}{} diff --git a/docs/index.md b/docs/index.md index 606b21c5..0deefda5 100644 --- a/docs/index.md +++ b/docs/index.md @@ -4,8 +4,9 @@ Lightweight MicroVM engine with dual hypervisor backends: [Cloud Hypervisor](https://github.com/cloud-hypervisor/cloud-hypervisor) (default) and [Firecracker](https://github.com/firecracker-microvm/firecracker). -One hypervisor process per VM, no daemon; Docker-like CLI; snapshots that -clone into new identities in tens of milliseconds. +One hypervisor process per VM, every command standalone (a resident daemon +is optional); Docker-like CLI; snapshots that clone into new identities in +tens of milliseconds. ``` cocoon CLI ──► images: OCI (EROFS layers, direct boot) | cloudimg (qcow2, UEFI) @@ -47,7 +48,7 @@ cocoon CLI ──► images: OCI (EROFS layers, direct boot) | cloudimg (qcow2, - **Image import** — import local qcow2 or tar files (also from stdin or gzip-wrapped streams), auto-detected by magic bytes - **UEFI boot** — CLOUDHV.fd firmware by default; direct kernel boot for OCI images (auto-detected) - **COW overlays** — copy-on-write disks backed by shared base images (raw for OCI, qcow2 for cloud images) -- **CNI networking** — automatic NIC creation via CNI plugins, multi-NIC support, per-VM IP allocation +- **CNI networking** — automatic NIC creation via CNI plugins, multi-NIC support, per-VM IP allocation; bridge mode and NIC hot-resize; `net_scope` keys host device names per installation so co-hosted cocoon roots never GC each other's guests - **CPU isolation** — every VM runs in its own cgroup v2 scope with Guaranteed-at-N defaults (`--cpu` is a hard cap); raw weight/quota/burst knobs, an optional host-core fence (`cgroup_cpus`), and per-VM pinning (`--cpuset-cpus`); see [CPU Isolation](vm.md#cpu-isolation-cgroup-v2) - **Multi-queue virtio-net** — TAP devices created with per-vCPU queue pairs; configurable ring depth (`--queue-size`, default 512); TSO/UFO/csum offload enabled by default - **TC redirect I/O path** — veth ↔ TAP wired via ingress qdisc + mirred redirect (no bridge in the data path) @@ -56,6 +57,7 @@ cocoon CLI ──► images: OCI (EROFS layers, direct boot) | cloudimg (qcow2, - **User data disks** — `--data-disk` attaches additional virtio-blk disks per VM, with optional ext4 mkfs at create time, cloud-init `mounts:` auto-mount on cloudimg+CH (via `/dev/disk/by-id/virtio-`), per-disk DirectIO override, and 1:1 inheritance through snapshot/clone/restore; `vm clone --data-disk` adds fresh disks to a clone and `vm disk attach/detach` hot-plugs existing raw files on a running VM (both CH only) - **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 - **Graceful shutdown** — ACPI power-button for UEFI VMs with configurable timeout, fallback to SIGTERM → SIGKILL - **Interactive console** — `cocoon vm console` with bidirectional PTY relay, SSH-style escape sequences (`~.` disconnect, `~?` help), configurable escape character, SIGWINCH propagation @@ -67,7 +69,7 @@ cocoon CLI ──► images: OCI (EROFS layers, direct boot) | cloudimg (qcow2, - **Structured logging** — configurable log level (`--log-level`), log rotation (max size / age / backups) - **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) -- **Zero-daemon architecture** — one hypervisor process per VM, no long-running daemon +- **Daemon-optional architecture** — one hypervisor process per VM, every command standalone; `cocoon daemon` optionally adopts running VMs to converge crashes as they happen - **Garbage collection** — modular lock-safe GC with cross-module snapshot resolution; protects blobs referenced by running VMs and snapshots - **Doctor script** — pre-flight environment check and one-command dependency installation diff --git a/extend/disk/disk.go b/extend/disk/disk.go index 073e4eb9..82b3eb84 100644 --- a/extend/disk/disk.go +++ b/extend/disk/disk.go @@ -1,6 +1,4 @@ -// Package disk is the runtime attach interface for extra virtio-blk disks -// backed by existing raw files. Attach is runtime-only — snapshot/hibernate -// is refused while one is attached; detach never deletes the backing file. +// Package disk is the runtime attach interface for extra virtio-blk disks backed by existing raw files; attach is runtime-only (snapshot/hibernate refuse while one is attached) and detach never deletes the backing file. package disk import ( @@ -18,9 +16,7 @@ const diskIDPrefix = "cocoon-disk-" // ErrUnsupportedBackend signals the backend cannot hot-plug virtio-blk disks (e.g. Firecracker). var ErrUnsupportedBackend = errors.New("backend does not support disk attach") -// Spec is one attach request: 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; DirectIO nil means the create-path default (O_DIRECT unless writable disks have it disabled), tmpfs-backed files need an explicit off. type Spec struct { Path string Name string @@ -69,8 +65,7 @@ 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 (foreign same-prefix ids with an illegal name suffix are not ours). func NameFromID(id string) string { name, ok := strings.CutPrefix(id, diskIDPrefix) if ok && types.ValidDataDiskName(name) { diff --git a/extend/fs/fs.go b/extend/fs/fs.go index bc9c3545..ca2462a4 100644 --- a/extend/fs/fs.go +++ b/extend/fs/fs.go @@ -1,5 +1,4 @@ -// Package fs is the runtime attach interface for vhost-user-fs devices (typically virtiofsd). -// Attach is runtime-only — devices don't persist past VM stop; re-attach after restart. +// Package fs is the runtime attach interface for vhost-user-fs devices (typically virtiofsd); attach is runtime-only, devices don't persist past VM stop. package fs import ( @@ -20,8 +19,7 @@ var ( // ErrUnsupportedBackend signals the backend cannot hot-plug vhost-user-fs (e.g. Firecracker). ErrUnsupportedBackend = errors.New("backend does not support fs attach") - // Tag charset is intentionally portable: usable as a CH device id suffix - // (cocoon-fs-) and safe for shell quoting and guest mount commands. + // Tag charset is intentionally portable: usable as a CH device id suffix (cocoon-fs-) and safe for shell quoting and guest mount commands. validTagRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]{0,35}$`) ) diff --git a/extend/vfio/vfio.go b/extend/vfio/vfio.go index 9c342d09..5e7448d8 100644 --- a/extend/vfio/vfio.go +++ b/extend/vfio/vfio.go @@ -1,5 +1,4 @@ -// Package vfio is the runtime attach interface for VFIO PCI passthrough (GPU, NIC, NVMe). -// Attach is runtime-only — devices don't persist past VM stop; host IOMMU + vfio-pci binding are the user's job. +// Package vfio is the runtime attach interface for VFIO PCI passthrough (GPU, NIC, NVMe); attach is runtime-only, and host IOMMU + vfio-pci binding are the user's job. package vfio import ( @@ -18,13 +17,11 @@ var ( // ErrUnsupportedBackend signals the backend cannot hot-plug VFIO devices (e.g. Firecracker). ErrUnsupportedBackend = errors.New("backend does not support device attach") - // Match BDF in either short (01:00.0) or full (0000:01:00.0) form so the - // CLI accepts what `lspci` prints by default. + // Match BDF in either short (01:00.0) or full (0000:01:00.0) form so the CLI accepts what `lspci` prints by default. bdfShortRe = regexp.MustCompile(`^[0-9a-f]{2}:[0-9a-f]{2}\.[0-7]$`) bdfFullRe = regexp.MustCompile(`^[0-9a-f]{4}:[0-9a-f]{2}:[0-9a-f]{2}\.[0-7]$`) - // User-facing id charset matches CH device-id constraints; the prefix - // "cocoon-" is reserved so cocoon-derived ids never collide. + // User-facing id charset matches CH device-id constraints; the prefix "cocoon-" is reserved so cocoon-derived ids never collide. validIDRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$`) ) diff --git a/gc/orchestrator.go b/gc/orchestrator.go index 7d6630ed..7df8903a 100644 --- a/gc/orchestrator.go +++ b/gc/orchestrator.go @@ -20,9 +20,7 @@ type Orchestrator struct { // New returns an Orchestrator with no registered modules. func New() *Orchestrator { return &Orchestrator{} } -// Run executes one GC cycle: recover tombstones by phase, then snapshot → -// resolve → collect; modules revalidate every destructive decision under -// their own entity locks (§5 loose-snapshot rule). +// Run executes one GC cycle: recover tombstones by phase, then snapshot → resolve → collect; modules revalidate every destructive decision under their own entity locks (§5). func (o *Orchestrator) Run(ctx context.Context) error { start := time.Now() logger := log.WithFunc("gc.Run") diff --git a/hypervisor/backend.go b/hypervisor/backend.go index 879ad707..d5705d3b 100644 --- a/hypervisor/backend.go +++ b/hypervisor/backend.go @@ -78,8 +78,7 @@ type Backend struct { Net VMNetwork } -// NewBackend wires shared init: EnsureDirs, the backend's namespace on the -// injected meta store, nil-recorder fallback. +// NewBackend wires EnsureDirs, the backend's namespace on the injected meta store and the nil-recorder fallback. func NewBackend(typ string, conf BackendConfig, rec metering.Recorder, store meta.Store) (*Backend, error) { if err := conf.EnsureDirs(); err != nil { return nil, fmt.Errorf("ensure dirs: %w", err) @@ -111,6 +110,9 @@ type LaunchSpec struct { DeferCPUQuota bool } +// VMOp is one backend's per-VM lifecycle step, run by ForEachVM under the batch fan-out. +type VMOp func(context.Context, string) error + // PreflightHook validates rec against the snapshot source dir before anything is applied. type PreflightHook func(dir string, rec *VMRecord) error diff --git a/hypervisor/choreo_trace_test.go b/hypervisor/choreo_trace_test.go index 7fa6a755..1016482b 100644 --- a/hypervisor/choreo_trace_test.go +++ b/hypervisor/choreo_trace_test.go @@ -36,8 +36,7 @@ func TestLegacyChoreographyTrace(t *testing.T) { } ctx := t.Context() - // The injected deterministic clock (design §10) makes results and final - // bytes byte-exact against the legacy-recorded golden. + // The injected deterministic clock (design §10) makes results and final bytes exact against the legacy-recorded golden. clock := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) origNow := timeNow timeNow = func() time.Time { return clock } @@ -104,8 +103,7 @@ func TestLegacyChoreographyTrace(t *testing.T) { _, err = b.ResolveRef(ctx, "beta") record("rollback-then-resolve", "", err) - // delete: the P1 phase protocol replaces legacy's inline record removal; - // the differential holds at values, errors and final state. + // delete: the P1 phase protocol replaces legacy's inline record removal; the differential holds at values, errors and final state. vm1, err := b.LoadRecord(ctx, "VM1") if err != nil { t.Fatalf("load VM1 pre-delete: %v", err) @@ -114,9 +112,7 @@ func TestLegacyChoreographyTrace(t *testing.T) { _, err = b.ResolveRef(ctx, "alpha") record("delete-then-resolve", "", err) - // gc pass: the REAL orchestration on both sides — discovery, age cutoff - // through the seamed clock, ops-lock, under-lock revalidation. - // Real dirs mirror tracegen: lockable run dir, paths never in compared output. + // gc pass: the real orchestration on both sides (discovery, seamed-clock age cutoff, ops lock, under-lock revalidation); real dirs mirror tracegen and never appear in compared output. cfgDelta := &types.VMConfig{Name: "delta", Config: types.Config{CPU: 1}} record("reserve-vm4", "", b.ReserveVM(ctx, "VM4", cfgDelta, nil, t.TempDir(), t.TempDir())) clock = clock.Add(25 * time.Hour) diff --git a/hypervisor/cloudhypervisor/args.go b/hypervisor/cloudhypervisor/args.go index d5152d4e..78366822 100644 --- a/hypervisor/cloudhypervisor/args.go +++ b/hypervisor/cloudhypervisor/args.go @@ -8,10 +8,10 @@ import ( "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/types" + "github.com/cocoonstack/cocoon/utils" ) const ( - // defaultDiskQueueSize is the virtio-blk queue depth per device. defaultDiskQueueSize = 512 cidataFile = "cidata.img" @@ -186,9 +186,7 @@ func effectiveDirectIO(sc *types.StorageConfig, noDirectIO bool) bool { } func storageConfigToDisk(storageConfig *types.StorageConfig, cpuCount, diskQueueSize int, noDirectIO bool, allowed []int) chDisk { - if diskQueueSize <= 0 { - diskQueueSize = defaultDiskQueueSize - } + diskQueueSize = utils.OrDefault(diskQueueSize, defaultDiskQueueSize) d := chDisk{ Path: storageConfig.Path, ReadOnly: storageConfig.RO, diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index 4ed19fce..bdd79ef5 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -12,7 +12,6 @@ import ( "github.com/projecteru2/core/log" - "github.com/cocoonstack/cocoon/cgroup" "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/types" @@ -98,7 +97,7 @@ func (ch *CloudHypervisor) cloneAfterExtractParsed(ctx context.Context, vmID str } consoleSock := hypervisor.ConsoleSockPath(runDir) - allowedCPUs := cgroup.EffectiveCPUs(vmCfg.CPUSetCPUs, ch.conf.CgroupCPUs) + allowedCPUs := ch.EffectiveCPUs(&vmCfg.Config) if err = patchCHConfig(chConfigPath, &patchOptions{ storageConfigs: patchStorageConfigs, netTAPs: netTAPs, @@ -213,10 +212,8 @@ func (ch *CloudHypervisor) prepareCloneDataDisks(ctx context.Context, vmID strin return nil, nil } for _, spec := range vmCfg.DataDisks { - for _, sc := range existing { - if sc.Serial == spec.Name { - return nil, fmt.Errorf("--data-disk name %q collides with a disk inherited from the snapshot", spec.Name) - } + if slices.ContainsFunc(existing, func(sc *types.StorageConfig) bool { return sc.Serial == spec.Name }) { + return nil, fmt.Errorf("--data-disk name %q collides with a disk inherited from the snapshot", spec.Name) } } return hypervisor.PrepareDataDisks(ctx, ch.conf.VMRunDir(vmID), vmCfg.DataDisks) @@ -288,13 +285,7 @@ func restorePatchStorageConfigs(storageConfigs []*types.StorageConfig, directBoo if directBoot || windows || hadCidataInSnapshot { return storageConfigs } - out := make([]*types.StorageConfig, 0, len(storageConfigs)) - for _, sc := range storageConfigs { - if sc.Role != types.StorageRoleCidata { - out = append(out, sc) - } - } - return out + return slices.DeleteFunc(slices.Clone(storageConfigs), hasCidataRole) } // updateDataDiskPaths rewrites Role==Data paths to clone runDir; sidecar carries source paths. diff --git a/hypervisor/cloudhypervisor/direct.go b/hypervisor/cloudhypervisor/direct.go index d06594a2..ef7e0334 100644 --- a/hypervisor/cloudhypervisor/direct.go +++ b/hypervisor/cloudhypervisor/direct.go @@ -75,17 +75,8 @@ func cloneSnapshotFiles(ctx context.Context, dstDir, srcDir string) (*chVMConfig // cleanSnapshotFiles enumerates by name so stale data-*.raw and cocoon.json from a previous incarnation don't linger; COW files are overwritten anyway. func cleanSnapshotFiles(runDir string) error { return hypervisor.CleanSnapshotFiles(runDir, func(name string) bool { - switch { - case strings.HasPrefix(name, memoryRangeFile): - return true - case name == configJSONName || name == stateJSONName: - return true - case name == hypervisor.SnapshotMetaFile: - return true - case hypervisor.IsDataDiskFile(name): - return true - } - return false + return strings.HasPrefix(name, memoryRangeFile) || name == configJSONName || name == stateJSONName || + name == hypervisor.SnapshotMetaFile || hypervisor.IsDataDiskFile(name) }) } diff --git a/hypervisor/cloudhypervisor/extend.go b/hypervisor/cloudhypervisor/extend.go index ef62d8ef..d0c4ba4c 100644 --- a/hypervisor/cloudhypervisor/extend.go +++ b/hypervisor/cloudhypervisor/extend.go @@ -13,7 +13,6 @@ import ( "github.com/projecteru2/core/log" - "github.com/cocoonstack/cocoon/cgroup" "github.com/cocoonstack/cocoon/extend/disk" "github.com/cocoonstack/cocoon/extend/fs" "github.com/cocoonstack/cocoon/extend/vfio" @@ -46,7 +45,7 @@ func (ch *CloudHypervisor) DiskAttach(ctx context.Context, vmRef string, spec di makeBody := func(rec *hypervisor.VMRecord) any { d := storageConfigToDisk(&types.StorageConfig{ Role: types.StorageRoleData, Path: path, Serial: spec.Name, RO: spec.ReadOnly, DirectIO: spec.DirectIO, - }, rec.Config.CPU, rec.Config.DiskQueueSize, rec.Config.NoDirectIO, cgroup.EffectiveCPUs(rec.Config.CPUSetCPUs, ch.conf.CgroupCPUs)) + }, rec.Config.CPU, rec.Config.DiskQueueSize, rec.Config.NoDirectIO, ch.EffectiveCPUs(&rec.Config.Config)) d.ID = id return d } @@ -318,8 +317,7 @@ func (ch *CloudHypervisor) runningVMClientWithRecord(ctx context.Context, vmRef return utils.NewSocketHTTPClient(sockPath), vmID, rec, nil } -// convergeOrphanedPause resumes a VM left paused by a capture whose process died, returning a refreshed vm.info for device classification. -// The pause is provably ownerless: callers hold the VM ops lock every capture window holds end to end, and the Running-record gate in runningVMClientWithRecord keeps restore/clone pause windows (record Stopped/creating) out of reach. +// convergeOrphanedPause resumes a VM left paused by a dead capture and returns a refreshed vm.info; the pause is provably ownerless because callers hold the ops lock every capture window holds and runningVMClientWithRecord's Running gate excludes restore/clone windows. func convergeOrphanedPause(ctx context.Context, hc *http.Client, vmID string, info *chVMInfoResponse) (*chVMInfoResponse, error) { if info.State != chStatePaused { return info, nil @@ -333,10 +331,7 @@ func convergeOrphanedPause(ctx context.Context, hc *http.Client, vmID string, in } // listWith returns nil (not error) for stopped VMs so inspect can omit the field. -func listWith[A any]( - ctx context.Context, ch *CloudHypervisor, vmRef string, - extract func(*chVMInfoResponse) []A, -) ([]A, error) { +func listWith[A any](ctx context.Context, ch *CloudHypervisor, vmRef string, extract func(*chVMInfoResponse) []A) ([]A, error) { hc, _, _, err := ch.runningVMClientWithRecord(ctx, vmRef) if err != nil { if errors.Is(err, hypervisor.ErrNotRunning) { diff --git a/hypervisor/cloudhypervisor/netresize_test.go b/hypervisor/cloudhypervisor/netresize_test.go index 0e97f27a..93ea6cb9 100644 --- a/hypervisor/cloudhypervisor/netresize_test.go +++ b/hypervisor/cloudhypervisor/netresize_test.go @@ -71,8 +71,7 @@ func TestReconcileOrphanNICsReclaimsSlotOnEjectTimeout(t *testing.T) { } } -// TestResolveFailedPersist covers the persist commit-ambiguity window: a -// committed write keeps the device; only a conclusive miss tears down. +// TestResolveFailedPersist covers the persist commit-ambiguity window: a committed write keeps the device; only a conclusive miss tears down. func TestResolveFailedPersist(t *testing.T) { ch := newTestCH(t) ctx := t.Context() @@ -107,9 +106,7 @@ func TestResolveFailedPersist(t *testing.T) { } } -// TestNetResizeRemoveResumesWithoutLiveDevice covers issue #104: a NIC the -// record still carries but CH has already ejected (an interrupted prior -// remove) must be truncated from the record, not wedge the retry forever. +// TestNetResizeRemoveResumesWithoutLiveDevice covers #104: a NIC the record still carries but CH already ejected (interrupted prior remove) must be truncated from the record, not wedge the retry. func TestNetResizeRemoveResumesWithoutLiveDevice(t *testing.T) { ch := newTestCH(t) ctx := t.Context() @@ -149,8 +146,7 @@ func TestNICPersisted(t *testing.T) { } } -// TestConvergeOrphanedPause pins the interrupted-capture wedge: a save killed -// inside its pause window leaves CH Paused with nobody left to resume it. +// TestConvergeOrphanedPause pins the interrupted-capture wedge: a save killed inside its pause window leaves CH Paused with nobody left to resume it. func TestConvergeOrphanedPause(t *testing.T) { var ( mu sync.Mutex @@ -261,9 +257,7 @@ func newStubHTTPClient(t *testing.T, mux *http.ServeMux) *http.Client { }} } -// newCHStubClient serves vm.info and vm.remove-device over an httptest server; -// removed() snapshots the eject calls. stickyIDs stay in the device tree after -// removal, simulating a guest that never acks B0EJ. +// newCHStubClient serves vm.info and vm.remove-device over httptest; removed() snapshots the eject calls, and stickyIDs stay in the device tree after removal like a guest that never acks B0EJ. func newCHStubClient(t *testing.T, nets []chNet, stickyIDs ...string) (*http.Client, func() []string) { t.Helper() var mu sync.Mutex diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index 056b17a1..5857fd56 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -11,7 +11,6 @@ import ( "github.com/projecteru2/core/log" - "github.com/cocoonstack/cocoon/cgroup" "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/types" "github.com/cocoonstack/cocoon/utils" @@ -85,7 +84,7 @@ func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string, diskQueueSize: vmCfg.DiskQueueSize, noDirectIO: vmCfg.NoDirectIO, cpu: vmCfg.CPU, - allowedCPUs: cgroup.EffectiveCPUs(vmCfg.CPUSetCPUs, ch.conf.CgroupCPUs), + allowedCPUs: ch.EffectiveCPUs(&vmCfg.Config), }); err != nil { return nil, fmt.Errorf("patch config: %w", err) } diff --git a/hypervisor/cloudhypervisor/snapshot.go b/hypervisor/cloudhypervisor/snapshot.go index 2c70b841..7b30ad54 100644 --- a/hypervisor/cloudhypervisor/snapshot.go +++ b/hypervisor/cloudhypervisor/snapshot.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "os" "path/filepath" "github.com/cocoonstack/cocoon/extend/disk" @@ -52,7 +51,7 @@ func (ch *CloudHypervisor) snapshotSpec(ctx context.Context) hypervisor.Snapshot if cidataSrc == "" { // pre-first-boot: file exists but is not yet a recorded disk cidataSrc = filepath.Join(rec.RunDir, cidataFile) } - if _, statErr := os.Stat(cidataSrc); statErr != nil { + if !utils.FileExists(cidataSrc) { return nil } if cpErr := utils.SparseCopy(filepath.Join(tmpDir, cidataFile), cidataSrc, utils.NoSync); cpErr != nil { diff --git a/hypervisor/cloudhypervisor/start.go b/hypervisor/cloudhypervisor/start.go index 61e7e980..01f763de 100644 --- a/hypervisor/cloudhypervisor/start.go +++ b/hypervisor/cloudhypervisor/start.go @@ -8,7 +8,6 @@ import ( "github.com/projecteru2/core/log" - "github.com/cocoonstack/cocoon/cgroup" "github.com/cocoonstack/cocoon/hypervisor" ) @@ -20,7 +19,7 @@ func (ch *CloudHypervisor) startOne(ctx context.Context, id string) error { return ch.StartSequence(ctx, id, hypervisor.StartSpec{ RuntimeFiles: runtimeFiles, Launch: func(ctx context.Context, rec *hypervisor.VMRecord, sockPath string) (int, error) { - vmCfg := buildVMConfig(rec, hypervisor.ConsoleSockPath(rec.RunDir), cgroup.EffectiveCPUs(rec.Config.CPUSetCPUs, ch.conf.CgroupCPUs)) + vmCfg := buildVMConfig(rec, hypervisor.ConsoleSockPath(rec.RunDir), ch.EffectiveCPUs(&rec.Config.Config)) args := buildCLIArgs(vmCfg, sockPath) ch.saveCmdline(ctx, rec, args) return ch.launchProcess(ctx, rec, args, rec.ResolvedNetnsPath(), false) diff --git a/hypervisor/cloudhypervisor/stop.go b/hypervisor/cloudhypervisor/stop.go index 84112159..4bcd061c 100644 --- a/hypervisor/cloudhypervisor/stop.go +++ b/hypervisor/cloudhypervisor/stop.go @@ -20,7 +20,6 @@ func (ch *CloudHypervisor) stopOne(ctx context.Context, id string) error { return ch.StopOneSequence(ctx, id, ch.stopSpec()) } -// stopOneLocked is stopOne for callers already holding the VM's ops lock (DeleteAll). func (ch *CloudHypervisor) stopOneLocked(ctx context.Context, id string) error { return ch.StopOneLocked(ctx, id, ch.stopSpec()) } diff --git a/hypervisor/cloudhypervisor/utils.go b/hypervisor/cloudhypervisor/utils.go index 2be8e5a5..8119ae69 100644 --- a/hypervisor/cloudhypervisor/utils.go +++ b/hypervisor/cloudhypervisor/utils.go @@ -147,8 +147,8 @@ func resumeVM(ctx context.Context, hc *http.Client) error { // isAlreadyInStateError matches CH's exact `Invalid transition: InvalidStateTransition(, )` in a 500 body. func isAlreadyInStateError(err error, state string) bool { - var ae *utils.APIError - if !errors.As(err, &ae) || ae.Code != http.StatusInternalServerError { + ae, ok := errors.AsType[*utils.APIError](err) + if !ok || ae.Code != http.StatusInternalServerError { return false } return strings.Contains(ae.Message, fmt.Sprintf("Invalid transition: InvalidStateTransition(%s, %s)", state, state)) diff --git a/hypervisor/firecracker/api.go b/hypervisor/firecracker/api.go index c0687c99..0a7ba7ba 100644 --- a/hypervisor/firecracker/api.go +++ b/hypervisor/firecracker/api.go @@ -115,7 +115,6 @@ func putMachineConfig(ctx context.Context, hc *http.Client, cfg fcMachineConfig) return putJSON(ctx, hc, "/machine-config", cfg, "machine-config") } -// patchDrivePath repoints a block device's backing file on a booted VM. func patchDrivePath(ctx context.Context, hc *http.Client, driveID, pathOnHost string) error { return sendJSONOnce(ctx, hc, http.MethodPatch, "/drives/"+driveID, struct { DriveID string `json:"drive_id"` diff --git a/hypervisor/firecracker/clone.go b/hypervisor/firecracker/clone.go index 259f1581..443e7ab6 100644 --- a/hypervisor/firecracker/clone.go +++ b/hypervisor/firecracker/clone.go @@ -218,7 +218,6 @@ func (fc *Firecracker) resumeAndReanchorClone(ctx context.Context, pid int, cl c return nil } -// rebuildCloneStorage rewrites paths per role (Layer→source, COW→cowPath, Data→runDir); cidata rejected. func rebuildCloneStorage(meta *hypervisor.SnapshotMeta, cowPath string) ([]*types.StorageConfig, error) { runDir := filepath.Dir(cowPath) configs := hypervisor.CloneStorageConfigs(meta.StorageConfigs) @@ -413,11 +412,10 @@ func managedSourceVMIDs(runRoot string, srcConfigs, dstConfigs []*types.StorageC func holdManagedSourceVMLeases(ctx context.Context, rootDir, runRoot string, srcConfigs, dstConfigs []*types.StorageConfig) ([]*vmlock.SharedLease, error) { var leases []*vmlock.SharedLease - release := func() { closeLeases(leases) } for _, id := range managedSourceVMIDs(runRoot, srcConfigs, dstConfigs) { lease, err := vmlock.NewSharedLease(ctx, rootDir, id) if err != nil { - release() + closeLeases(leases) return nil, fmt.Errorf("lease source VM %s: %w", id, err) } leases = append(leases, lease) diff --git a/hypervisor/firecracker/create.go b/hypervisor/firecracker/create.go index 83299fc2..27256bfb 100644 --- a/hypervisor/firecracker/create.go +++ b/hypervisor/firecracker/create.go @@ -118,7 +118,7 @@ func EnsureVmlinux(kernelPath string) (string, error) { } vmlinuxPath := filepath.Join(filepath.Dir(kernelPath), "vmlinux") - if _, statErr := os.Stat(vmlinuxPath); statErr == nil { + if utils.FileExists(vmlinuxPath) { return vmlinuxPath, nil } diff --git a/hypervisor/firecracker/relay.go b/hypervisor/firecracker/relay.go index a7decc24..a4308cbe 100644 --- a/hypervisor/firecracker/relay.go +++ b/hypervisor/firecracker/relay.go @@ -215,7 +215,6 @@ func relayInheritedLeaseCount() (int, bool) { return count, true } -// relaySession handles one console connection: subscribes to the PTY broadcast, copies conn→master for input, and unsubscribes on disconnect. func relaySession(ctx context.Context, master io.Writer, conn net.Conn, bc *broadcaster) { defer conn.Close() //nolint:errcheck diff --git a/hypervisor/firecracker/restore.go b/hypervisor/firecracker/restore.go index 6b978886..adfbbdae 100644 --- a/hypervisor/firecracker/restore.go +++ b/hypervisor/firecracker/restore.go @@ -52,7 +52,7 @@ func (fc *Firecracker) restoreAfterExtract(ctx context.Context, vmID string, vmC snapshotCOW := filepath.Join(rec.RunDir, hypervisor.COWRawFileName) if snapshotCOW != cowPath { - if _, statErr := os.Stat(snapshotCOW); statErr == nil { + if utils.FileExists(snapshotCOW) { if renameErr := os.Rename(snapshotCOW, cowPath); renameErr != nil { return nil, fmt.Errorf("move COW: %w", renameErr) } diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index 4b12d219..036f8564 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -164,7 +164,6 @@ func (fc *Firecracker) launchProcessWithLeases(ctx context.Context, rec *hypervi _ = f.Close() } - // Create PTY pair: slave → FC stdin/stdout, master → console relay. master, slave, err := pty.Open() if err != nil { return 0, nil, fmt.Errorf("open pty: %w", err) diff --git a/hypervisor/firecracker/stop.go b/hypervisor/firecracker/stop.go index 76054806..6be435a2 100644 --- a/hypervisor/firecracker/stop.go +++ b/hypervisor/firecracker/stop.go @@ -18,7 +18,6 @@ func (fc *Firecracker) stopOne(ctx context.Context, id string) error { return fc.StopOneSequence(ctx, id, fc.stopSpec()) } -// stopOneLocked is stopOne for callers already holding the VM's ops lock (DeleteAll). func (fc *Firecracker) stopOneLocked(ctx context.Context, id string) error { return fc.StopOneLocked(ctx, id, fc.stopSpec()) } @@ -43,7 +42,6 @@ func (fc *Firecracker) gracefulStop(ctx context.Context, hc *http.Client, vmID, ) } -// forceTerminate skips graceful shutdown, going straight to SIGTERM → SIGKILL. func (fc *Firecracker) forceTerminate(ctx context.Context, sockPath string, pid int) error { return utils.TerminateProcess(ctx, pid, fc.conf.BinaryName(), sockPath, fc.conf.TerminateGracePeriod()) } diff --git a/hypervisor/hibernate_test.go b/hypervisor/hibernate_test.go index e491f4df..898d9f4d 100644 --- a/hypervisor/hibernate_test.go +++ b/hypervisor/hibernate_test.go @@ -139,8 +139,7 @@ func newHibernateTestVM(t *testing.T) (*Backend, string) { if err := cmd.Start(); err != nil { t.Fatalf("start stub vmm: %v", err) } - // Background reaper: a test-killed stub must not linger as a zombie, or - // TerminateProcess's IsProcessAlive polling never sees it exit. + // Background reaper: a test-killed stub must not linger as a zombie, or TerminateProcess's IsProcessAlive polling never sees it exit. waitDone := make(chan struct{}) go func() { _ = cmd.Wait() diff --git a/hypervisor/meta_test.go b/hypervisor/meta_test.go index b8c3a624..d870c616 100644 --- a/hypervisor/meta_test.go +++ b/hypervisor/meta_test.go @@ -23,9 +23,7 @@ var testVMTables = metajson.TableCodec{Specs: []metajson.TableSpec{ {Key: "tombstones", Table: tombstone.TableName, Optional: true}, }} -// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json -// and requires byte-identical output to what the LEGACY storage layer wrote -// for the same operations (fixtures generated at master by cmd/fixturegen). +// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json and requires bytes identical to what the legacy storage layer wrote (fixtures generated at master by cmd/fixturegen). func TestLegacyDifferentialTrace(t *testing.T) { ctx := t.Context() dir := t.TempDir() @@ -96,9 +94,7 @@ func TestLegacyDifferentialTrace(t *testing.T) { } } -// TestCrossComponentVMLockPath asserts CH/FC (via LockVMOps) and CNI (via -// lock/vmlock directly) resolve one vmID to the SAME lock file — a stale -// backend-specific resolver would let CNI GC and VM lifecycle interleave. +// TestCrossComponentVMLockPath asserts CH/FC (LockVMOps) and CNI (lock/vmlock) resolve one vmID to the same lock file; a stale backend-specific resolver would let CNI GC and VM lifecycle interleave. func TestCrossComponentVMLockPath(t *testing.T) { ctx := t.Context() b, _ := newMeteringTestBackend(t) @@ -143,8 +139,7 @@ func (idx *VMIndex) Init() { } } -// dbUpdate is the test-only whole-index shim: materialize, run fn, write the -// difference back. Production code never uses it. +// dbUpdate is the test-only whole-index shim: materialize, run fn, write the difference back. func (b *Backend) dbUpdate(ctx context.Context, fn func(*VMIndex) error) error { return b.update(ctx, func(t *vmTx) error { before, idx, err := materialize(t) @@ -169,8 +164,7 @@ func (b *Backend) dbRead(ctx context.Context, fn func(*VMIndex) error) error { }) } -// addOrphanDir survives only for fixtures/shims: production writes cleanup -// intent through tombstone payloads now. +// addOrphanDir survives only for fixtures/shims; production writes cleanup intent through tombstone payloads. func (t *vmTx) addOrphanDir(dir string) error { if _, ok, err := t.w.GetRaw(t.ctx, t.ns, TableOrphanDirs, dir); err != nil || ok { return err diff --git a/hypervisor/snapshot.go b/hypervisor/snapshot.go index c2cf25e2..61f59e60 100644 --- a/hypervisor/snapshot.go +++ b/hypervisor/snapshot.go @@ -234,8 +234,7 @@ func PopulateFromSrc(runDir, srcDir string, clean func(string) error, clone func return nil } -// PreflightRestore: load+validate sidecar, run backend-specific integrity, assert snapshot role sequence is a prefix of rec. -// The validated meta is returned so later restore phases reuse it instead of re-reading the copied-verbatim sidecar. +// PreflightRestore loads and validates the sidecar, runs the backend integrity check and asserts the snapshot role sequence prefixes rec; the validated meta is returned so later phases skip re-reading it. func PreflightRestore(srcDir, rootDir, runDir string, rec *VMRecord, integrity func(srcDir string, sidecar []*types.StorageConfig) error) (*SnapshotMeta, error) { meta, err := LoadAndValidateMeta(srcDir, rootDir, runDir) if err != nil { diff --git a/hypervisor/start.go b/hypervisor/start.go index dfe2f6d1..582aa0eb 100644 --- a/hypervisor/start.go +++ b/hypervisor/start.go @@ -17,7 +17,7 @@ import ( ) // StartAll runs startOne per ref; each start flips its own state under its VM's ops lock. -func (b *Backend) StartAll(ctx context.Context, refs []string, startOne func(context.Context, string) error) ([]string, error) { +func (b *Backend) StartAll(ctx context.Context, refs []string, startOne VMOp) ([]string, error) { ids, err := b.ResolveRefs(ctx, refs) if err != nil { return nil, err @@ -175,6 +175,11 @@ func (b *Backend) ArmCPUQuota(id string, cfg *types.Config) error { return nil } +// EffectiveCPUs resolves the host cpu set a VM under this backend may run on: explicit placement over the machine fence. +func (b *Backend) EffectiveCPUs(cfg *types.Config) []int { + return cgroup.EffectiveCPUs(cfg.CPUSetCPUs, b.Conf.CgroupCPUFence()) +} + // AbortLaunch terminates a failed launch and clears runtime files. func (b *Backend) AbortLaunch(ctx context.Context, pid int, sockPath, runDir string, runtimeFiles []string) { _ = utils.TerminateProcess(ctx, pid, b.Conf.BinaryName(), sockPath, b.Conf.TerminateGracePeriod()) diff --git a/hypervisor/state.go b/hypervisor/state.go index 3c5bbe57..ecc1e733 100644 --- a/hypervisor/state.go +++ b/hypervisor/state.go @@ -282,7 +282,6 @@ func (b *Backend) markFailedOperation(ctx context.Context, id string, markError } } -// detachedWrite returns a context for bookkeeping writes that must survive caller cancellation, bounded by persistTimeout. func detachedWrite(ctx context.Context) (context.Context, context.CancelFunc) { return context.WithTimeout(context.WithoutCancel(ctx), persistTimeout) } diff --git a/hypervisor/state_test.go b/hypervisor/state_test.go index 7a07d7a1..abca6c2e 100644 --- a/hypervisor/state_test.go +++ b/hypervisor/state_test.go @@ -301,8 +301,7 @@ func TestDirectRestoreSequenceEmitsComputeStopThenTransition(t *testing.T) { } func TestDirectRestoreSequenceEmitsOnlyComputeStopOnPopulateFailure(t *testing.T) { - // Storage must stay open when restore fails after kill — the on-disk files - // are still the old shape and vm rm will close it later with reason vm-rm. + // Storage stays open when restore fails after kill: the on-disk files are still the old shape and vm rm closes it later. b, rec := newMeteringTestBackend(t) ctx := t.Context() seedRunningVM(t, b, "vm1", 2, 2<<30, 20<<30) @@ -583,9 +582,7 @@ func TestPrepareStartRefusesCreating(t *testing.T) { } } -// TestForcedRetryStateOps is the design §10 gate: the migrated -// UpdateStates/BatchMarkStarted run on a forced-retry engine (every closure -// executes twice) and must emit each metering entry exactly once. +// TestForcedRetryStateOps is the design §10 gate: on a forced-retry engine (every closure runs twice) each metering entry must emit exactly once. func TestForcedRetryStateOps(t *testing.T) { const typ = "test-hv" dir := t.TempDir() @@ -620,8 +617,7 @@ func TestForcedRetryStateOps(t *testing.T) { } } -// stubBackendConfig satisfies BackendConfig for tests that only exercise the -// metering wiring; unused methods panic so accidental dependence shows up loud. +// stubBackendConfig satisfies BackendConfig for metering-wiring tests; unused methods panic so accidental dependence shows up loud. type stubBackendConfig struct { rootDir string indexFile string @@ -648,8 +644,7 @@ func (c stubBackendConfig) CgroupParentDir() string { return filepath.Join(c.roo func (stubBackendConfig) CgroupCPUFence() string { return "" } -// meteringStubConfig gives the metering stub a real VMRunDir so sequences -// can take the per-VM ops lock and MkdirTemp under it. +// meteringStubConfig gives the metering stub a real VMRunDir so sequences can take the per-VM ops lock and MkdirTemp under it. type meteringStubConfig struct { stubBackendConfig vmRunRoot string diff --git a/hypervisor/stop.go b/hypervisor/stop.go index 696345b1..354124b6 100644 --- a/hypervisor/stop.go +++ b/hypervisor/stop.go @@ -72,7 +72,7 @@ func (b *Backend) StopOneLocked(ctx context.Context, id string, spec StopSpec) e } // StopAll mirrors StartAll: stopOne per ref, each flipping its own state under its VM's ops lock. -func (b *Backend) StopAll(ctx context.Context, refs []string, stopOne func(context.Context, string) error) ([]string, error) { +func (b *Backend) StopAll(ctx context.Context, refs []string, stopOne VMOp) ([]string, error) { ids, err := b.ResolveRefs(ctx, refs) if err != nil { return nil, err @@ -81,7 +81,7 @@ func (b *Backend) StopAll(ctx context.Context, refs []string, stopOne func(conte } // DeleteAll removes VMs by ref; each VM's stop+probe+delete runs under its ops lock (#103), so stopLocked must not re-take it. -func (b *Backend) DeleteAll(ctx context.Context, refs []string, force bool, stopLocked func(context.Context, string) error) ([]string, error) { +func (b *Backend) DeleteAll(ctx context.Context, refs []string, force bool, stopLocked VMOp) ([]string, error) { ids, err := b.ResolveRefs(ctx, refs) if err != nil { return nil, err @@ -115,8 +115,7 @@ func (b *Backend) HandleStopResult(ctx context.Context, id, runDir string, runti return nil } -// deleteOneLocked is DeleteAll's per-VM body, run under the ops lock. -func (b *Backend) deleteOneLocked(ctx context.Context, id string, force bool, stopLocked func(context.Context, string) error, rec *VMRecord, procScan utils.ProcScan) error { +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 if runningErr := b.WithRunningVM(ctx, rec, func(_ int) error { diff --git a/hypervisor/stop_test.go b/hypervisor/stop_test.go index 666de348..a8b4bd0e 100644 --- a/hypervisor/stop_test.go +++ b/hypervisor/stop_test.go @@ -125,9 +125,7 @@ func TestStopStaleStoppedRecordWithLiveVMMStillTransitions(t *testing.T) { } } -// TestDeleteAllForceStopsUnderLock drives the full force path against a live -// stub VMM: the delete body holds the ops lock while stopLocked runs, so this -// deadlocks (and times out) if the stop variant re-takes the lock. +// TestDeleteAllForceStopsUnderLock drives the force path against a live stub VMM: the delete body holds the ops lock while stopLocked runs, so a re-locking stop variant deadlocks here. func TestDeleteAllForceStopsUnderLock(t *testing.T) { b, id := newHibernateTestVM(t) ctx := t.Context() @@ -217,9 +215,7 @@ func TestDeleteMigratedVMClearsCleanupIntent(t *testing.T) { } } -// A forced delete of a running VM stops it first, and that stop already closes -// the compute interval; emitting again from the pre-stop record would bill two -// stops against one start. +// A forced delete's stop already closes the compute interval; emitting again from the pre-stop record would bill two stops against one start. func TestDeleteForceEmitsOneComputeStop(t *testing.T) { b, id := newHibernateTestVM(t) ctx := t.Context() @@ -250,8 +246,7 @@ func TestDeleteForceEmitsOneComputeStop(t *testing.T) { } } -// shortTempDir is a /tmp-based TempDir: unix socket paths built under -// t.TempDir() can exceed the ~104-byte sockaddr cap (EINVAL on dial). +// shortTempDir is a /tmp-based TempDir: unix socket paths under t.TempDir() can exceed the ~104-byte sockaddr cap (EINVAL on dial). func shortTempDir(t *testing.T) string { t.Helper() dir, err := os.MkdirTemp("/tmp", "cocoon-test-") diff --git a/hypervisor/supervisor_test.go b/hypervisor/supervisor_test.go index a39da3a5..32d749bf 100644 --- a/hypervisor/supervisor_test.go +++ b/hypervisor/supervisor_test.go @@ -322,8 +322,7 @@ func TestTryLockVMOpsReportsBusyWithoutError(t *testing.T) { } } -// StoppedAt is when the VM was observed stopped, so a record predating the -// interval bookkeeping still gets one. +// StoppedAt is the observation time, so a record predating the interval bookkeeping still gets one. func TestConvergeDeadDatesARecordWithNoOpenInterval(t *testing.T) { b, rec := newMeteringTestBackend(t) ctx := t.Context() diff --git a/hypervisor/teardown_test.go b/hypervisor/teardown_test.go index 7df40b28..5295a798 100644 --- a/hypervisor/teardown_test.go +++ b/hypervisor/teardown_test.go @@ -180,9 +180,7 @@ func TestEntryGuardDisciplines(t *testing.T) { } } -// TestLiveHolderKeepsLease is §9's negative fencing case: while A is alive -// and slow (ops lock held, tombstone deleting), B's recovery sweep can -// neither take the lock nor steal the lease. +// TestLiveHolderKeepsLease is §9's negative fencing case: while A is alive and slow (ops lock held, tombstone deleting), B's sweep can neither take the lock nor steal the lease. func TestLiveHolderKeepsLease(t *testing.T) { b, _ := newMeteringTestBackend(t) ctx := t.Context() @@ -231,9 +229,7 @@ func TestLiveHolderKeepsLease(t *testing.T) { } } -// TestPrepareStartRefusesMidDeleting is the §9 entry gate through the real -// start entrypoint: a dead worker's deleting tombstone must drive recovery -// and refuse the boot. +// TestPrepareStartRefusesMidDeleting is the §9 entry gate through the real start entrypoint: a dead worker's deleting tombstone drives recovery and refuses the boot. func TestPrepareStartRefusesMidDeleting(t *testing.T) { b, _ := newMeteringTestBackend(t) var netTorn []string diff --git a/hypervisor/utils.go b/hypervisor/utils.go index df0052c8..e5b4da8b 100644 --- a/hypervisor/utils.go +++ b/hypervisor/utils.go @@ -118,7 +118,7 @@ func (b *Backend) LogPath(ctx context.Context, ref string) (string, error) { } // ForEachVM runs fn over ids in parallel up to EffectivePoolSize, logging per-id failures. -func (b *Backend) ForEachVM(ctx context.Context, ids []string, op string, fn func(context.Context, string) error) ([]string, error) { +func (b *Backend) ForEachVM(ctx context.Context, ids []string, op string, fn VMOp) ([]string, error) { logger := log.WithFunc(b.Typ + "." + op) result := utils.ForEach(ctx, ids, fn, b.Conf.EffectivePoolSize()) for _, err := range result.Errors { diff --git a/hypervisor/utils_test.go b/hypervisor/utils_test.go index 2f9dfc5a..04ab3f9e 100644 --- a/hypervisor/utils_test.go +++ b/hypervisor/utils_test.go @@ -202,8 +202,7 @@ func TestIsDataDiskFile(t *testing.T) { } func TestBuildBaseCmdline(t *testing.T) { - // Locks the cmdline format so a refactor of the shared builder can't silently - // shift kernel boot parameters for either backend. + // Locks the cmdline format so a refactor of the shared builder can't silently shift kernel boot parameters for either backend. const ( chPrefix = "console=hvc0 loglevel=3" fcPrefix = "console=ttyS0 reboot=k loglevel=3 pci=off i8042.noaux 8250.nr_uarts=1" diff --git a/images/cloudimg/cloudimg.go b/images/cloudimg/cloudimg.go index b47187cf..b4bfa4a1 100644 --- a/images/cloudimg/cloudimg.go +++ b/images/cloudimg/cloudimg.go @@ -26,7 +26,7 @@ type CloudImg struct { conf *Config store *images.Store[imageEntry] pullGroup singleflight.Group - pinnedElsewhere func(context.Context) (map[string]struct{}, error) + pinnedElsewhere images.PinRecheck } // New builds the cloud image backend under rootDir; pullConns <= 0 defaults to 8 concurrent Range connections. diff --git a/images/cloudimg/gc.go b/images/cloudimg/gc.go index 5597212f..d698489f 100644 --- a/images/cloudimg/gc.go +++ b/images/cloudimg/gc.go @@ -30,11 +30,10 @@ func (c *CloudImg) RegisterGC(orch *gc.Orchestrator) { } // SetPinnedElsewhere injects the cross-subsystem pin recheck used by GC. -func (c *CloudImg) SetPinnedElsewhere(fn func(context.Context) (map[string]struct{}, error)) { +func (c *CloudImg) SetPinnedElsewhere(fn images.PinRecheck) { c.pinnedElsewhere = fn } -// PinBlobs implements images.Images: digest locks held while the caller commits a pin. func (c *CloudImg) PinBlobs(_ context.Context, blobIDs map[string]struct{}) (func(), error) { return images.PinBlobs(&c.conf.BaseConfig, blobIDs) } diff --git a/images/cloudimg/pull.go b/images/cloudimg/pull.go index dfcd4568..0d06c2b1 100644 --- a/images/cloudimg/pull.go +++ b/images/cloudimg/pull.go @@ -22,13 +22,9 @@ import ( ) const ( - urlDownloadTimeout = 30 * time.Minute - - // maxDownloadBytes is the maximum allowed download size (20 GiB). - maxDownloadBytes int64 = 20 << 30 - - // progressInterval is how often download progress is reported (1 MiB). - progressInterval = 1 << 20 + urlDownloadTimeout = 30 * time.Minute + maxDownloadBytes int64 = 20 << 30 + progressInterval = 1 << 20 ) // progressCounter emits PhaseDownload events every ~1 MiB; mutex-guarded so it serves both the serial writer and parallel range workers. diff --git a/images/cloudimg/pull_test.go b/images/cloudimg/pull_test.go index f5840a2e..e6756f13 100644 --- a/images/cloudimg/pull_test.go +++ b/images/cloudimg/pull_test.go @@ -156,8 +156,7 @@ func TestDownloadToFileShortRangeBodyFails(t *testing.T) { _, _ = w.Write(data[start : end+1]) // probe and tiny ranges served fully return } - // Serve half the promised bytes, then flush to force chunked encoding so - // the client sees a clean EOF instead of a Content-Length mismatch. + // Serve half the promised bytes, then flush to force chunked encoding so the client sees a clean EOF instead of a Content-Length mismatch. _, _ = w.Write(data[start : start+(end-start)/2]) w.(http.Flusher).Flush() })) @@ -193,8 +192,7 @@ func TestDownloadToFileMismatchedContentRangeFails(t *testing.T) { } } -// rangeHandler serves data with full HTTP Range support; fail, if non-nil, lets a test force a -// specific requested range to error out (simulating a mid-download server failure). +// rangeHandler serves data with full HTTP Range support; fail, if non-nil, lets a test force a specific requested range to error out (simulating a mid-download server failure). func rangeHandler(data []byte, fail func(start, end int64) bool) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { rangeHeader := r.Header.Get("Range") diff --git a/images/gc.go b/images/gc.go index 52f9c056..e7c9a194 100644 --- a/images/gc.go +++ b/images/gc.go @@ -17,6 +17,9 @@ import ( "github.com/cocoonstack/cocoon/utils" ) +// PinRecheck reports blobs pinned by VM/snapshot records, consulted under the digest lock since the candidate list predates those pins. +type PinRecheck func(ctx context.Context) (map[string]struct{}, error) + // ImageGCSnapshot is the unified GC snapshot for image backends. type ImageGCSnapshot struct { refs map[string]struct{} // digest hexes referenced by the index @@ -25,24 +28,19 @@ type ImageGCSnapshot struct { // GCModuleConfig configures a generic image GC module. type GCModuleConfig[E any] struct { - Name string - Store *Store[E] - // LockPath returns the per-digest blob lock path. + Name string + Store *Store[E] LockPath func(hex string) string - // ReadRefs extracts referenced digest hexes from the index. ReadRefs func(map[string]*E) map[string]struct{} - // ScanDisk returns digest hexes found on disk (blobs). ScanDisk func() ([]string, error) // ExtraDisk returns additional hex IDs on disk (e.g., OCI boot dirs). Optional. ExtraDisk func() ([]string, error) - // Removers are called per hex ID during collect. - Removers []func(string) error - // TempDir for stale temp cleanup. - TempDir string + Removers []func(string) error + TempDir string // DirOnly: true for OCI (temp dirs), false for cloudimg (temp files). DirOnly bool - // PinnedElsewhere reports blobs pinned by VM/snapshot records, consulted under the digest lock since the candidate list predates those pins. Optional. - PinnedElsewhere func(ctx context.Context) (map[string]struct{}, error) + // PinnedElsewhere is optional. + PinnedElsewhere PinRecheck } // BuildGCModule constructs a gc.Module from the config. diff --git a/images/gc_test.go b/images/gc_test.go index 1ae5775d..0452528d 100644 --- a/images/gc_test.go +++ b/images/gc_test.go @@ -18,9 +18,7 @@ var testImageTables = metajson.TableCodec{Specs: []metajson.TableSpec{ {Key: "tombstones", Table: tombstone.TableName, Optional: true}, }} -// TestGCCollectSkipsRepublishedBlob pins the loose-GC revalidation: a digest -// that became referenced after the snapshot (a publish finished and released -// its lock) must survive Collect. +// TestGCCollectSkipsRepublishedBlob pins the loose-GC revalidation: a digest that became referenced after the snapshot (a publish finished and released its lock) must survive Collect. func TestGCCollectSkipsRepublishedBlob(t *testing.T) { ctx := t.Context() dir := t.TempDir() @@ -77,8 +75,7 @@ func TestGCCollectSkipsRepublishedBlob(t *testing.T) { } } -// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json -// and requires byte-identical output to the legacy storage layer's writes. +// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json and requires byte-identical output to the legacy storage layer's writes. func TestLegacyDifferentialTrace(t *testing.T) { ctx := t.Context() dir := t.TempDir() @@ -131,9 +128,7 @@ func TestLegacyDifferentialTrace(t *testing.T) { } } -// TestGCCollectRespectsExternalPins pins the create-window race: a VM pin -// committed after the GC snapshot must save the blob via the under-lock -// recheck (design §5 step 2). +// TestGCCollectRespectsExternalPins pins the create-window race: a VM pin committed after the GC snapshot must save the blob via the under-lock recheck (design §5 step 2). func TestGCCollectRespectsExternalPins(t *testing.T) { ctx := t.Context() dir := t.TempDir() @@ -176,8 +171,7 @@ func TestGCCollectRespectsExternalPins(t *testing.T) { } } -// TestPinBlobs pins the primitive: the digest lock excludes a concurrent -// taker while held, releases cleanly, and a missing blob fails the pin. +// TestPinBlobs pins the primitive: the digest lock excludes a concurrent taker while held, releases cleanly, and a missing blob fails the pin. func TestPinBlobs(t *testing.T) { cfg := &BaseConfig{RootDir: t.TempDir(), Subdir: "oci", BlobExt: ".erofs"} if err := cfg.EnsureDirs(); err != nil { diff --git a/images/index_test.go b/images/index_test.go index 8dc6ee89..5982aff9 100644 --- a/images/index_test.go +++ b/images/index_test.go @@ -20,8 +20,7 @@ func TestLookupOne(t *testing.T) { if _, e, ok := LookupOne(images, "sha256:aabb000011112222333344445555666a"); !ok || (*e).EntryID() != "sha256:aabb000011112222333344445555666a" { t.Errorf("multi-tag single-digest must resolve: ok=%v", ok) } - // 16-hex prefix spanning two distinct digests: past the minHexLen guard, - // must be refused by the cross-digest check. + // 16-hex prefix spanning two distinct digests: past the minHexLen guard, must be refused by the cross-digest check. if _, _, ok := LookupOne(images, "aabb000011112222"); ok { t.Error("ambiguous cross-digest prefix must not resolve") } @@ -54,8 +53,7 @@ func TestDeleteByIDRejectsAmbiguousPrefix(t *testing.T) { } } -// testIndex has two distinct digests sharing a 16-hex prefix, so an ambiguous -// query can pass LookupRefs' minHexLen guard and reach the cross-digest check. +// testIndex has two distinct digests sharing a 16-hex prefix, so an ambiguous query can pass LookupRefs' minHexLen guard and reach the cross-digest check. func testIndex() map[string]*testEntry { return map[string]*testEntry{ "img:v1": {id: "sha256:aabb000011112222333344445555666a", ref: "img:v1"}, diff --git a/images/oci/boot.go b/images/oci/boot.go index 7c075988..068688b4 100644 --- a/images/oci/boot.go +++ b/images/oci/boot.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "strings" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -22,17 +23,8 @@ import ( const maxKernelBytes = 512 << 20 func bootFilesPresent(results []pullLayerResult) (hasKernel, hasInitrd bool) { - for i := range results { - if results[i].kernelPath != "" { - hasKernel = true - } - if results[i].initrdPath != "" { - hasInitrd = true - } - if hasKernel && hasInitrd { - return hasKernel, hasInitrd - } - } + hasKernel = slices.ContainsFunc(results, func(r pullLayerResult) bool { return r.kernelPath != "" }) + hasInitrd = slices.ContainsFunc(results, func(r pullLayerResult) bool { return r.initrdPath != "" }) return hasKernel, hasInitrd } diff --git a/images/oci/fetch.go b/images/oci/fetch.go index 50280f65..c5d7f50a 100644 --- a/images/oci/fetch.go +++ b/images/oci/fetch.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "runtime" + "slices" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" @@ -67,12 +68,7 @@ func isUpToDate(conf *Config, idx *imageIndex, ref, digestHex string) bool { !utils.ValidFile(conf.InitrdPath(entry.InitrdLayer.Hex())) { return false } - for _, layer := range entry.Layers { - if !utils.ValidFile(conf.BlobPath(layer.Digest.Hex())) { - return false - } - } - return true + return !slices.ContainsFunc(entry.Layers, func(l layerEntry) bool { return !utils.ValidFile(conf.BlobPath(l.Digest.Hex())) }) } func collectBootHexes(idx *imageIndex) map[string]struct{} { diff --git a/images/oci/gc.go b/images/oci/gc.go index 29afadb7..3b43aaa1 100644 --- a/images/oci/gc.go +++ b/images/oci/gc.go @@ -28,7 +28,7 @@ func (o *OCI) GCModule() gc.Module[images.ImageGCSnapshot] { } // SetPinnedElsewhere injects the cross-subsystem pin recheck used by GC. -func (o *OCI) SetPinnedElsewhere(fn func(context.Context) (map[string]struct{}, error)) { +func (o *OCI) SetPinnedElsewhere(fn images.PinRecheck) { o.pinnedElsewhere = fn } diff --git a/images/oci/oci.go b/images/oci/oci.go index 1429e59d..ed01ed9f 100644 --- a/images/oci/oci.go +++ b/images/oci/oci.go @@ -30,7 +30,7 @@ type OCI struct { conf *Config store *images.Store[imageEntry] pullGroup singleflight.Group - pinnedElsewhere func(context.Context) (map[string]struct{}, error) + pinnedElsewhere images.PinRecheck } // New builds the OCI backend under rootDir; poolSize <= 0 means NumCPU. diff --git a/images/op_test.go b/images/op_test.go index a6c39715..3b3d174e 100644 --- a/images/op_test.go +++ b/images/op_test.go @@ -21,8 +21,7 @@ func TestSingleflightDoDetachesCanceledWaiter(t *testing.T) { }) close(done) }() - // The blocking flight must register first, or the canceled call could win - // the key and select its own instant result over the canceled ctx. + // The blocking flight must register first, or the canceled call could win the key and select its own instant result over the canceled ctx. <-started ctx, cancel := context.WithCancel(t.Context()) diff --git a/images/store.go b/images/store.go index b50b5f7f..8320b814 100644 --- a/images/store.go +++ b/images/store.go @@ -103,10 +103,7 @@ type BlobLocks struct { // Lock acquires every path in sorted order, blocking. func (b *BlobLocks) Lock(paths ...string) error { - sorted := slices.Clone(paths) - slices.Sort(sorted) - sorted = slices.Compact(sorted) - for _, p := range sorted { + for _, p := range slices.Compact(slices.Sorted(slices.Values(paths))) { fl := gofrsflock.New(p) if err := fl.Lock(); err != nil { _ = fl.Close() diff --git a/lock/vmlock/gc.go b/lock/vmlock/gc.go index 74f6f523..9b9e0b60 100644 --- a/lock/vmlock/gc.go +++ b/lock/vmlock/gc.go @@ -2,6 +2,8 @@ package vmlock import ( "context" + "errors" + "io/fs" "os" "strings" @@ -23,7 +25,7 @@ func GCModule(rootDir string) gc.Module[lockSnapshot] { ReadDB: func(context.Context) (lockSnapshot, error) { entries, err := os.ReadDir(lockDir(rootDir)) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return lockSnapshot{}, nil } return lockSnapshot{}, err diff --git a/main.go b/main.go index ce1c8c5b..40ba9738 100644 --- a/main.go +++ b/main.go @@ -18,8 +18,7 @@ func main() { return } if err := cmd.Execute(ctx); err != nil { - var exitErr *cmdvm.ExecExitError - if errors.As(err, &exitErr) { + if exitErr, ok := errors.AsType[*cmdvm.ExecExitError](err); ok { os.Exit(exitErr.Code) } os.Exit(1) diff --git a/meta/broadcast.go b/meta/broadcast.go index 0a7e03fa..96383859 100644 --- a/meta/broadcast.go +++ b/meta/broadcast.go @@ -63,8 +63,7 @@ func (b *Broadcaster) Stop() { _ = b.watcher.Close() } -// Run is the notifier goroutine body: watcher events are debounced, watcher -// errors, the optional extra trigger and a safety poll check immediately. +// Run is the notifier goroutine body: watcher events are debounced; watcher errors, the optional extra trigger and a safety poll check immediately. func (b *Broadcaster) Run(check func(), extra <-chan struct{}) { timer := time.NewTimer(0) if !timer.Stop() { diff --git a/meta/contracttest/suite.go b/meta/contracttest/suite.go index b9024063..d21062ea 100644 --- a/meta/contracttest/suite.go +++ b/meta/contracttest/suite.go @@ -385,9 +385,7 @@ func testEvents(t *testing.T, factory Factory) { } } -// testLogCursor asserts §9's log contract: committed Seq unique and strictly -// increasing, a rolled-back append never surfaces in Scan (its number may be -// reused or left as a gap), Scan(after) exclusive and in order. +// testLogCursor asserts §9's log contract: committed Seq unique and strictly increasing, a rolled-back append never surfaces in Scan (its number may be reused or left as a gap), Scan(after) exclusive and in order. func testLogCursor(t *testing.T, factory Factory) { ctx := t.Context() s := factory(t, []string{nsAlpha}) diff --git a/meta/json/multiprocess_test.go b/meta/json/multiprocess_test.go index ace63e23..55a3c8ae 100644 --- a/meta/json/multiprocess_test.go +++ b/meta/json/multiprocess_test.go @@ -21,9 +21,7 @@ const ( inverseOps = 10 ) -// TestMultiProcessCorrectness is the design §9 gate with real processes, not -// goroutines: every worker's acknowledged insert must be present afterwards, -// every failure a mapped taxonomy error, and the reopened store uncorrupted. +// TestMultiProcessCorrectness is the design §9 gate with real processes, not goroutines: every worker's acknowledged insert must be present afterwards, every failure a mapped taxonomy error, and the reopened store uncorrupted. func TestMultiProcessCorrectness(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -54,8 +52,7 @@ func TestMultiProcessCorrectness(t *testing.T) { } } -// TestMultiProcessWorker is the helper-process body; it only runs when -// re-invoked by TestMultiProcessCorrectness with the env set. +// TestMultiProcessWorker is the helper-process body; it only runs when re-invoked by TestMultiProcessCorrectness with the env set. func TestMultiProcessWorker(t *testing.T) { dir := os.Getenv("META_MP_DIR") if dir == "" { @@ -80,9 +77,7 @@ func TestMultiProcessWorker(t *testing.T) { } } -// TestInverseScopeNoDeadlock is the §9 cross-process gate: mutually-inverse -// scopes (write alpha read beta vs write beta read alpha) storm concurrently -// and must never deadlock — the engine's fixed global lock order is the proof. +// TestInverseScopeNoDeadlock is the §9 cross-process gate: mutually-inverse scopes (write alpha read beta vs write beta read alpha) storm concurrently and must never deadlock — the engine's fixed global lock order is the proof. func TestInverseScopeNoDeadlock(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -141,8 +136,7 @@ func TestInverseScopeWorker(t *testing.T) { } } -// TestAckedDurableSurvivesKill is the §9 durability gate: every insert the -// worker ACKed after a CommitDurable return must be present after SIGKILL. +// TestAckedDurableSurvivesKill is the §9 durability gate: every insert the worker ACKed after a CommitDurable return must be present after SIGKILL. func TestAckedDurableSurvivesKill(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -205,8 +199,7 @@ func TestAckWorker(t *testing.T) { } } -// TestEventsExternalProcess is §9's cross-process signal gate for the json -// engine: another process's committed rename must reach subscribers here. +// TestEventsExternalProcess is §9's cross-process signal gate for the json engine: another process's committed rename must reach subscribers here. func TestEventsExternalProcess(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -248,8 +241,7 @@ func TestEventsWriterWorker(t *testing.T) { } } -// stormWorkers is the design's 256-process gate by default; constrained CI -// runners dial it down via COCOON_STORM_WORKERS (full scale runs offline). +// stormWorkers is the design's 256-process gate by default; constrained CI runners dial it down via COCOON_STORM_WORKERS (full scale runs offline). func stormWorkers() int { if v, err := strconv.Atoi(os.Getenv("COCOON_STORM_WORKERS")); err == nil && v > 0 { return v diff --git a/meta/json/store.go b/meta/json/store.go index 80a27ce4..e33fd5eb 100644 --- a/meta/json/store.go +++ b/meta/json/store.go @@ -157,9 +157,9 @@ func (s *Store) withLocked(ctx context.Context, states []*nsState, fn func() err logger := log.WithFunc("meta.json.withLocked") for i, st := range states { if err := st.locker.Lock(ctx); err != nil { - for j := i - 1; j >= 0; j-- { - if uerr := states[j].locker.Unlock(ctx); uerr != nil { - logger.Errorf(ctx, uerr, "unlock %s", states[j].def.Name) + for _, held := range slices.Backward(states[:i]) { + if uerr := held.locker.Unlock(ctx); uerr != nil { + logger.Errorf(ctx, uerr, "unlock %s", held.def.Name) } } return fmt.Errorf("lock %s: %w", st.def.Name, err) diff --git a/meta/json/store_test.go b/meta/json/store_test.go index f4d55425..45e47c1e 100644 --- a/meta/json/store_test.go +++ b/meta/json/store_test.go @@ -211,14 +211,11 @@ func TestOpenValidation(t *testing.T) { } } -// TestEventsForcedOverflow is the §7 forced-overflow gate: with fsnotify -// severed (lost events), an injected overflow error must make the notifier -// re-read its token and signal the missed change. +// TestEventsForcedOverflow is the §7 forced-overflow gate: with fsnotify severed (lost events), an injected overflow error must make the notifier re-read its token and signal the missed change. func TestEventsForcedOverflow(t *testing.T) { ctx := t.Context() dir := t.TempDir() - // Seam set before the notifier loop starts; reset runs after the store's - // Cleanup closes that loop (LIFO), so neither races the loop's reads. + // Seam set before the notifier loop starts; reset runs after the store's Cleanup closes that loop (LIFO), so neither races the loop's reads. testWatchErrs = make(chan struct{}) t.Cleanup(func() { testWatchErrs = nil }) s := newStore(t, dir, "alpha") @@ -254,9 +251,7 @@ func TestEventsForcedOverflow(t *testing.T) { } } -// TestDurableSyncOrder is the §9 durability gate at the engine level: a -// CommitDurable ack happens only after rename → main fsync → prev fsync → -// parent-dir fsync all ran, while CommitRelaxed relinquishes the post-main syncs. +// TestDurableSyncOrder is the §9 durability gate at the engine level: a CommitDurable ack happens only after rename → main fsync → prev fsync → parent-dir fsync all ran, while CommitRelaxed relinquishes the post-main syncs. func TestDurableSyncOrder(t *testing.T) { var steps []string testCrashStep = func(step string) error { steps = append(steps, step); return nil } @@ -293,8 +288,7 @@ func TestDurableSyncOrder(t *testing.T) { } } -// TestCleanUpdateSkipsCommit pins the dirty-skip: a read-only Update must not -// re-encode, rotate or fsync — the entry guards run on every VM operation. +// TestCleanUpdateSkipsCommit pins the dirty-skip: a read-only Update must not re-encode, rotate or fsync — the entry guards run on every VM operation. func TestCleanUpdateSkipsCommit(t *testing.T) { var steps []string testCrashStep = func(step string) error { steps = append(steps, step); return nil } @@ -351,8 +345,7 @@ func TestTrailingDataFallsBackToPrev(t *testing.T) { t.Fatal(err) } } - // Legacy json.Unmarshal rejected trailing bytes; the streaming decoder - // must too, so this main is corrupt and .prev is served. + // Legacy json.Unmarshal rejected trailing bytes; the streaming decoder must too, so this main is corrupt and .prev is served. main, err := os.ReadFile(path) if err != nil { t.Fatal(err) diff --git a/meta/namedtx.go b/meta/namedtx.go index b11ed6cb..a93e4874 100644 --- a/meta/namedtx.go +++ b/meta/namedtx.go @@ -10,8 +10,7 @@ import ( const resolvePrefixMin = 3 -// RecordTx is the id→record map view of one table inside a transaction: -// Get mirrors map lookup (nil when absent), Put is an upsert. +// RecordTx is the id→record map view of one table inside a transaction: Get mirrors map lookup (nil when absent), Put is an upsert. type RecordTx[R any] struct { ctx context.Context r Reader @@ -107,8 +106,7 @@ func (x *NamedTx[R]) NameDelIfOwned(name, id string) error { return x.NameDel(name) } -// Resolve ports utils.ResolveRef: exact ID, then name, then ID prefix of at -// least three characters; notFound is the subsystem's sentinel. +// Resolve ports utils.ResolveRef: exact ID, then name, then ID prefix of at least three characters; notFound is the subsystem's sentinel. func (x *NamedTx[R]) Resolve(ref string, notFound error) (string, error) { if rec, err := x.Get(ref); err != nil { return "", err diff --git a/meta/sqlite/events_test.go b/meta/sqlite/events_test.go index bbd9c58e..0978bc21 100644 --- a/meta/sqlite/events_test.go +++ b/meta/sqlite/events_test.go @@ -11,8 +11,7 @@ import ( "github.com/cocoonstack/cocoon/meta" ) -// TestEventsExternalProcess is §9's cross-process signal gate: a commit by -// ANOTHER process must reach this process's subscribers. +// TestEventsExternalProcess is §9's cross-process signal gate: a commit by ANOTHER process must reach this process's subscribers. func TestEventsExternalProcess(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -48,8 +47,7 @@ func TestEventsWriterWorker(t *testing.T) { } } -// TestEventsPoolChurn: reader-pool connections cycling under load must not -// blind the notifier — its pinned connection is untouched by the pool (§7). +// TestEventsPoolChurn: reader-pool connections cycling under load must not blind the notifier — its pinned connection is untouched by the pool (§7). func TestEventsPoolChurn(t *testing.T) { dir := t.TempDir() s := newStore(t, dir, "alpha") @@ -77,8 +75,7 @@ func TestEventsPoolChurn(t *testing.T) { } } -// TestEventsSeveredWatchPollFallback: a lost fsnotify watch must degrade to -// the change-token-confirmed safety poll, not a permanently missed change. +// TestEventsSeveredWatchPollFallback: a lost fsnotify watch must degrade to the change-token-confirmed safety poll, not a permanently missed change. func TestEventsSeveredWatchPollFallback(t *testing.T) { dir := t.TempDir() s := newStore(t, dir, "alpha") diff --git a/meta/sqlite/fscheck_linux.go b/meta/sqlite/fscheck_linux.go index d121e9cb..54c77ef1 100644 --- a/meta/sqlite/fscheck_linux.go +++ b/meta/sqlite/fscheck_linux.go @@ -5,8 +5,7 @@ import ( "syscall" ) -// WAL needs coherent shared memory; these magics mark filesystems that -// cannot provide it (§4). FUSE is refused as unknowable. +// WAL needs coherent shared memory; these magics mark filesystems that cannot provide it (§4), FUSE refused as unknowable. var unsupportedFS = map[uint32]string{ 0x6969: "nfs", 0xFF534D42: "cifs", diff --git a/meta/sqlite/init_test.go b/meta/sqlite/init_test.go index 48519e08..b6df7449 100644 --- a/meta/sqlite/init_test.go +++ b/meta/sqlite/init_test.go @@ -35,8 +35,7 @@ func TestFailedInitRestarts(t *testing.T) { func TestInitIdentityAtomicWithSchema(t *testing.T) { path := filepath.Join(t.TempDir(), DBFileName) - // The retired pragmas-after-commit window: schema present, identity - // absent. It must refuse loudly, never strand or silently delete. + // The retired pragmas-after-commit window: schema present, identity absent. It must refuse loudly, never strand or silently delete. db, err := open(path, "FULL", true) if err != nil { t.Fatal(err) diff --git a/meta/sqlite/maintenance_test.go b/meta/sqlite/maintenance_test.go index 4673204d..9a7bc9cc 100644 --- a/meta/sqlite/maintenance_test.go +++ b/meta/sqlite/maintenance_test.go @@ -26,8 +26,7 @@ func TestBackupFidelity(t *testing.T) { t.Fatalf("put %s: %v", id, err) } } - // Pre-snapshot marker: acked before backup, MUST be captured even while - // a writer keeps the WAL non-empty (§9 backup fidelity). + // Pre-snapshot marker: acked before backup, MUST be captured even while a writer keeps the WAL non-empty (§9 backup fidelity). put("marker", `{"v":1}`) stop := make(chan struct{}) go func() { @@ -49,7 +48,6 @@ func TestBackupFidelity(t *testing.T) { t.Fatalf("marker missing from backup: %q ok=%v", raw, ok) } - // Replacement: a second backup overwrites and carries new state. put("marker", `{"v":2}`) if err := Backup(ctx, filepath.Join(dir, DBFileName), dest); err != nil { t.Fatalf("replace backup: %v", err) @@ -158,9 +156,7 @@ func TestBusyCtxDeadline(t *testing.T) { } } -// TestBackupConcurrentSameDest pins the flock serialization: without it, a -// concurrent run's stale-tmp cleanup yanks the first run's tmp mid-verify -// and publishes an empty backup with a nil error. +// TestBackupConcurrentSameDest pins the flock serialization: without it, a concurrent run's stale-tmp cleanup yanks the first run's tmp mid-verify and publishes an empty backup with a nil error. func TestBackupConcurrentSameDest(t *testing.T) { ctx := t.Context() dir := t.TempDir() diff --git a/meta/sqlite/multiprocess_test.go b/meta/sqlite/multiprocess_test.go index d681324c..e1f4a591 100644 --- a/meta/sqlite/multiprocess_test.go +++ b/meta/sqlite/multiprocess_test.go @@ -25,9 +25,7 @@ const ( inverseOps = 10 ) -// TestMultiProcessCorrectness is §9's real-process storm for the sqlite -// engine: acknowledged inserts and their name rows all present, names -// referentially consistent, zero corruption on reopen. +// TestMultiProcessCorrectness is §9's real-process storm for the sqlite engine: acknowledged inserts and their name rows all present, names referentially consistent, zero corruption on reopen. func TestMultiProcessCorrectness(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -151,10 +149,7 @@ func TestInverseScopeWorker(t *testing.T) { } } -// TestKillStormAtomicity is §9's commit-atomicity-under-crash gate for the -// sqlite engine: SIGKILL sampled across WAL-append and commit-frame windows; -// every multi-record transaction must reopen wholly applied or wholly absent, -// every acked transaction wholly applied. +// TestKillStormAtomicity is §9's commit-atomicity-under-crash gate for the sqlite engine: SIGKILL sampled across WAL-append and commit-frame windows; every multi-record transaction must reopen wholly applied or wholly absent, every acked transaction wholly applied. func TestKillStormAtomicity(t *testing.T) { if testing.Short() { t.Skip("multi-process gate skipped in -short") @@ -178,8 +173,7 @@ func TestKillStormAtomicity(t *testing.T) { if id, ok := strings.CutPrefix(sc.Text(), "ACK "); ok { acked[id] = struct{}{} got++ - // Kill at a round-varied depth so the SIGKILL lands in - // different WAL-append/commit windows. + // Kill at a round-varied depth so the SIGKILL lands in different WAL-append/commit windows. if got >= 3+round*2 { break } @@ -243,8 +237,7 @@ func TestKillStormWorker(t *testing.T) { } } -// updateRetry retries on ErrBusy — the mapped, retryable-by-contract outcome -// under extreme oversubscription (§4); reconciliation still demands exact counts. +// updateRetry retries on ErrBusy — the mapped, retryable-by-contract outcome under extreme oversubscription (§4); reconciliation still demands exact counts. func updateRetry(t *testing.T, s *Store, sc meta.Scope, fn func(meta.Writer) error) { t.Helper() for { diff --git a/meta/tombstone/tombstone.go b/meta/tombstone/tombstone.go index 636c3364..9559ef37 100644 --- a/meta/tombstone/tombstone.go +++ b/meta/tombstone/tombstone.go @@ -26,8 +26,7 @@ const ( ModeSubset Mode = "subset" ) -// ErrLost reports a fenced write that matched zero rows: another worker -// recovered and finalized this lease after its owner died. +// ErrLost reports a fenced write that matched zero rows: another worker recovered and finalized this lease after its owner died. var ErrLost = errors.New("tombstone lease lost") // Phase is the tombstone's protocol position. @@ -102,8 +101,7 @@ func (t *Table) TakeOver(ctx context.Context, w meta.Writer, id string) (*Record return rec, nil } -// MarkDeleting flips the tombstone to the deleting phase, fenced by leaseID; -// it is committed BEFORE any filesystem work. +// MarkDeleting flips the tombstone to the deleting phase, fenced by leaseID; it is committed before any filesystem work. func (t *Table) MarkDeleting(ctx context.Context, w meta.Writer, id, leaseID string) error { rec, err := t.fenced(ctx, w, id, leaseID) if err != nil { @@ -113,8 +111,7 @@ func (t *Table) MarkDeleting(ctx context.Context, w meta.Writer, id, leaseID str return t.recs.Replace(ctx, w, id, rec) } -// Rollback removes a still-leased tombstone (no filesystem work happened), -// fenced by leaseID. +// Rollback removes a still-leased tombstone (no filesystem work happened), fenced by leaseID. func (t *Table) Rollback(ctx context.Context, w meta.Writer, id, leaseID string) error { rec, err := t.fenced(ctx, w, id, leaseID) if err != nil { diff --git a/metadata/fat12.go b/metadata/fat12.go index 06837a15..33a845d7 100644 --- a/metadata/fat12.go +++ b/metadata/fat12.go @@ -34,8 +34,8 @@ const ( // 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) - rootDir []byte // root directory area + fat []byte // single FAT copy (written twice) + rootDir []byte data [][]byte // file data in cluster-allocation order nextCluster uint16 // next free cluster (starts at 2) rootUsed int // root directory entries consumed diff --git a/metering/metalog/metalog.go b/metering/metalog/metalog.go index 7cc280ce..3346cabd 100644 --- a/metering/metalog/metalog.go +++ b/metering/metalog/metalog.go @@ -1,6 +1,4 @@ -// Package metalog records metering entries in the meta store's log (§1 P3): -// one Relaxed append per entry — the file backend's no-fsync durability — -// with a Seq cursor committed atomically alongside each entry. +// Package metalog records metering entries in the meta store's log (§1 P3): one Relaxed append per entry (the file backend's no-fsync durability) with the Seq cursor committed alongside. package metalog import ( @@ -20,8 +18,7 @@ const ( var _ metering.Recorder = (*Recorder)(nil) -// Recorder appends entries through the meta store; Emit swallows errors so -// callers never block (metering contract). +// Recorder appends entries through the meta store; Emit swallows errors so callers never block. type Recorder struct { store meta.Store log *meta.Log[metering.Entry] diff --git a/network/bridge/bridge_linux.go b/network/bridge/bridge_linux.go index bb9be5bc..52ee6b81 100644 --- a/network/bridge/bridge_linux.go +++ b/network/bridge/bridge_linux.go @@ -46,7 +46,7 @@ func New(conf *config.Config, bridgeDev string) (*Bridge, error) { return nil, fmt.Errorf("%s is not a bridge (type: %s)", bridgeDev, br.Type()) } return &Bridge{ - tapPrefix: network.BridgeTAPPrefix(conf.NetScope), + tapPrefix: conf.BridgeTAPPrefix(), bridgeDev: bridgeDev, bridgeIdx: br.Attrs().Index, }, nil diff --git a/network/cni/cni.go b/network/cni/cni.go index 817a0fa5..6c00644f 100644 --- a/network/cni/cni.go +++ b/network/cni/cni.go @@ -247,9 +247,7 @@ func loadConfLists(dir string) (map[string]*libcni.NetworkConfigList, string, er return nil, "", fmt.Errorf("parse %s: %w", f, parseErr) } lists[cl.Name] = cl - if defaultName == "" { - defaultName = cl.Name - } + defaultName = cmp.Or(defaultName, cl.Name) } return lists, defaultName, nil } diff --git a/network/cni/cni_test.go b/network/cni/cni_test.go index 2f667778..f37c00dd 100644 --- a/network/cni/cni_test.go +++ b/network/cni/cni_test.go @@ -176,8 +176,7 @@ func TestRemoveKeepsFailedNICRecords(t *testing.T) { ctx := t.Context() seedRecords(t, c, "vm1", "eth0", "eth1") - // eth1's CNI DEL fails: its record must survive the sweep so vm rm/GC/retry can - // still release the IPAM lease; eth0's record must be swept. + // eth1's CNI DEL fails: its record must survive the sweep so vm rm/GC/retry can still release the IPAM lease; eth0's record must be swept. if err := c.Remove(ctx, "vm1", 0, 1); err == nil || !strings.Contains(err.Error(), "eth1") { t.Fatalf("Remove err = %v, want eth1 failure", err) } @@ -196,8 +195,7 @@ func TestRemoveSweepsDuplicateIfNameRecords(t *testing.T) { stubLifecycleSeams(t) ctx := t.Context() - // A failed reclaim can leave two records for one ifname; Remove must tear down - // and sweep both (DEL is idempotent), not strand one as a phantom. + // A failed reclaim can leave two records for one ifname; Remove must tear down and sweep both (DEL is idempotent), not strand one as a phantom. 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"}) @@ -271,8 +269,7 @@ func TestReclaimStaleNIC(t *testing.T) { } assertRecordIDs(t, c, []string{"n-eth1"}) - // TAP-delete failure also keeps the record: sweeping would leave a live TAP - // that collides with the re-add's CreateTAP, with nothing left to retry it. + // TAP-delete failure also keeps the record: sweeping would leave a live TAP that collides with the re-add's CreateTAP, with nothing left to retry it. exec.failIf = "" deleteTAPFn = func(string, string) error { return fmt.Errorf("device busy") } if err := c.reclaimStaleNIC(ctx, "vm1", "/run/netns/vm1", rec); err == nil { @@ -295,8 +292,7 @@ func TestAddFailsClosedOnStaleReclaim(t *testing.T) { ctx := t.Context() seedRecords(t, c, "vm1", "eth0") - // The stale record's DEL fails: Add must fail instead of double-allocating (lenient - // IPAM) or burying the root cause under the ADD failure (strict IPAM). + // The stale record's DEL fails: Add must fail instead of double-allocating (lenient IPAM) or burying the root cause under the ADD failure (strict IPAM). exec.failIf = "eth0" if _, err := c.Add(ctx, "vm1", testVMCfg(), network.AddSpec{Index: 0}); err == nil || !strings.Contains(err.Error(), "reclaim stale NIC") { t.Fatalf("Add err = %v, want reclaim failure", err) @@ -336,8 +332,7 @@ func TestQuiesceSkipsMissingNetns(t *testing.T) { seedRecords(t, c, "vm1", "eth0") - // A host reboot wipes every netns while the records survive; with no plumbing - // left, a stop's pending quiesce must settle instead of retrying forever. + // A host reboot wipes every netns while the records survive; with no plumbing left, a stop's pending quiesce must settle instead of retrying forever. if err := c.Quiesce(t.Context(), "vm1"); err != nil { t.Fatalf("Quiesce with a missing netns: %v", err) } diff --git a/network/cni/config.go b/network/cni/config.go index 206ca295..a57d60c5 100644 --- a/network/cni/config.go +++ b/network/cni/config.go @@ -4,7 +4,6 @@ import ( "path/filepath" "github.com/cocoonstack/cocoon/config" - "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/utils" ) @@ -35,5 +34,5 @@ func (c *Config) netnsPath(vmID string) string { } func (c *Config) netnsName(vmID string) string { - return network.NetnsPrefix(c.NetScope) + vmID + return c.NetnsPrefix() + vmID } diff --git a/network/cni/gc.go b/network/cni/gc.go index 42e10c3b..07199266 100644 --- a/network/cni/gc.go +++ b/network/cni/gc.go @@ -13,7 +13,6 @@ import ( "github.com/cocoonstack/cocoon/gc" "github.com/cocoonstack/cocoon/lock/vmlock" - "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/utils" ) @@ -25,7 +24,7 @@ type cniSnapshot struct { // GCModule returns the GC module for orphan netns and stale CNI record cleanup. func (c *CNI) GCModule() gc.Module[cniSnapshot] { // The prefix scopes GC to this installation's netns, so docker/containerd and peer-installation entries survive. - netnsPrefix := network.NetnsPrefix(c.conf.NetScope) + netnsPrefix := c.conf.NetnsPrefix() return gc.Module[cniSnapshot]{ Name: typ, Recover: c.gcRecover, diff --git a/network/cni/lifecycle_linux.go b/network/cni/lifecycle_linux.go index 14994846..f0d239f0 100644 --- a/network/cni/lifecycle_linux.go +++ b/network/cni/lifecycle_linux.go @@ -1,6 +1,7 @@ package cni import ( + "cmp" "context" "errors" "fmt" @@ -133,10 +134,7 @@ func tcRedirectInNS(ifName, tapName string, queues int, overrideMAC string) (str } } - mac := link.Attrs().HardwareAddr.String() - if overrideMAC != "" { - mac = overrideMAC - } + mac := cmp.Or(overrideMAC, link.Attrs().HardwareAddr.String()) addrs, err := netlink.AddrList(link, netlink.FAMILY_ALL) if err != nil { diff --git a/network/cni/teardown_test.go b/network/cni/teardown_test.go index 98b64766..6ba99b1c 100644 --- a/network/cni/teardown_test.go +++ b/network/cni/teardown_test.go @@ -15,10 +15,7 @@ import ( "github.com/cocoonstack/cocoon/network" ) -// TestSubsetTeardownRecovery is the design §9 subset gate: crash mid-deleting -// on a one-NIC `vm net remove`, then recover — the untouched NIC row must -// survive and the netns must not be removed. An aggregate-shaped test passes -// while this fails, which is why it exists separately. +// TestSubsetTeardownRecovery is the design §9 subset gate: crash mid-deleting on a one-NIC `vm net remove`, then recover — the untouched NIC row survives and the netns stays (an aggregate-shaped test would pass while this fails). func TestSubsetTeardownRecovery(t *testing.T) { c, exec := newTestCNIWithStore(t) stubLifecycleSeams(t) @@ -64,15 +61,13 @@ func TestSubsetTeardownRecovery(t *testing.T) { if netnsRemoved != 0 { t.Fatalf("subset recovery removed the netns (%d removals)", netnsRemoved) } - // Remove ran with deleteTAP=true; recovery restores it for exactly the - // subset's TAP and no other. + // Remove ran with deleteTAP=true; recovery restores it for exactly the subset's TAP and no other. if want := []string{tapNameForVM("vm1", 1)}; !slices.Equal(tapsDeleted, want) { t.Fatalf("subset recovery TAP deletions = %v, want %v", tapsDeleted, want) } } -// TestAggregateTeardownRecovery rolls a deleting aggregate forward: all rows -// and the netns go. +// TestAggregateTeardownRecovery rolls a deleting aggregate forward: all rows and the netns go. func TestAggregateTeardownRecovery(t *testing.T) { c, _ := newTestCNIWithStore(t) stubLifecycleSeams(t) @@ -96,9 +91,7 @@ func TestAggregateTeardownRecovery(t *testing.T) { } } -// TestAggregateRecoveryAfterNetnsGone converges the crash window between -// netns removal and the record sweep: TAP deletion is skipped (the TAPs died -// with the ns), DELs still run, rows sweep, the tombstone finalizes. +// TestAggregateRecoveryAfterNetnsGone converges the crash window between netns removal and the record sweep: TAP deletion is skipped (the TAPs died with the ns), DELs still run, rows sweep, the tombstone finalizes. func TestAggregateRecoveryAfterNetnsGone(t *testing.T) { c, _ := newTestCNIWithStore(t) stubLifecycleSeams(t) @@ -148,8 +141,7 @@ func TestAggregateRecoveryAfterNetnsGone(t *testing.T) { } } -// TestSubsetFailureKeepsTombstone pins the retry story: a failing DEL keeps -// the row and the subset tombstone, and the error names the retry path. +// TestSubsetFailureKeepsTombstone pins the retry story: a failing DEL keeps the row and the subset tombstone, and the error names the retry path. func TestSubsetFailureKeepsTombstone(t *testing.T) { c, exec := newTestCNIWithStore(t) exec.failIf = "eth1" @@ -181,8 +173,7 @@ func TestSubsetFailureKeepsTombstone(t *testing.T) { assertRecordIDs(t, c, []string{"n-eth0"}) } -// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json -// and requires byte-identical output to the legacy storage layer's writes. +// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json and requires byte-identical output to the legacy storage layer's writes. func TestLegacyDifferentialTrace(t *testing.T) { ctx := t.Context() dir := t.TempDir() @@ -240,9 +231,7 @@ func TestLegacyDifferentialTrace(t *testing.T) { } } -// TestAddRefusesAfterAggregateRollForward pins the entry rule: recovery of a -// whole-VM deleting tombstone completes the teardown AND refuses the Add; a -// retry then proceeds on the clean slate. +// TestAddRefusesAfterAggregateRollForward pins the entry rule: recovery of a whole-VM deleting tombstone completes the teardown and refuses the Add; a retry then proceeds on the clean slate. func TestAddRefusesAfterAggregateRollForward(t *testing.T) { c, _ := newTestCNIWithStore(t) stubLifecycleSeams(t) @@ -277,9 +266,7 @@ func TestAddRefusesAfterAggregateRollForward(t *testing.T) { } } -// TestAddRefusesAfterSubsetRollForward pins the §5 binding rule without a -// mode exception: even a subset deleting recovery fails the current Add; the -// retry proceeds from the recovered state. +// TestAddRefusesAfterSubsetRollForward pins the §5 binding rule without a mode exception: even a subset deleting recovery fails the current Add; the retry proceeds from the recovered state. func TestAddRefusesAfterSubsetRollForward(t *testing.T) { c, _ := newTestCNIWithStore(t) stubLifecycleSeams(t) diff --git a/snapshot/localfile/gc_test.go b/snapshot/localfile/gc_test.go index e66e63a8..8caaae49 100644 --- a/snapshot/localfile/gc_test.go +++ b/snapshot/localfile/gc_test.go @@ -486,9 +486,7 @@ func TestGCModule_OrphanAndStalePendingDoNotEmit(t *testing.T) { } } -// TestGCCollectsFreshPendingWithFreeLease pins the lease-free ownerless proof: -// a dead save's pending record is reclaimed on the next pass, however young — -// the free build lease is the proof, not an age gate. +// TestGCCollectsFreshPendingWithFreeLease pins the ownerless proof: a dead save's pending record is reclaimed on the next pass however young — the free build lease is the proof, not an age gate. func TestGCCollectsFreshPendingWithFreeLease(t *testing.T) { lf := newTestLF(t) ctx := t.Context() @@ -520,8 +518,7 @@ func TestGCCollectsFreshPendingWithFreeLease(t *testing.T) { } } -// TestGCSkipsPendingHeldByLiveBuild pins the other half of the proof: a save -// still holding its build lease must not lose its pending record to GC. +// TestGCSkipsPendingHeldByLiveBuild pins the other half of the proof: a save still holding its build lease must not lose its pending record to GC. func TestGCSkipsPendingHeldByLiveBuild(t *testing.T) { lf := newTestLF(t) ctx := t.Context() diff --git a/snapshot/localfile/localfile.go b/snapshot/localfile/localfile.go index 98ce39a0..6584e443 100644 --- a/snapshot/localfile/localfile.go +++ b/snapshot/localfile/localfile.go @@ -36,13 +36,16 @@ var osRename = os.Rename // seam for EXDEV fallback tests // Option configures a LocalFile constructed via New. type Option func(*LocalFile) +// BlobPinner holds the digest locks of an envelope's image pins while a commit lands. +type BlobPinner func(context.Context, map[string]struct{}) (func(), error) + // WithGCPolicy attaches an LRU eviction policy used by RegisterGC. func WithGCPolicy(p EvictionPolicy) Option { return func(lf *LocalFile) { lf.gcPolicy = p } } // WithBlobPinner injects the digest-lock hook Import holds while committing an envelope's image pins. -func WithBlobPinner(fn func(context.Context, map[string]struct{}) (func(), error)) Option { +func WithBlobPinner(fn BlobPinner) Option { return func(lf *LocalFile) { lf.pinBlobs = fn } } @@ -60,7 +63,7 @@ type LocalFile struct { meta meta.Store metering metering.Recorder gcPolicy EvictionPolicy - pinBlobs func(context.Context, map[string]struct{}) (func(), error) + pinBlobs BlobPinner } // New builds a LocalFile snapshot backend; rec may be nil and falls back to NopRecorder on emit. diff --git a/snapshot/localfile/localfile_test.go b/snapshot/localfile/localfile_test.go index 8b5fe850..5301f5e8 100644 --- a/snapshot/localfile/localfile_test.go +++ b/snapshot/localfile/localfile_test.go @@ -89,8 +89,7 @@ func TestCreateAndDeleteEmitMetering(t *testing.T) { } func TestDeleteOneIdempotentDoesNotEmitTwice(t *testing.T) { - // Racing rm: the loser's closure sees a nil rec and must report success - // without emitting a phantom stop. deleteOne twice on one id simulates it. + // Racing rm: the loser's closure sees a nil rec and must report success without emitting a phantom stop; deleteOne twice on one id simulates it. rec := meteringcapture.New() lf := newTestLFWithRecorder(t, rec) ctx := t.Context() @@ -135,8 +134,7 @@ func TestCreateFromDirDirectMatchesCreateLayout(t *testing.T) { "cocoon.json": []byte(`{"storage_configs":[]}`), } - // A capture dir under the store root shares its filesystem, so the direct - // (rename) path is taken rather than the cross-fs streaming fallback. + // A capture dir under the store root shares its filesystem, so the direct (rename) path is taken rather than the cross-fs streaming fallback. srcDir := writeCaptureDir(t, filepath.Join(lf.conf.RootDir, "capture-src"), files) id, ok, err := lf.CreateFromDir(ctx, &types.SnapshotConfig{ ID: testID(t), Name: "direct-snap", Hypervisor: "cloud-hypervisor", @@ -197,9 +195,7 @@ func TestCreateFromDirEXDEVFallsBack(t *testing.T) { if _, statErr := os.Stat(srcDir); statErr != nil { t.Errorf("srcDir removed on EXDEV fallback: %v", statErr) } - // Check the index directly: Inspect hides pending records, so it cannot - // distinguish "rolled back" from "stale pending left behind" — and a stale - // name reservation would fail the tar-fallback Create with "already in use". + // Check the index directly: Inspect hides pending records, so it cannot tell "rolled back" from "stale pending left behind", and a stale name reservation would fail the tar-fallback Create. if err := lf.dbRead(ctx, func(idx *snapshotIndex) error { if _, stale := idx.Snapshots[id]; stale { return fmt.Errorf("pending record %s still in index", id) @@ -270,10 +266,7 @@ func TestRollbackCreateSurvivesCanceledContext(t *testing.T) { } } -// TestNameOwnerSeesPendingReservation pins what a killed save leaves behind: the -// pending record still holds the name in the index, so Inspect reports not-found -// while insertRecord rejects the reuse. The save preflight resolves through the -// index for exactly this reason — otherwise the capture runs to completion first. +// TestNameOwnerSeesPendingReservation pins what a killed save leaves behind: the pending record still holds the name, so Inspect reports not-found while insertRecord rejects the reuse; the save preflight resolves through the index for exactly this reason. func TestNameOwnerSeesPendingReservation(t *testing.T) { lf := newTestLF(t) ctx := t.Context() @@ -871,8 +864,7 @@ func TestRestore_CloseWaitsForGoroutine(t *testing.T) { } if err := rc.Close(); err != nil { - // A broken pipe or similar error is acceptable here since we didn't - // consume the stream — but it must not hang or panic. + // A broken pipe is acceptable since the stream was not consumed, but it must not hang or panic. t.Logf("Close returned (expected) error: %v", err) } } @@ -1116,8 +1108,7 @@ func TestImport_CorruptGzipTrailerRejected(t *testing.T) { tw.Close() gw.Close() - // Flip a bit in the gzip ISIZE trailer: tar extraction still succeeds, so - // only the drain-to-EOF integrity check can catch it. + // Flip a bit in the gzip ISIZE trailer: tar extraction still succeeds, so only the drain-to-EOF integrity check can catch it. raw := buf.Bytes() raw[len(raw)-1] ^= 0xff diff --git a/snapshot/localfile/meta_shim_test.go b/snapshot/localfile/meta_shim_test.go index 84a94566..7a979ff6 100644 --- a/snapshot/localfile/meta_shim_test.go +++ b/snapshot/localfile/meta_shim_test.go @@ -12,8 +12,7 @@ import ( "github.com/cocoonstack/cocoon/snapshot" ) -// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json -// and requires byte-identical output to the legacy storage layer's writes. +// TestLegacyDifferentialTrace replays the fixture op sequence over meta-json and requires byte-identical output to the legacy storage layer's writes. func TestLegacyDifferentialTrace(t *testing.T) { ctx := t.Context() dir := t.TempDir() diff --git a/snapshot/localfile/teardown.go b/snapshot/localfile/teardown.go index 1b0745b0..79129e32 100644 --- a/snapshot/localfile/teardown.go +++ b/snapshot/localfile/teardown.go @@ -1,6 +1,7 @@ package localfile import ( + "cmp" "context" "encoding/json" "errors" @@ -49,10 +50,7 @@ func (lf *LocalFile) deleteSnapshotProtocol(ctx context.Context, id string, reva return nil } hyp = rec.Hypervisor - cl = snapCleanup{Name: rec.Name, DataDir: rec.DataDir} - if cl.DataDir == "" { - cl.DataDir = lf.conf.SnapshotDataDir(id) - } + cl = snapCleanup{Name: rec.Name, DataDir: cmp.Or(rec.DataDir, lf.conf.SnapshotDataDir(id))} } var resumed *tombstone.Record leaseID, resumed, err = ts.Acquire(ctx, t.Writer(), id, func() (tombstone.Payload, error) { diff --git a/snapshot/snapshot.go b/snapshot/snapshot.go index 1e3b4c81..e93cdbe2 100644 --- a/snapshot/snapshot.go +++ b/snapshot/snapshot.go @@ -39,7 +39,6 @@ type Snapshot interface { // Create persists a snapshot from the given config and data stream, returning the snapshot ID. Create(ctx context.Context, cfg *types.SnapshotConfig, stream io.Reader) (string, error) - // List returns all snapshots. List(ctx context.Context) ([]*types.Snapshot, error) // Inspect returns a single snapshot by ID or name. Inspect(ctx context.Context, ref string) (*types.Snapshot, error) diff --git a/types/boot.go b/types/boot.go index 398a80b5..f3802bc1 100644 --- a/types/boot.go +++ b/types/boot.go @@ -2,12 +2,10 @@ package types // BootConfig holds kernel and firmware paths used to boot a VM. type BootConfig struct { - // Direct-boot fields (OCI images). KernelPath string `json:"kernel_path,omitempty"` InitrdPath string `json:"initrd_path,omitempty"` // Cmdline is the direct-boot kernel command line, set at Create from the storage layout (cocoon.layers=, cocoon.cow=, …). Cmdline string `json:"cmdline,omitempty"` - // UEFI-boot field (cloud images). FirmwarePath string `json:"firmware_path,omitempty"` } diff --git a/types/storage.go b/types/storage.go index 1f3f1692..a502dd76 100644 --- a/types/storage.go +++ b/types/storage.go @@ -92,13 +92,7 @@ func ValidateStorageConfigs(configs []*StorageConfig) error { // ValidDataDiskName reports whether s is a legal data disk name; shared with untrusted sidecar loading. func ValidDataDiskName(s string) bool { - if !dataDiskNameRe.MatchString(s) { - return false - } - if strings.HasPrefix(s, "cocoon-") { - return false - } - return true + return dataDiskNameRe.MatchString(s) && !strings.HasPrefix(s, "cocoon-") } func validDataDiskFSType(t string) bool { diff --git a/types/vm.go b/types/vm.go index 82c6595f..d9736b3e 100644 --- a/types/vm.go +++ b/types/vm.go @@ -71,6 +71,9 @@ func (cfg *VMConfig) Validate() error { if cfg.DiskQueueSize < 0 { return fmt.Errorf("--disk-queue-size must be non-negative, got %d", cfg.DiskQueueSize) } + if cfg.Mergeable && (cfg.HugePages || cfg.SharedMemory) { + return fmt.Errorf("--mergeable needs plain private memory; drop --hugepages/--shared-memory") + } if cfg.User != "" && !validUsername.MatchString(cfg.User) { return fmt.Errorf("--user %q is invalid: must be a lowercase Linux username (letters, digits, underscores, hyphens)", cfg.User) } diff --git a/utils/file.go b/utils/file.go index ca981c01..481bcafb 100644 --- a/utils/file.go +++ b/utils/file.go @@ -60,8 +60,7 @@ func FileHead(f *os.File, n int) ([]byte, error) { return buf[:m], nil } -// FileExists reports bare existence; ValidFile additionally demands a -// non-empty regular file. +// FileExists reports bare existence; ValidFile additionally demands a non-empty regular file. func FileExists(path string) bool { _, err := os.Stat(path) return err == nil diff --git a/utils/http_test.go b/utils/http_test.go index 5295a9c7..099f9769 100644 --- a/utils/http_test.go +++ b/utils/http_test.go @@ -151,8 +151,8 @@ func TestDoAPI_StatusMismatch_ReturnsAPIError(t *testing.T) { if err == nil { t.Fatal("expected error") } - var ae *APIError - if !errors.As(err, &ae) { + ae, ok := errors.AsType[*APIError](err) + if !ok { t.Fatalf("expected APIError, got %T: %v", err, err) } if ae.Code != http.StatusInternalServerError { @@ -169,8 +169,7 @@ func TestDoAPI_ConnectionError(t *testing.T) { if err == nil { t.Fatal("expected connection error") } - var ae *APIError - if errors.As(err, &ae) { + if ae, ok := errors.AsType[*APIError](err); ok { t.Errorf("expected non-APIError, got APIError{%d}", ae.Code) } } @@ -294,8 +293,7 @@ func TestDoWithRetry_NonRetryableError_StopsImmediately(t *testing.T) { if calls != 1 { t.Errorf("expected 1 call (non-retryable), got %d", calls) } - var ae *APIError - if !errors.As(err, &ae) || ae.Code != 404 { + if ae, ok := errors.AsType[*APIError](err); !ok || ae.Code != 404 { t.Errorf("expected APIError{404}, got %v", err) } } diff --git a/utils/process_test.go b/utils/process_test.go index 4e7fb099..d22bda7a 100644 --- a/utils/process_test.go +++ b/utils/process_test.go @@ -155,8 +155,7 @@ func TestTerminateProcess_SleepProcess(t *testing.T) { pid := cmd.Process.Pid waitForExec(t, pid, "sleep", "60") - // Reap the child in background so it doesn't become a zombie after SIGTERM. - // Without this, kill(pid, 0) keeps returning nil for zombies and WaitFor times out. + // Reap the child in background: kill(pid, 0) keeps returning nil for a zombie and WaitFor would time out. waitDone := make(chan struct{}) go func() { _ = cmd.Wait() @@ -208,8 +207,7 @@ func TestTerminateProcess_InvalidPID(t *testing.T) { } func TestTerminateProcess_SIGTERMIgnored_FallsBackToKill(t *testing.T) { - // Traps SIGTERM so only SIGKILL can end it. The trailing "; :" stops bash from - // tail-exec'ing into sleep, which would rename argv0 and fail the "bash" cmdline check. + // Traps SIGTERM so only SIGKILL can end it; the trailing "; :" stops bash from tail-exec'ing into sleep, which would rename argv0 and fail the "bash" cmdline check. cmd := exec.Command("bash", "-c", `trap "" TERM; sleep 60; :`) if err := cmd.Start(); err != nil { t.Fatalf("start: %v", err) @@ -296,8 +294,7 @@ func TestTerminateProcess_ContextCancelled(t *testing.T) { _ = TerminateProcess(ctx, pid, "sleep", "60", 100*time.Millisecond) } -// waitForExec parks until the child's execve lands: cmd.Start returns -// pre-exec, and TerminateProcess declines a mismatched cmdline (#87 race). +// waitForExec parks until the child's execve lands: cmd.Start returns pre-exec, and TerminateProcess declines a mismatched cmdline (#87 race). func waitForExec(t *testing.T, pid int, binaryName, expectArg string) { t.Helper() if err := WaitFor(t.Context(), 5*time.Second, time.Millisecond, func() (bool, error) { diff --git a/utils/tar_sparse_linux.go b/utils/tar_sparse_linux.go index 1cd2c98c..bc3016bb 100644 --- a/utils/tar_sparse_linux.go +++ b/utils/tar_sparse_linux.go @@ -51,7 +51,6 @@ func tarFileMaybeSparse(tw *tar.Writer, path, nameInTar string) error { return fmt.Errorf("marshal sparse map for %s: %w", path, err) } - // Segment-map JSON exceeds tar's 1MB PAX cap. Fall back to non-sparse. if len(mapJSON) > maxSparseMapJSONSize { return rewindAndTarFull(tw, f, fi, path, nameInTar) }