From 86bf631bda32212fe7b3d382b7f663c8c1cee0cf Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Wed, 1 Jul 2026 12:31:02 +0200 Subject: [PATCH] fix(disks): eliminate DragonFly disklabel fan-out and md phantoms The DragonFly disks/partitions probe spawned disklabel on device+s1..s4 for every kern.disks device (~660 spawns on a 132-device guest, 127 of them empty md memory disks) and leaked those md disks into the disks fact as 9.77 MB phantoms. - dragonFlyPseudoDisk skips md/cd pseudo-devices (driver class) in both the disks list and the partition probe. - dragonFlyDisklabelTargets now globs the slice nodes that actually exist (/dev/s) instead of the fixed s1..s4, and drops the never-productive whole-disk target (the DragonFly label lives on the slice). - currentDragonFlyPartitions takes s.glob; unattached vn is handled by the existing size gate + empty glob, attached vn is kept. Scope is DragonFly-only: nlab ground truth shows OpenBSD/NetBSD hw.disknames is kernel-curated to attached instances (no md/rd/cd leak), and filtering rd there would be harmful in a bsd.rd root boot. Validated on the nlab DragonFly guest: gate green, disks={da0} only, partitions unchanged, discovery 1m42s->0.65s, disklabel spawns ~660->~1. --- CHANGELOG.md | 6 + internal/engine/disks.go | 60 +++++++- internal/engine/disks_test.go | 137 ++++++++++++++---- .../optimize-bsd-disks-probe/design.md | 16 +- .../optimize-bsd-disks-probe/proposal.md | 34 ++++- .../go-port-supported-platform-facts/spec.md | 26 ++-- .../changes/optimize-bsd-disks-probe/tasks.md | 18 ++- 7 files changed, 227 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e22a168..1102377f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,12 @@ ### Fixed +- The DragonFly `disks`/`partitions` probe no longer reports empty memory-disk + (`md`) phantoms and no longer fans out `disklabel` across non-existent slice + targets. It skips `md`/`cd` pseudo-devices and enumerates only the slice nodes + that actually exist (`/dev/s`), so on a host with many memory disks + discovery drops from hundreds of `disklabel` spawns to a handful. Real disks, + their partitions, and attached file-backed (`vn`) disks are unchanged. - Linux DHCP lease discovery now matches explicit dhclient interface names exactly, so a lease for a similarly named interface such as `eth0-backup` is not attributed to `eth0`, and parses lease blocks without being confused diff --git a/internal/engine/disks.go b/internal/engine/disks.go index 29c57c8a..8f7be04e 100644 --- a/internal/engine/disks.go +++ b/internal/engine/disks.go @@ -171,7 +171,7 @@ func currentPartitions(s *Session) map[string]any { case "freebsd": return parseFreeBSDGeomPartitions(s.commandOutput("sysctl", "-n", "kern.geom.confxml")) case "dragonfly": - return currentDragonFlyPartitions(s.commandOutput) + return currentDragonFlyPartitions(s.commandOutput, s.glob) case "openbsd": return currentOpenBSDPartitions(s.commandOutput) case "netbsd": @@ -474,6 +474,9 @@ func currentDragonFlyDisks(run commandRunner) map[string]any { } disks := map[string]any{} for _, device := range strings.Fields(run("sysctl", "-n", "kern.disks")) { + if dragonFlyPseudoDisk(device) { + continue + } disk := parseDragonFlyDiskInfo(run("diskinfo", "/dev/"+device)) if len(disk) > 0 { disks[device] = disk @@ -661,13 +664,16 @@ func parseBSDDisklabelInt(input, key string) int { return 0 } -func currentDragonFlyPartitions(run commandRunner) map[string]any { - if run == nil { +func currentDragonFlyPartitions(run commandRunner, glob pathGlobber) map[string]any { + if run == nil || glob == nil { return nil } partitions := map[string]any{} for _, device := range strings.Fields(run("sysctl", "-n", "kern.disks")) { - for _, target := range dragonFlyDisklabelTargets(device) { + if dragonFlyPseudoDisk(device) { + continue + } + for _, target := range dragonFlyDisklabelTargets(device, glob) { for name, partition := range parseDragonFlyDisklabelPartitions(target, run("disklabel", target)) { partitions[name] = partition } @@ -679,8 +685,50 @@ func currentDragonFlyPartitions(run commandRunner) map[string]any { return partitions } -func dragonFlyDisklabelTargets(device string) []string { - return []string{device, device + "s1", device + "s2", device + "s3", device + "s4"} +// dragonFlyDisklabelTargets returns the slice nodes that actually exist for a +// device (glob /dev/s). The DragonFly disklabel lives on the slice, +// not the whole disk, so the whole-disk target is deliberately omitted. +func dragonFlyDisklabelTargets(device string, glob pathGlobber) []string { + matches, err := glob("/dev/" + device + "s*") + if err != nil { + return nil + } + var targets []string + for _, match := range matches { + name := strings.TrimPrefix(match, "/dev/") + if isDragonFlySlice(device, name) { + targets = append(targets, name) + } + } + return targets +} + +// isDragonFlySlice reports whether name is a whole-slice node for device +// (s), excluding partition-within-slice nodes like s1a. +func isDragonFlySlice(device, name string) bool { + rest, ok := strings.CutPrefix(name, device+"s") + if !ok || rest == "" { + return false + } + for _, r := range rest { + if r < '0' || r > '9' { + return false + } + } + return true +} + +// dragonFlyPseudoDisk reports whether a kern.disks device is a memory-disk or +// optical pseudo-device (driver class md or cd) rather than real host storage. +// Unattached vn is not listed here: the size gate already drops it from disks +// and it has no slice nodes, so the glob yields no disklabel spawns for it. +func dragonFlyPseudoDisk(name string) bool { + switch strings.TrimRight(name, "0123456789") { + case "md", "cd": + return true + default: + return false + } } func parseDragonFlyDisklabelPartitions(device, input string) map[string]any { diff --git a/internal/engine/disks_test.go b/internal/engine/disks_test.go index adbfb7d7..288367c5 100644 --- a/internal/engine/disks_test.go +++ b/internal/engine/disks_test.go @@ -792,46 +792,124 @@ func TestParseDragonFlyDisklabelPartitions_returnsDevicePartitions(t *testing.T) } } -func TestCurrentDragonFlyPartitionsReadsKernelDiskNames(t *testing.T) { - got := currentDragonFlyPartitions(func(name string, args ...string) string { +func TestCurrentDisksDragonFlySkipsMemoryDiskPhantoms(t *testing.T) { + // The DragonFly guest lists 127 md memory disks whose diskinfo returns a + // positive 9.77 MB size; they must not leak into the disks fact, and the + // probe must not even spawn diskinfo for them. + got := currentDisks("dragonfly", func(name string, args ...string) string { switch strings.Join(append([]string{name}, args...), " ") { case "sysctl -n kern.disks": - return "da0 vn3\n" - case "disklabel da0": - return "disklabel: Operation not supported by device\n" - case "disklabel da0s1": - return dragonFlyDisklabelDA0S1 - case "disklabel da0s2", "disklabel da0s3", "disklabel da0s4", - "disklabel vn3", "disklabel vn3s1", "disklabel vn3s2", "disklabel vn3s3", "disklabel vn3s4": - return "disklabel: Operation not supported by device\n" + return "md1 da0 vn0 md0\n" + case "diskinfo /dev/da0": + return "/dev/da0 blksize=512 offset=0x000000000000 size=0x002000000000 128.00 GB\n" + case "diskinfo /dev/vn0": + return "No such file or directory\n" default: - t.Fatalf("unexpected command %q %#v", name, args) + t.Fatalf("unexpected command %q %#v (memory disk probed?)", name, args) return "" } - }) + }, osHost{}) + want := map[string]any{ + "da0": map[string]any{"size": "128.00 GiB", "size_bytes": 137_438_953_472}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("currentDisks(dragonfly) = %#v, want %#v", got, want) + } +} + +func TestDragonFlyDisklabelTargetsGlobsExistingSlices(t *testing.T) { + t.Parallel() + glob := func(pattern string) ([]string, error) { + if pattern != "/dev/da0s*" { + t.Fatalf("glob pattern = %q, want /dev/da0s*", pattern) + } + // devfs exposes the slice node plus its partition-within-slice nodes. + return []string{"/dev/da0s1", "/dev/da0s1a", "/dev/da0s1b", "/dev/da0s1d"}, nil + } + got := dragonFlyDisklabelTargets("da0", glob) + if want := []string{"da0s1"}; !reflect.DeepEqual(got, want) { + t.Fatalf("dragonFlyDisklabelTargets() = %#v, want %#v", got, want) + } +} + +func TestCurrentDragonFlyPartitionsSkipsPseudoAndGlobsSlices(t *testing.T) { + t.Parallel() + var disklabelCalls []string + run := func(name string, args ...string) string { + if name == "disklabel" { + disklabelCalls = append(disklabelCalls, strings.Join(args, " ")) + if len(args) == 1 && args[0] == "da0s1" { + return dragonFlyDisklabelDA0S1 + } + return "disklabel: Operation not supported by device\n" + } + if strings.Join(append([]string{name}, args...), " ") == "sysctl -n kern.disks" { + return "md1 da0 vn0 md0\n" + } + t.Fatalf("unexpected command %q %#v", name, args) + return "" + } + glob := func(pattern string) ([]string, error) { + switch pattern { + case "/dev/da0s*": + return []string{"/dev/da0s1", "/dev/da0s1a", "/dev/da0s1b", "/dev/da0s1d"}, nil + case "/dev/vn0s*": // unattached vn: no slice nodes + return nil, nil + default: + t.Fatalf("unexpected glob %q (pseudo-device globbed?)", pattern) + return nil, nil + } + } + + got := currentDragonFlyPartitions(run, glob) if _, ok := got["/dev/da0s1d"]; !ok { t.Fatalf("currentDragonFlyPartitions() = %#v, want /dev/da0s1d", got) } + // Only the real slice is probed: no md*, no whole-disk da0, no da0s2..s4, + // no unattached vn0. + if want := []string{"da0s1"}; !reflect.DeepEqual(disklabelCalls, want) { + t.Fatalf("disklabel calls = %#v, want %#v", disklabelCalls, want) + } } -func TestCurrentDragonFlyPartitionsTriesOtherSlices(t *testing.T) { - got := currentDragonFlyPartitions(func(name string, args ...string) string { +func TestCurrentDragonFlyKeepsAttachedVnode(t *testing.T) { + // An attached vn (file-backed disk) has a positive diskinfo size and real + // slice nodes; it is NOT a pseudo-device and must stay in disks with its + // partitions probed. This guards the "keep attached vn" contract against a + // regression that wrongly adds vn to dragonFlyPseudoDisk. + t.Parallel() + run := func(name string, args ...string) string { switch strings.Join(append([]string{name}, args...), " ") { case "sysctl -n kern.disks": - return "da0\n" - case "disklabel da0", "disklabel da0s1": - return "disklabel: Operation not supported by device\n" - case "disklabel da0s2": - return strings.ReplaceAll(dragonFlyDisklabelDA0S1, "/dev/da0s1", "/dev/da0s2") - case "disklabel da0s3", "disklabel da0s4": - return "" + return "vn0 da0\n" + case "diskinfo /dev/vn0": + return "/dev/vn0 blksize=512 offset=0x000000000000 size=0x000040000000 1.00 GB\n" + case "diskinfo /dev/da0": + return "/dev/da0 blksize=512 offset=0x000000000000 size=0x002000000000 128.00 GB\n" + case "disklabel vn0s1", "disklabel da0s1": + return dragonFlyDisklabelDA0S1 default: t.Fatalf("unexpected command %q %#v", name, args) return "" } - }) - if _, ok := got["/dev/da0s2d"]; !ok { - t.Fatalf("currentDragonFlyPartitions() = %#v, want /dev/da0s2d", got) + } + glob := func(pattern string) ([]string, error) { + switch pattern { + case "/dev/vn0s*": + return []string{"/dev/vn0s1"}, nil + case "/dev/da0s*": + return []string{"/dev/da0s1"}, nil + default: + t.Fatalf("unexpected glob %q", pattern) + return nil, nil + } + } + + if disks := currentDisks("dragonfly", run, osHost{}); disks["vn0"] == nil { + t.Fatalf("currentDisks(dragonfly) = %#v, want attached vn0 kept", disks) + } + if parts := currentDragonFlyPartitions(run, glob); parts["/dev/vn0s1d"] == nil { + t.Fatalf("currentDragonFlyPartitions() = %#v, want /dev/vn0s1d", parts) } } @@ -1390,21 +1468,16 @@ func TestCurrentPartitionsDispatchesBySessionPlatform(t *testing.T) { platform: "dragonfly", runOutputs: map[string]string{ fakeRunKey("sysctl", "-n", "kern.disks"): "da0\n", - fakeRunKey("disklabel", "da0"): "disklabel: Operation not supported by device\n", fakeRunKey("disklabel", "da0s1"): dragonFlyDisklabelDA0S1, - fakeRunKey("disklabel", "da0s2"): "disklabel: Operation not supported by device\n", - fakeRunKey("disklabel", "da0s3"): "disklabel: Operation not supported by device\n", - fakeRunKey("disklabel", "da0s4"): "disklabel: Operation not supported by device\n", + }, + globs: map[string][]string{ + "/dev/da0s*": {"/dev/da0s1", "/dev/da0s1a", "/dev/da0s1b", "/dev/da0s1d"}, }, }, wantKey: "/dev/da0s1a", wantCalls: []fakeHostRunCall{ {name: "sysctl", args: []string{"-n", "kern.disks"}}, - {name: "disklabel", args: []string{"da0"}}, {name: "disklabel", args: []string{"da0s1"}}, - {name: "disklabel", args: []string{"da0s2"}}, - {name: "disklabel", args: []string{"da0s3"}}, - {name: "disklabel", args: []string{"da0s4"}}, }, }, } diff --git a/openspec/changes/optimize-bsd-disks-probe/design.md b/openspec/changes/optimize-bsd-disks-probe/design.md index cebd5896..fcf47276 100644 --- a/openspec/changes/optimize-bsd-disks-probe/design.md +++ b/openspec/changes/optimize-bsd-disks-probe/design.md @@ -1,11 +1,15 @@ ## Decisions -- **Denylist known pseudo-devices, do not whitelist real ones.** Filter the `kern.disks` device list to exclude the pseudo-device prefixes `md` (memory disk), `vn` (vnode/file-backed disk), and `cd` (optical) before probing. A denylist of well-known pseudo-devices is robust to new real-disk driver prefixes; a whitelist would silently drop unfamiliar real disks. -- **DragonFly: stop the unconditional five-target fan-out.** Instead of spawning `disklabel` on ``, `s1`…`s4` for every device, probe the device and only the slices that actually exist. Investigate whether one `disklabel ` already enumerates the slices, avoiding per-slice spawns entirely. -- **Behavior-preserving for real disks.** The `disks`/`partitions` facts for real storage must be byte-identical to today; only pseudo-devices are removed and only wasted subprocesses are eliminated. +- **DragonFly-only, grounded in nlab probing.** The perf pathology and the phantom leak both live on DragonFly. OpenBSD (`hw.disknames = sd0,fd0`) and NetBSD (`hw.disknames = sd0 dk0 dk1`) never list ram/memory/optical pseudo-devices — the kernel already curates the list to attached instances — so there is nothing to filter and no fan-out to bound. This change does not touch their code paths. +- **Do NOT filter `rd`/`md`/`cd` on OpenBSD/NetBSD.** Besides being dead code (they never appear in `hw.disknames`), filtering `rd` would be *harmful*: in a `bsd.rd` recovery/installer boot the root disk *is* `rd0`, and the NetBSD root here mounts from `dk0` — a real GPT wedge already recovered as a partition by the existing `dkctl listwedges` path. Never denylist a class that can legitimately be the root device. +- **Match pseudo-devices by driver class, not prefix.** Strip trailing digits from the `kern.disks` name (`md126` → `md`, `da0` → `da`, `vn0` → `vn`) and compare against the DragonFly pseudo set `{md, cd}`. `strings.HasPrefix` would misclassify a hypothetical real `mdXX`-style driver; class comparison is exact. +- **Enumerate real slices instead of guessing `s1..s4`.** DragonFly slice device nodes exist under `/dev` only for slices that exist (`da0` → `/dev/da0s1` only). Glob `/dev/s*`, keep names of the form `s` (a slice, not a `s1a` partition-within-slice), and probe exactly those. Drop the whole-disk target: `disklabel ` is always `Operation not supported by device` on DragonFly (the label lives on the slice). +- **Leave `vn` to the existing gates.** Unattached `vn` (the only kind present) is already excluded from `disks` because `diskinfo` returns no size, and it produces no slice nodes (`/dev/vn0s*` is empty) so the glob yields zero `disklabel` spawns. Special-casing it (e.g. `vnconfig -l`) would add a subprocess to solve a problem the size-gate and glob already solve. Attached `vn` has slice nodes and a size, so it is kept — correct. +- **`mountpoints` stays unfiltered** — a mounted `md`/`vn` (tmpmfs, attached image) still surfaces there, so nothing mounted becomes invisible. ## Implementation Notes -- Add a shared helper (`isPseudoDisk(name) bool` / `filterRealDisks([]string) []string`) and apply it wherever a BSD probe iterates `kern.disks`: `currentDragonFlyPartitions`, `currentOpenBSDPartitions`, `currentNetBSDPartitions`, and the disks-list enumeration in `disks.go`. -- TDD: table tests with a fake `commandRunner` asserting (a) `md*`/`vn*`/`cd*` devices are skipped — no `disklabel` spawned for them (record the runner's calls), (b) real devices are still probed and parsed, (c) partition output for real disks is unchanged. -- Validate on the nlab DragonFly/OpenBSD/NetBSD guests: discovery time and `disklabel` spawn count drop sharply, `partitions` excludes `md*`, real-disk facts are unchanged, and the native release gates still pass. +- Add `dragonFlyPseudoDisk(name string) bool` (driver-class: `strings.TrimRight(name, "0123456789")` ∈ `{md, cd}`). Apply it in `currentDragonFlyDisks` (skip before `diskinfo`) and `currentDragonFlyPartitions` (skip before the slice loop). Do not add a `goos` parameter — this is DragonFly-only. +- Rewrite `dragonFlyDisklabelTargets(device string, glob pathGlobber) []string` to glob `/dev/s*` and return the base names matching `s` (slice only). Thread `s.glob` into `currentDragonFlyPartitions` (the illumos partition probe already takes `s.glob`, so the wiring pattern exists). +- TDD with a recording `commandRunner` + fake `pathGlobber`: assert (a) **zero** `disklabel` spawns for `md*`, (b) `md*` absent from `disks` while `da0` is byte-identical, (c) only existing slices are probed — `disklabel da0s1` runs, `disklabel da0`/`da0s2..s4` do not, (d) unattached `vn` (empty glob) yields no spawns. +- Validate on the nlab DragonFly guest: `disklabel` spawn count and discovery time drop; `disks` excludes `md*`; `da0`/`da0s1*` partitions unchanged; native release gate passes. Spot-check OpenBSD/NetBSD guests to confirm their facts are unchanged. diff --git a/openspec/changes/optimize-bsd-disks-probe/proposal.md b/openspec/changes/optimize-bsd-disks-probe/proposal.md index 7c43c207..fb916c62 100644 --- a/openspec/changes/optimize-bsd-disks-probe/proposal.md +++ b/openspec/changes/optimize-bsd-disks-probe/proposal.md @@ -1,16 +1,34 @@ ## Why -The BSD disks/partitions probe enumerates every device from `kern.disks` and probes each with `disklabel`; on DragonFly it runs `disklabel` on five slice targets (`s1`–`s4` plus the device) per device. On a host with many pseudo-devices this fans out to hundreds of subprocess spawns per discovery. The nlab/CI DragonFly guest lists **132 devices, 127 of them `md` memory disks**, so one discovery spawns **660 `disklabel` processes** (~1m42s of system time), and every full `Run()` discovery pays it — which is why the emulated DragonFly VM's `internal/app` integration suite runs for ~11 minutes. The `md`/`vn` pseudo-devices are also not physical host storage and should not be reported as disks/partitions at all. +The DragonFly `disks`/`partitions` probe has two independent problems, both confirmed on the nlab DragonFly guest while diagnosing the CI timeout: + +1. **Fan-out.** `dragonFlyDisklabelTargets` blindly probes `device + s1..s4` (plus the whole disk) for *every* device in `kern.disks`. The guest lists **132 devices** (`da0`, `vn0..3`, `md0..md126`), so one discovery spawns **~660 `disklabel` processes** (~1m42s system time), of which exactly one (`disklabel da0s1`) is productive. The whole-disk target is never productive on DragonFly — the label lives on the slice, so `disklabel da0` returns `Operation not supported by device`. +2. **Phantom disks.** The 127 empty `md` memory disks also **leak into the `disks` fact** as phantom 9.77 MB disks, because `diskinfo /dev/md0` returns a positive size. + +**Ground truth from the nlab guests reshaped the scope to DragonFly-only:** + +| OS | `hw.disknames` / `kern.disks` | Phantom leak? | +|----|-------------------------------|---------------| +| **DragonFly** | `md1..md126 da0 vn0..3 md0` (132 devices) | **Yes** — 127 `md` phantoms, ~660 disklabel spawns | +| OpenBSD | `sd0:,fd0:` | No — `rd0`/`cd0` are **not** listed; `fd0` errors on `disklabel` and is dropped | +| NetBSD | `sd0 dk0 dk1` | No — `md0`/`cd0` are **not** listed; `dk` wedges are already handled | + +On OpenBSD and NetBSD the kernel curates `hw.disknames` down to *attached* instances, so ram/memory/optical pseudo-devices never enter the probe in the first place. Adding `rd`/`md`/`cd` filtering there would be dead code — and filtering `rd` would be actively **harmful** in a `bsd.rd` recovery boot, where `rd0` *is* the root disk. So this change touches DragonFly only; OpenBSD/NetBSD keep their current behavior, including the existing NetBSD `dk`-wedge handling. + +Ruby Facter has **no BSD disks/partitions resolver** (both are Linux-only), so there is no parity constraint — reporting only real host storage is a strict improvement. ## What Changes -- The BSD disks/partitions probes **skip pseudo-devices** — memory disks (`md*`), vnode/file-backed disks (`vn*`), and optical devices (`cd*`) — when enumerating `kern.disks`. -- The DragonFly probe stops blindly spawning `disklabel` on non-existent slice targets for every device; it probes only real devices and their actual slices. -- Result: on hosts with many pseudo-devices, discovery drops from hundreds of subprocess spawns to a handful, and `disks`/`partitions` no longer list phantom memory/vnode disks. +**DragonFly only:** +- `dragonFlyDisklabelTargets` enumerates the slices that actually exist for a device (glob `/dev/s`) instead of the fixed `device + s1..s4`, and drops the never-productive whole-disk target. This eliminates the fan-out. +- Drop DragonFly memory-disk / optical pseudo-devices (`md`, `cd`) from both the `disks` fact and the partition probe, matched by **driver class** (name with trailing digits stripped), so the 127 `md` phantoms disappear and no `disklabel` is spawned for them. +- Unattached `vn` needs no special-casing: it is already excluded from `disks` by the existing size gate (`diskinfo` returns no size), and the glob yields no slice nodes for it (`/dev/vn0s*` is empty), so it contributes zero `disklabel` spawns. *Attached* `vn` still has slice nodes and a positive size, so it is kept. + +**OpenBSD / NetBSD:** no change. ## Impact -- **Behavior**: `disks`/`partitions` on the BSDs exclude `md*`/`vn*`/`cd*` pseudo-devices — a correctness improvement (these were never real host storage). -- **Performance**: full discovery is markedly faster on BSD hosts with many pseudo-devices; the emulated CI/lab DragonFly `internal/app` suite speeds up. -- **Compatibility**: real disks (`da*`, `ada*`, `ad*`, `wd*`, `sd*`, `vtbd*`, …) and their partitions are unchanged. -- **Out of scope**: the FreeBSD path (single `sysctl kern.geom.confxml` call, no per-device fan-out); the Linux/darwin/Windows/illumos disk probes; the CI `go test -timeout` bump (already landed on the disable-mechanism PR). +- **Behavior**: DragonFly `disks`/`partitions` no longer report empty `md` phantoms; the real `da0` disk, its `da0s1` slice partitions, and any *attached* `vn` are unchanged. OpenBSD/NetBSD facts are byte-for-byte unchanged. +- **Performance**: DragonFly discovery drops from ~660 `disklabel` spawns to one. Other platforms unaffected (no fan-out to begin with). +- **Not lost**: mounted pseudo-devices (tmpmfs `/tmp`, attached `vn` images) still surface in the **`mountpoints`** fact, which is intentionally not device-class filtered. +- **Out of scope**: OpenBSD/NetBSD/FreeBSD disk probes; the Linux/darwin/Windows/illumos disk probes; the CI `go test -timeout` bump (already landed). diff --git a/openspec/changes/optimize-bsd-disks-probe/specs/go-port-supported-platform-facts/spec.md b/openspec/changes/optimize-bsd-disks-probe/specs/go-port-supported-platform-facts/spec.md index dca98e03..37ffa6be 100644 --- a/openspec/changes/optimize-bsd-disks-probe/specs/go-port-supported-platform-facts/spec.md +++ b/openspec/changes/optimize-bsd-disks-probe/specs/go-port-supported-platform-facts/spec.md @@ -1,21 +1,27 @@ ## ADDED Requirements -### Requirement: BSD disk probes skip pseudo-devices and bound subprocess fan-out +### Requirement: DragonFly disk probes drop empty memory disks without dropping real storage -Facts SHALL exclude pseudo-devices — memory disks (`md*`), vnode/file-backed disks (`vn*`), and optical devices (`cd*`) — from the BSD `disks`/`partitions` probes, and SHALL NOT spawn a partition-probe subprocess for non-existent slice targets, so discovery stays fast on hosts with many pseudo-devices while real-disk facts are unchanged. +Facts SHALL exclude DragonFly memory-disk and optical pseudo-devices (`md`, `cd`, matched by driver class — the device name with trailing digits stripped) from the DragonFly `disks` and `partitions` facts, while keeping real disks, their real slices, and attached file-backed (`vn`) disks. -#### Scenario: pseudo-devices are neither probed nor reported +#### Scenario: empty memory disks are neither probed nor reported -- **WHEN** `kern.disks` lists `md*`, `vn*`, or `cd*` pseudo-devices alongside real disks -- **THEN** the disks/partitions probe MUST NOT spawn a `disklabel` (or equivalent) subprocess for the pseudo-devices -- **AND** `disks` and `partitions` MUST NOT report the pseudo-devices +- **WHEN** `kern.disks` lists empty `md` memory disks alongside a real disk +- **THEN** the probe MUST NOT spawn a `disklabel` subprocess for the `md` devices +- **AND** `disks` and `partitions` MUST NOT report them -#### Scenario: real-disk facts are unchanged +#### Scenario: real disks and attached file-backed disks are unchanged -- **WHEN** a real disk such as `da0`, `ada0`, or `wd0` is present +- **WHEN** a real disk (`da0`) or an attached file-backed disk (a configured `vn0`) is present - **THEN** its `disks` and `partitions` facts MUST be identical to before this change -#### Scenario: DragonFly does not fan out to non-existent slices +### Requirement: The DragonFly partition probe enumerates only existing slices + +Facts SHALL probe only the slices that actually exist for a DragonFly device — discovered by enumerating `/dev/s` slice nodes — rather than a fixed `s1`–`s4` set plus the whole disk, so discovery does not spawn wasted `disklabel` processes on hosts with many devices. + +#### Scenario: no fan-out to non-existent slices - **WHEN** the DragonFly probe enumerates a device's partitions -- **THEN** it MUST NOT spawn a `disklabel` subprocess for slice targets that do not exist on the device +- **THEN** it MUST issue `disklabel` only for slice targets that exist on the device +- **AND** it MUST NOT issue a fixed `device + s1..s4` set of `disklabel` calls per device +- **AND** it MUST NOT issue `disklabel` on the whole-disk device (the DragonFly label lives on the slice) diff --git a/openspec/changes/optimize-bsd-disks-probe/tasks.md b/openspec/changes/optimize-bsd-disks-probe/tasks.md index 066dcfcd..5cfd699a 100644 --- a/openspec/changes/optimize-bsd-disks-probe/tasks.md +++ b/openspec/changes/optimize-bsd-disks-probe/tasks.md @@ -1,16 +1,18 @@ ## 1. Tests First -- [ ] 1.1 Add a probe test (fake `commandRunner` recording calls) proving `md*`/`vn*`/`cd*` devices from `kern.disks` are skipped — no `disklabel` (or equivalent) subprocess is issued for them. -- [ ] 1.2 Add a test proving real devices (`da0`, `ada0`, `wd0`, …) are still probed and their partitions parsed unchanged. -- [ ] 1.3 Add a DragonFly test proving the probe does not spawn `disklabel` for non-existent slice targets of every device. +- [x] 1.1 DragonFly disks: with a fake `commandRunner`, assert `md*` devices from `kern.disks` are absent from the `disks` fact and no `diskinfo` phantom leaks, while `da0` is byte-identical to today. +- [x] 1.2 DragonFly partitions: with a recording `commandRunner` + fake `pathGlobber`, assert **zero** `disklabel` spawns for `md*`, and that a device is probed only on the slices the globber reports — `disklabel da0s1` runs; `disklabel da0`, `da0s2`, `da0s3`, `da0s4` do not. +- [x] 1.3 `dragonFlyDisklabelTargets`: given a globber returning `/dev/da0s1 /dev/da0s1a /dev/da0s1b /dev/da0s1d`, assert it returns exactly `[da0s1]` (slice only, not the partition-within-slice nodes, not the whole disk). +- [x] 1.4 Unattached `vn`: given an empty glob for `/dev/vn0s*`, assert the partition probe issues no `disklabel` for `vn0`. Attached `vn`: a positive test asserts a configured `vn0` (positive `diskinfo`, `/dev/vn0s1` slice node) stays in `disks` and its partitions are probed. ## 2. Implementation -- [ ] 2.1 Add a shared `isPseudoDisk`/`filterRealDisks` helper and apply it in `currentDragonFlyPartitions`, `currentOpenBSDPartitions`, `currentNetBSDPartitions`, and the disks-list enumeration (`disks.go`). -- [ ] 2.2 DragonFly: bound the slice-target fan-out to slices that actually exist (or derive slices from a single `disklabel `). +- [x] 2.1 Add `dragonFlyPseudoDisk(name string) bool` (driver class ∈ `{md, cd}`); apply in `currentDragonFlyDisks` and `currentDragonFlyPartitions`. +- [x] 2.2 Rewrite `dragonFlyDisklabelTargets` to take a `pathGlobber`, glob `/dev/s*`, and return only `s` slice names; drop the whole-disk target. +- [x] 2.3 Thread `s.glob` into `currentDragonFlyPartitions`; update the `currentPartitions` dispatch. ## 3. Verification -- [ ] 3.1 Run `go test ./...` and `go vet ./...`. -- [ ] 3.2 nlab validation on the DragonFly/OpenBSD/NetBSD guests: measure the drop in `disklabel` spawns and discovery time, confirm `partitions` excludes `md*`, confirm real-disk facts unchanged, and run the native release gates. -- [ ] 3.3 Run `openspec validate optimize-bsd-disks-probe --strict`. +- [x] 3.1 `go test ./...` and `go vet ./...`. +- [x] 3.2 nlab validation on the DragonFly guest: release gate passes; `disks` = `{da0}` only (127 `md` phantoms gone); `partitions` = real `da0s1a`/`b`/`d` unchanged; discovery `disks partitions` drops from ~1m42s system time to **0.65s**; `disklabel` spawns drop from ~660 to ~1. OpenBSD/NetBSD code paths are untouched (DragonFly-only change), so their facts are byte-identical by construction. +- [x] 3.3 `openspec validate optimize-bsd-disks-probe --strict`.