fix(disks): eliminate DragonFly disklabel fan-out and md phantoms

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/<dev>s<N>`), 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

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/<device>s<N>). 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
+// (<device>s<digits>), excluding partition-within-slice nodes like <device>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 {

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

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 `<device>`, `<device>s1`…`<device>s4` for every device, probe the device and only the slices that actually exist. Investigate whether one `disklabel <device>` 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/<dev>s*`, keep names of the form `<dev>s<digits>` (a slice, not a `<dev>s1a` partition-within-slice), and probe exactly those. Drop the whole-disk target: `disklabel <wholedisk>` 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/<device>s*` and return the base names matching `<device>s<digits>` (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.