fix: Handle JBOD/passthrough disks behind RAID controllers gracefully

disk.go

@@ -2,13 +2,22 @@ package disko
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
 
 	"machinerun.io/disko/partid"
 )
 
+// ErrDiskTypeUndetermined is returned by RAIDController.GetDiskType when
+// the controller layer cannot determine the disk type. Typical cases:
+// the device is on the controller's sysfs tree but is not a configured
+// virtual/logical drive and cannot be matched as a JBOD/passthrough
+// disk, or controller queries were inconclusive (e.g. controller tool
+// unavailable). Callers should fall back to generic udev-based detection.
+var ErrDiskTypeUndetermined = errors.New("RAID controller could not determine disk type")
+
 // DiskType enumerates supported disk types.
 type DiskType int
 
@@ -24,19 +33,28 @@ const (
 
 	// TYPEFILE - A file on disk, not a block device.
 	TYPEFILE
+
+	// Unknown is an internal disko placeholder returned alongside a
+	// non-nil error by RAID-controller drivers when they cannot classify
+	// a device. It MUST NOT leak out of disko onto disko.Disk.Type: the
+	// linux system layer either consumes it via udev fallback or
+	// propagates the accompanying error. External callers should never
+	// observe this value.
+	Unknown
 )
 
 func (t DiskType) String() string {
-	return []string{"HDD", "SSD", "NVME", "FILE"}[t]
+	return []string{"HDD", "SSD", "NVME", "FILE", "UNKNOWN"}[t]
 }
 
 // StringToDiskType - convert a string to a disk type.
 func StringToDiskType(typeStr string) DiskType {
 	kmap := map[string]DiskType{
-		"HDD":  HDD,
-		"SSD":  SSD,
-		"NVME": NVME,
-		"FILE": TYPEFILE,
+		"HDD":     HDD,
+		"SSD":     SSD,
+		"NVME":    NVME,
+		"FILE":    TYPEFILE,
+		"UNKNOWN": Unknown,
 	}
 	if dtype, ok := kmap[typeStr]; ok {
 		return dtype

linux/raidcontroller.go

@@ -12,7 +12,6 @@ const (
 
 type RAIDController interface {
 	// Type() RAIDControllerType
-	GetDiskType(string) (disko.DiskType, error)
-	IsSysPathRAID(string) bool
+	GetDiskType(path string, udInfo disko.UdevInfo) (disko.DiskType, error)
 	DriverSysfsPath() string
 }

linux/sysfs/scsi.go

@@ -0,0 +1,50 @@
+package sysfs
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+)
+
+// ReadSCSITarget returns T from /sys/block/<kname>/device -> H:C:T:L.
+// For SCSI-attached JBOD/passthrough disks behind a RAID HBA, T matches
+// the controller-reported drive ID (megaraid Drive.DID, mpi3mr
+// PhysicalDrive.PID), which lets callers correlate a Linux block device
+// with an entry in the controller's PD list.
+//
+// ok=false (nil err) for non-SCSI devices (no "device" symlink, link does
+// not end in H:C:T:L, e.g. NVMe, virtio). sysRoot is injectable for tests;
+// production passes "/sys".
+func ReadSCSITarget(sysRoot, kname string) (target int, ok bool, err error) {
+	if kname == "" {
+		return 0, false, nil
+	}
+
+	link := filepath.Join(sysRoot, "block", kname, "device")
+
+	dest, err := os.Readlink(link)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return 0, false, nil
+		}
+		return 0, false, fmt.Errorf("readlink %q: %w", link, err)
+	}
+
+	last := filepath.Base(dest)
+	parts := strings.Split(last, ":")
+
+	const hctlFields = 4
+	if len(parts) != hctlFields {
+		return 0, false, nil
+	}
+
+	t, cerr := strconv.Atoi(parts[2])
+	if cerr != nil {
+		return 0, false, fmt.Errorf("parse SCSI target from %q: %w", last, cerr)
+	}
+
+	return t, true, nil
+}

linux/sysfs/scsi_test.go

@@ -0,0 +1,113 @@
+package sysfs
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// makeBlockDeviceSymlink wires up a fake sysfs entry at
+// <root>/block/<kname>/device pointing at ../../scsi_device/<hctl>.
+func makeBlockDeviceSymlink(t *testing.T, root, kname, hctl string) {
+	t.Helper()
+
+	blockDir := filepath.Join(root, "block", kname)
+	if err := os.MkdirAll(blockDir, 0o755); err != nil {
+		t.Fatalf("mkdir %q: %s", blockDir, err)
+	}
+
+	scsiDir := filepath.Join(root, "scsi_device", hctl)
+	if err := os.MkdirAll(scsiDir, 0o755); err != nil {
+		t.Fatalf("mkdir %q: %s", scsiDir, err)
+	}
+
+	link := filepath.Join(blockDir, "device")
+	target := filepath.Join("..", "..", "scsi_device", hctl)
+	if err := os.Symlink(target, link); err != nil {
+		t.Fatalf("symlink: %s", err)
+	}
+}
+
+func TestReadSCSITargetJBOD(t *testing.T) {
+	root := t.TempDir()
+	makeBlockDeviceSymlink(t, root, "sdb", "0:2:3:0")
+
+	target, ok, err := ReadSCSITarget(root, "sdb")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if !ok {
+		t.Fatalf("expected ok=true for a SCSI-backed block device")
+	}
+	if target != 3 {
+		t.Errorf("target: got %d, want 3", target)
+	}
+}
+
+// An NVMe/virtio-style block device has no H:C:T:L "device" symlink.
+// ReadSCSITarget must report ok=false (not an error) so the caller can
+// fall through to generic udev detection.
+func TestReadSCSITargetNoDevice(t *testing.T) {
+	root := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "block", "nvme0n1"), 0o755); err != nil {
+		t.Fatalf("mkdir: %s", err)
+	}
+
+	_, ok, err := ReadSCSITarget(root, "nvme0n1")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if ok {
+		t.Errorf("expected ok=false when device symlink is absent")
+	}
+}
+
+// A symlink whose last segment isn't H:C:T:L (e.g. points at a PCI node) must
+// not be mistaken for a SCSI device. ReadSCSITarget returns ok=false with no
+// error so the caller falls through to udev.
+func TestReadSCSITargetNonHCTL(t *testing.T) {
+	root := t.TempDir()
+	blockDir := filepath.Join(root, "block", "vda")
+	if err := os.MkdirAll(blockDir, 0o755); err != nil {
+		t.Fatalf("mkdir: %s", err)
+	}
+	other := filepath.Join(root, "devices", "virtio0")
+	if err := os.MkdirAll(other, 0o755); err != nil {
+		t.Fatalf("mkdir: %s", err)
+	}
+	if err := os.Symlink(filepath.Join("..", "..", "devices", "virtio0"),
+		filepath.Join(blockDir, "device")); err != nil {
+		t.Fatalf("symlink: %s", err)
+	}
+
+	_, ok, err := ReadSCSITarget(root, "vda")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if ok {
+		t.Errorf("expected ok=false when link target is not H:C:T:L")
+	}
+}
+
+func TestReadSCSITargetBadTarget(t *testing.T) {
+	root := t.TempDir()
+	makeBlockDeviceSymlink(t, root, "sdc", "0:2:notanum:0")
+
+	_, ok, err := ReadSCSITarget(root, "sdc")
+	if err == nil {
+		t.Fatalf("expected parse error, got nil")
+	}
+	if ok {
+		t.Errorf("expected ok=false on parse error")
+	}
+}
+
+func TestReadSCSITargetEmptyKname(t *testing.T) {
+	_, ok, err := ReadSCSITarget("/sys", "")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if ok {
+		t.Errorf("expected ok=false for empty kname")
+	}
+}

linux/sysfs/sysfs.go

@@ -0,0 +1,66 @@
+// Package sysfs contains Linux-specific helpers for inspecting the sysfs
+// hierarchy. The helpers are driver-agnostic so they can be shared by the
+// top-level linux package and by individual RAID driver packages without
+// creating an import cycle back into linux.
+package sysfs
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+)
+
+// IsSysPathRAID checks whether syspath (udevadm DEVPATH) belongs to a RAID
+// controller whose PCI driver is registered at driverSysPath.
+//
+//	syspath will look something like
+//	   /devices/pci0000:3a/0000:3a:02.0/0000:3c:00.0/host0/target0:2:2/0:2:2:0/block/sdc
+func IsSysPathRAID(syspath string, driverSysPath string) bool {
+	if !strings.HasPrefix(syspath, "/sys") {
+		syspath = "/sys" + syspath
+	}
+
+	if !strings.Contains(syspath, "/host") {
+		return false
+	}
+
+	fp, err := filepath.EvalSymlinks(syspath)
+	if err != nil {
+		fmt.Printf("seriously? %s\n", err)
+		return false
+	}
+
+	for _, path := range GetSysPaths(driverSysPath) {
+		if strings.HasPrefix(fp, path) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// GetSysPaths returns the resolved PCI device paths for a RAID driver.
+func GetSysPaths(driverSysPath string) []string {
+	paths := []string{}
+	// a raid driver has directory entries for each of the scsi hosts on that controller.
+	//   $cd /sys/bus/pci/drivers/<driver name>
+	//   $ for d in *; do [ -d "$d" ] || continue; echo "$d -> $( cd "$d" && pwd -P )"; done
+	//    0000:3c:00.0 -> /sys/devices/pci0000:3a/0000:3a:02.0/0000:3c:00.0
+	//    module -> /sys/module/<driver module name>
+
+	// We take a hack path and consider anything with a ":" in that dir as a host path.
+	matches, err := filepath.Glob(driverSysPath + "/*:*")
+
+	if err != nil {
+		fmt.Printf("errors: %s\n", err)
+		return paths
+	}
+
+	for _, p := range matches {
+		if fp, err := filepath.EvalSymlinks(p); err == nil {
+			paths = append(paths, fp)
+		}
+	}
+
+	return paths
+}

linux/sysfs/sysfs_test.go

@@ -0,0 +1,69 @@
+package sysfs
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// buildFakeDriverDir creates a tempdir layout mirroring /sys/bus/pci when a
+// RAID PCI driver is loaded:
+//
+//	<root>/drivers/<driver>/<bdf>  -> <root>/devices/pci0000/<bdf>
+//	<root>/drivers/<driver>/module -> <root>/module/<driver>
+//
+// It returns the driver directory and the resolved device paths that
+// GetSysPaths should return for it.
+func buildFakeDriverDir(t *testing.T, driver string, bdfs []string) (string, []string) {
+	t.Helper()
+	root := t.TempDir()
+
+	driverDir := filepath.Join(root, "drivers", driver)
+	require.NoError(t, os.MkdirAll(driverDir, 0o755))
+
+	// Real driver dirs also contain a "module" symlink; GetSysPaths must
+	// skip it because its name has no ':' (the "*:*" glob excludes it).
+	modulePath := filepath.Join(root, "module", driver)
+	require.NoError(t, os.MkdirAll(modulePath, 0o755))
+	require.NoError(t, os.Symlink(modulePath, filepath.Join(driverDir, "module")))
+
+	resolved := make([]string, 0, len(bdfs))
+	for _, bdf := range bdfs {
+		dev := filepath.Join(root, "devices", "pci0000", bdf)
+		require.NoError(t, os.MkdirAll(dev, 0o755))
+		require.NoError(t, os.Symlink(dev, filepath.Join(driverDir, bdf)))
+
+		resolvedDev, err := filepath.EvalSymlinks(dev)
+		require.NoError(t, err)
+		resolved = append(resolved, resolvedDev)
+	}
+
+	return driverDir, resolved
+}
+
+func TestGetSysPaths_EmptyDriverDir(t *testing.T) {
+	driverDir, _ := buildFakeDriverDir(t, "megaraid_sas", nil)
+	assert.Empty(t, GetSysPaths(driverDir))
+}
+
+func TestGetSysPaths_SingleBoundDevice(t *testing.T) {
+	driverDir, resolved := buildFakeDriverDir(t, "megaraid_sas",
+		[]string{"0000:3c:00.0"})
+
+	assert.ElementsMatch(t, resolved, GetSysPaths(driverDir))
+}
+
+func TestGetSysPaths_MultipleBoundDevices(t *testing.T) {
+	bdfs := []string{"0000:3c:00.0", "0000:82:00.0"}
+	driverDir, resolved := buildFakeDriverDir(t, "mpi3mr", bdfs)
+
+	assert.ElementsMatch(t, resolved, GetSysPaths(driverDir))
+}
+
+func TestIsSysPathRAID_NoHostSegment(t *testing.T) {
+	assert.False(t, IsSysPathRAID("/sys/devices/virtual/block/loop0", ""))
+	assert.False(t, IsSysPathRAID("/devices/virtual/block/loop0", ""))
+}