From c3650270f992b769a11e2e7d405dacd04da9b714 Mon Sep 17 00:00:00 2001 From: Alex Rodriguez <131964409+ezekiel-alexrod@users.noreply.github.com> Date: Mon, 20 Jul 2026 09:17:57 +0200 Subject: [PATCH 1/3] fix(megaraid): rescan SCSI bus after creating a virtual drive storcli's "add vd" returns before the kernel has a block device for the new volume. The megaraid_sas driver only auto-announces the first few VDs created in rapid succession; later ones exist on the controller but have no /dev node (and therefore no /dev/disk/by-id symlink) until the SCSI bus is explicitly rescanned. createLV could then fail with "failed to compute permanent path" once several volumes were created back to back. Force a targeted rescan of the megaraid_sas SCSI hosts after "add vd" and poll findNewLogicalVolume until the new volume's device node is discovered, bounded by a timeout. Only megaraid_sas hosts are scanned to avoid the multi-second link-down timeouts of empty SATA ports. --- .../raidcontroller/megaraid/logicalvolume.go | 112 +++++++++++++++++- .../raidcontroller/megaraid/megaraid_test.go | 65 ++++++++++ 2 files changed, 174 insertions(+), 3 deletions(-) diff --git a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go index ee0dcdf..7a17ec2 100644 --- a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go +++ b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go @@ -2,10 +2,12 @@ package megaraid import ( "fmt" + "os" "path/filepath" "sort" "strconv" "strings" + "time" "github.com/pkg/errors" @@ -15,11 +17,38 @@ import ( "github.com/scality/raidmgmt/pkg/utils" ) -// patternLV is the pattern for the logical volume selector. const ( + // patternLV is the pattern for the logical volume selector. patternLV string = "/c%d/v%s" + + // scsiHostRoot holds one sysfs directory per SCSI host adapter. + scsiHostRoot = "/sys/class/scsi_host" + + // megaraidSASDriver is the proc_name of the MegaRAID kernel driver, used to + // select which SCSI hosts to rescan for newly created virtual drives. + megaraidSASDriver = "megaraid_sas" +) + +// newVolumeSettleTimeout and newVolumeSettleInterval bound the wait for a +// freshly created volume's device node to appear after "add vd". The kernel +// only auto-discovers the first few VDs created in rapid succession (the +// megaraid_sas driver stops emitting hotplug events), so createLV forces a bus +// rescan and then polls until the new volume resolves. Package-level vars so +// tests can shrink them. +// +//nolint:gochecknoglobals // Tunables for the post-create device-settle poll. +var ( + newVolumeSettleTimeout = 60 * time.Second + newVolumeSettleInterval = 1 * time.Second ) +// CustomRescanSCSIHosts forces the kernel to probe for newly created virtual +// drives. It is a package-level var so tests can stub the sysfs side effect, +// mirroring CustomFileExists / CustomEvalSymlinks. +// +//nolint:gochecknoglobals // Test seam for the sysfs bus rescan. +var CustomRescanSCSIHosts = rescanMegaraidHosts + // logicalvolumes returns all logical volumes for a given controller. func (a *Adapter) logicalvolumes(metadata *raidcontroller.Metadata) ( []*logicalvolume.LogicalVolume, @@ -315,8 +344,10 @@ func (a *Adapter) createLV(request *logicalvolume.Request) ( return nil, errors.Wrap(err, ErrCommandFailed.Error()) } - // Get the newly created logical volume - newLV, err := a.findNewLogicalVolume(request.PDrivesMetadata) + // Get the newly created logical volume. storcli returns before the kernel + // has a block device for the new volume, so force a bus rescan and poll + // until it resolves instead of failing on the first attempt. + newLV, err := a.findNewVolumeUntilSettled(request.PDrivesMetadata) if err != nil { return nil, errors.Wrap(err, "failed to find new logical volume") } @@ -324,6 +355,81 @@ func (a *Adapter) createLV(request *logicalvolume.Request) ( return newLV, nil } +// findNewVolumeUntilSettled forces a SCSI bus rescan and retries +// findNewLogicalVolume until the newly created volume's device node has been +// discovered and its permanent path resolves, or newVolumeSettleTimeout +// elapses. The rescan is required because the megaraid_sas driver stops +// auto-announcing VDs after a few rapid creations, leaving later volumes with +// no /dev node until the kernel is told to probe the bus. +func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( + *logicalvolume.LogicalVolume, + error, +) { + deadline := time.Now().Add(newVolumeSettleTimeout) + + var lastRescanErr error + + for { + // Force discovery of the new VD. A failure here is not fatal on its own + // (auto-discovery may still succeed), but it is surfaced if we time out. + if rescanErr := CustomRescanSCSIHosts(); rescanErr != nil { + lastRescanErr = rescanErr + } + + lv, err := a.findNewLogicalVolume(pds) + if err == nil { + return lv, nil + } + + if time.Now().After(deadline) { + if lastRescanErr != nil { + return nil, errors.Wrapf(err, + "device node not settled after %s (scsi rescan failing: %v)", + newVolumeSettleTimeout, lastRescanErr) + } + + return nil, errors.Wrapf(err, "device node not settled after %s", + newVolumeSettleTimeout) + } + + time.Sleep(newVolumeSettleInterval) + } +} + +// rescanMegaraidHosts asks the kernel to probe the SCSI bus for newly created +// virtual drives on the MegaRAID controller. Only megaraid_sas hosts are +// scanned, to avoid the multi-second link-down timeouts of empty SATA ports. +func rescanMegaraidHosts() error { + entries, err := os.ReadDir(scsiHostRoot) + if err != nil { + return errors.Wrap(err, "failed to list scsi hosts") + } + + scanned := 0 + + for _, entry := range entries { + hostDir := filepath.Join(scsiHostRoot, entry.Name()) + + procName, err := os.ReadFile(filepath.Join(hostDir, "proc_name")) + if err != nil || strings.TrimSpace(string(procName)) != megaraidSASDriver { + continue + } + + // "- - -" means "scan every channel, target and LUN" on this host. + if err := os.WriteFile(filepath.Join(hostDir, "scan"), []byte("- - -"), 0); err != nil { + return errors.Wrapf(err, "failed to rescan scsi host %s", entry.Name()) + } + + scanned++ + } + + if scanned == 0 { + return errors.New("no megaraid_sas scsi host found to rescan") + } + + return nil +} + // megaraidCreateCacheFlags returns the "add vd" cache flags. Each policy is // mapped to its megaraid CLI token through the adapter-owned mappers rather than // emitted from the domain enum's string value (see cache.go). The megaraid v1 diff --git a/pkg/implementation/raidcontroller/megaraid/megaraid_test.go b/pkg/implementation/raidcontroller/megaraid/megaraid_test.go index ec39ce4..11257cf 100644 --- a/pkg/implementation/raidcontroller/megaraid/megaraid_test.go +++ b/pkg/implementation/raidcontroller/megaraid/megaraid_test.go @@ -49,6 +49,10 @@ func (s *UnitTestSuite) SetupTest() { s.a = megaraid2.New(s.mockRunner) s.mockPathResolver = mocks2.NewPathResolver(s.T()) + + // Never touch the real /sys during unit tests; createLV triggers a SCSI + // bus rescan, which tests override individually when they assert on it. + megaraid2.CustomRescanSCSIHosts = func() error { return nil } } // mockOutput reads the output from a file and returns it. @@ -708,6 +712,67 @@ func (s *UnitTestSuite) TestCreateLV() { } } +// TestCreateLVForcesRescanAndSettles reproduces the post-create discovery race: +// storcli returns before the kernel has a block device for the new volume, so +// the first path resolution fails ("failed to compute permanent path") while a +// later one succeeds. createLV must force a SCSI bus rescan and poll until the +// device node is discovered instead of failing on the first try. +func (s *UnitTestSuite) TestCreateLVForcesRescanAndSettles() { + s.wasCreateLVCalledOnce = false + + s.setupMockCallsCreateLV() + + // Stub the sysfs bus rescan and count how often it runs (never touch the + // real /sys in unit tests). + rescanCalls := 0 + origRescan := megaraid2.CustomRescanSCSIHosts + megaraid2.CustomRescanSCSIHosts = func() error { + rescanCalls++ + + return nil + } + + defer func() { megaraid2.CustomRescanSCSIHosts = origRescan }() + + // First resolution: device not yet discovered. For the single-drive volume + // this falls back to ComputePaths on the real filesystem, which fails. Every + // subsequent resolution finds the symlink. + s.mockPathResolver.On("FileExists", mock.Anything).Return(false).Once() + s.mockPathResolver.On("FileExists", mock.Anything).Return(true) + s.mockPathResolver.On( + "EvalSymlinks", + "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377", + ).Return("/dev/sda", nil) + + s.setupCustomFileExists() + defer s.restoreCustomFileExists() + + s.setupCustomEvalSymlinks() + defer s.restoreCustomEvalSymlinks() + + request := &logicalvolume.Request{ + CtrlMetadata: &raidcontroller.Metadata{ID: 0}, + RAIDLevel: logicalvolume.RAIDLevel0, + PDrivesMetadata: []*physicaldrive.Metadata{ + {CtrlMetadata: &raidcontroller.Metadata{ID: 0}, ID: "251:12"}, + }, + CacheOptions: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteThrough, + IOPolicy: logicalvolume.IOPolicyDirect, + }, + } + + newLv, err := s.a.CreateLV(request) + + s.Require().NoError(err) + s.Require().NotNil(newLv) + s.Equal("228", newLv.ID) + s.Equal("/dev/sda", newLv.DevicePath) + s.Equal("/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377", newLv.PermanentPath) + s.GreaterOrEqual(rescanCalls, 1, "createLV must force a SCSI rescan for the new volume") +} + func (s *UnitTestSuite) TestDeleteLV() { s.mockRunner.On("Run", []string{"/c0/v228", "delete"}). Return(mockReturn("logicalvolumes/delete/success")) From 5eb13d014a9e5173afcfef777cd2606d4a88febe Mon Sep 17 00:00:00 2001 From: Alex Rodriguez <131964409+ezekiel-alexrod@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:09:07 +0200 Subject: [PATCH 2/3] refactor(megaraid): rescan only after a failed volume lookup Per review: try findNewLogicalVolume first and only force a SCSI bus rescan when it fails, instead of rescanning before every attempt. A volume the kernel discovers on its own now resolves on the first attempt with no rescan and no side effect on the SCSI bus; the rescan stays a targeted remedy for volumes whose device node has not appeared yet. --- .../raidcontroller/megaraid/logicalvolume.go | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go index 7a17ec2..51fb3e8 100644 --- a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go +++ b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go @@ -345,8 +345,8 @@ func (a *Adapter) createLV(request *logicalvolume.Request) ( } // Get the newly created logical volume. storcli returns before the kernel - // has a block device for the new volume, so force a bus rescan and poll - // until it resolves instead of failing on the first attempt. + // has a block device for the new volume, so poll until it resolves (forcing + // a bus rescan between attempts) instead of failing on the first attempt. newLV, err := a.findNewVolumeUntilSettled(request.PDrivesMetadata) if err != nil { return nil, errors.Wrap(err, "failed to find new logical volume") @@ -355,12 +355,13 @@ func (a *Adapter) createLV(request *logicalvolume.Request) ( return newLV, nil } -// findNewVolumeUntilSettled forces a SCSI bus rescan and retries -// findNewLogicalVolume until the newly created volume's device node has been -// discovered and its permanent path resolves, or newVolumeSettleTimeout -// elapses. The rescan is required because the megaraid_sas driver stops -// auto-announcing VDs after a few rapid creations, leaving later volumes with -// no /dev node until the kernel is told to probe the bus. +// findNewVolumeUntilSettled retries findNewLogicalVolume until the newly created +// volume's device node has been discovered and its permanent path resolves, or +// newVolumeSettleTimeout elapses. When a lookup fails it forces a SCSI bus +// rescan before retrying: the megaraid_sas driver stops auto-announcing VDs +// after a few rapid creations, leaving later volumes with no /dev node until the +// kernel is told to probe the bus. Volumes the kernel discovers on its own +// resolve on the first attempt with no rescan. func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( *logicalvolume.LogicalVolume, error, @@ -370,12 +371,6 @@ func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( var lastRescanErr error for { - // Force discovery of the new VD. A failure here is not fatal on its own - // (auto-discovery may still succeed), but it is surfaced if we time out. - if rescanErr := CustomRescanSCSIHosts(); rescanErr != nil { - lastRescanErr = rescanErr - } - lv, err := a.findNewLogicalVolume(pds) if err == nil { return lv, nil @@ -392,6 +387,13 @@ func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( newVolumeSettleTimeout) } + // The volume did not resolve: the kernel may not have discovered its + // device node yet. Force a bus rescan, then wait before retrying. A + // rescan failure is not fatal on its own, but it is surfaced on timeout. + if rescanErr := CustomRescanSCSIHosts(); rescanErr != nil { + lastRescanErr = rescanErr + } + time.Sleep(newVolumeSettleInterval) } } From 7892145eb0061755a36dffe7727f797976b4b3f5 Mon Sep 17 00:00:00 2001 From: Alex Rodriguez <131964409+ezekiel-alexrod@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:16:51 +0200 Subject: [PATCH 3/3] refactor(megaraid): export settle tunables so tests can shrink them The settle timeout and interval were unexported while the test lives in an external package (megaraid_test), so its comment claiming tests could shrink them was wrong: the retry test slept a real second, and a future timeout-path test would sleep for a full minute. Export NewVolumeSettleTimeout / NewVolumeSettleInterval (matching the CustomRescanSCSIHosts / CustomFileExists seams) and shrink them in the retry test so it runs without real wall-clock delay. --- .../raidcontroller/megaraid/logicalvolume.go | 22 +++++++++---------- .../raidcontroller/megaraid/megaraid_test.go | 12 ++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go index 51fb3e8..ba25a54 100644 --- a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go +++ b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go @@ -29,17 +29,17 @@ const ( megaraidSASDriver = "megaraid_sas" ) -// newVolumeSettleTimeout and newVolumeSettleInterval bound the wait for a +// NewVolumeSettleTimeout and NewVolumeSettleInterval bound the wait for a // freshly created volume's device node to appear after "add vd". The kernel // only auto-discovers the first few VDs created in rapid succession (the -// megaraid_sas driver stops emitting hotplug events), so createLV forces a bus -// rescan and then polls until the new volume resolves. Package-level vars so -// tests can shrink them. +// megaraid_sas driver stops emitting hotplug events), so createLV polls until +// the new volume resolves, forcing a bus rescan between failed attempts. +// Exported so tests can shrink them (like CustomRescanSCSIHosts). // //nolint:gochecknoglobals // Tunables for the post-create device-settle poll. var ( - newVolumeSettleTimeout = 60 * time.Second - newVolumeSettleInterval = 1 * time.Second + NewVolumeSettleTimeout = 60 * time.Second + NewVolumeSettleInterval = 1 * time.Second ) // CustomRescanSCSIHosts forces the kernel to probe for newly created virtual @@ -357,7 +357,7 @@ func (a *Adapter) createLV(request *logicalvolume.Request) ( // findNewVolumeUntilSettled retries findNewLogicalVolume until the newly created // volume's device node has been discovered and its permanent path resolves, or -// newVolumeSettleTimeout elapses. When a lookup fails it forces a SCSI bus +// NewVolumeSettleTimeout elapses. When a lookup fails it forces a SCSI bus // rescan before retrying: the megaraid_sas driver stops auto-announcing VDs // after a few rapid creations, leaving later volumes with no /dev node until the // kernel is told to probe the bus. Volumes the kernel discovers on its own @@ -366,7 +366,7 @@ func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( *logicalvolume.LogicalVolume, error, ) { - deadline := time.Now().Add(newVolumeSettleTimeout) + deadline := time.Now().Add(NewVolumeSettleTimeout) var lastRescanErr error @@ -380,11 +380,11 @@ func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( if lastRescanErr != nil { return nil, errors.Wrapf(err, "device node not settled after %s (scsi rescan failing: %v)", - newVolumeSettleTimeout, lastRescanErr) + NewVolumeSettleTimeout, lastRescanErr) } return nil, errors.Wrapf(err, "device node not settled after %s", - newVolumeSettleTimeout) + NewVolumeSettleTimeout) } // The volume did not resolve: the kernel may not have discovered its @@ -394,7 +394,7 @@ func (a *Adapter) findNewVolumeUntilSettled(pds []*physicaldrive.Metadata) ( lastRescanErr = rescanErr } - time.Sleep(newVolumeSettleInterval) + time.Sleep(NewVolumeSettleInterval) } } diff --git a/pkg/implementation/raidcontroller/megaraid/megaraid_test.go b/pkg/implementation/raidcontroller/megaraid/megaraid_test.go index 11257cf..f5774bc 100644 --- a/pkg/implementation/raidcontroller/megaraid/megaraid_test.go +++ b/pkg/implementation/raidcontroller/megaraid/megaraid_test.go @@ -6,6 +6,7 @@ import ( "os" "strings" "testing" + "time" "github.com/pkg/errors" @@ -720,6 +721,17 @@ func (s *UnitTestSuite) TestCreateLV() { func (s *UnitTestSuite) TestCreateLVForcesRescanAndSettles() { s.wasCreateLVCalledOnce = false + // Shrink the settle poll so the retry does not add real wall-clock time. + origTimeout := megaraid2.NewVolumeSettleTimeout + origInterval := megaraid2.NewVolumeSettleInterval + megaraid2.NewVolumeSettleTimeout = 5 * time.Second + megaraid2.NewVolumeSettleInterval = time.Millisecond + + defer func() { + megaraid2.NewVolumeSettleTimeout = origTimeout + megaraid2.NewVolumeSettleInterval = origInterval + }() + s.setupMockCallsCreateLV() // Stub the sysfs bus rescan and count how often it runs (never touch the