Skip to content

fix(megaraid): rescan SCSI bus after creating a virtual drive#86

Merged
ezekiel-alexrod merged 3 commits into
mainfrom
bugfix/megaraid-rescan-scsi-after-create
Jul 20, 2026
Merged

fix(megaraid): rescan SCSI bus after creating a virtual drive#86
ezekiel-alexrod merged 3 commits into
mainfrom
bugfix/megaraid-rescan-scsi-after-create

Conversation

@ezekiel-alexrod

@ezekiel-alexrod ezekiel-alexrod commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Problem

Creating several MegaRAID virtual drives back to back could fail with:

failed to find new logical volume: failed to get logical volumes: failed to get logical volume <id>: failed to get paths: failed to compute paths from physical drive: failed to compute permanent path

createLV resolves the freshly created volume through its /dev/disk/by-id/wwn-0x<naa> symlink. When many volumes are created in a row, that symlink was sometimes missing at resolution time and appeared a few seconds later, so the call failed non-deterministically.

Root cause

Not a udev latency issue. Instrumenting the sequence showed:

  • storcli add vd returns before the OS has a block device for the volume.
  • The megaraid_sas driver auto-announces new VDs via async events, but only for the first few created in rapid succession. After that, VDs exist on the controller with no /dev/sdX at all.
  • Once a device is finally discovered, its by-id symlink appears in the same instant, so udevadm settle is a no-op here (it returned in single-digit ms every time).

Measured per-volume discovery latency: ~1s for the first volumes (auto discovery), then no discovery until an explicit SCSI bus rescan, after which the device and its symlink appear together.

Fix

After add vd, force a targeted SCSI bus rescan and poll findNewLogicalVolume until the new volume resolves (bounded by a 60s timeout, 1s interval):

  • Rescan writes - - - only to hosts whose proc_name is megaraid_sas. Scanning every host walks empty SATA ports and adds multi-second link down timeouts, so it is deliberately scoped to the MegaRAID hosts.
  • The rescan is best effort: its error is only surfaced if the poll times out.
  • CustomRescanSCSIHosts is a package-level seam (like CustomFileExists) so the sysfs side effect can be stubbed in tests.

Testing

  • New unit test TestCreateLVForcesRescanAndSettles: reproduces the failure on the first resolution attempt (matches the exact production error chain via the single-drive ComputePaths fallback), verifies createLV retries after a rescan and returns the volume, and asserts a rescan was triggered.
  • go build ./..., full go test ./..., go vet ./... and golangci-lint all clean.
  • Verified end to end on hardware: a multi-volume creation run that previously failed now completes.

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.
@ezekiel-alexrod
ezekiel-alexrod requested a review from a team as a code owner July 20, 2026 07:33
Comment thread pkg/implementation/raidcontroller/megaraid/megaraid_test.go
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  • megaraid_test.go:720 — TestCreateLVForcesRescanAndSettles sleeps 1s unnecessarily; the settle timeout/interval vars were made package-level for test shrinking but the test doesn't use that seam.

    Everything else looks solid: error handling follows the Wrap/Wrapf convention, the rescan is properly scoped to megaraid_sas hosts, the CustomRescanSCSIHosts seam mirrors the existing CustomFileExists pattern, and the test correctly exercises the retry path through the FileExists once-false mock.

    Review by Claude Code

Comment thread pkg/implementation/raidcontroller/megaraid/logicalvolume.go Outdated
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.
Comment thread pkg/implementation/raidcontroller/megaraid/logicalvolume.go Outdated
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  • logicalvolume.go:41-42 — newVolumeSettleTimeout and newVolumeSettleInterval are unexported but the tests use package megaraid_test (external), so they cannot be shrunk as the comment claims. Export them to match the CustomRescanSCSIHosts / CustomFileExists pattern, and shrink them in TestCreateLVForcesRescanAndSettles to avoid a 1 s sleep per run.

    Overall: solid fix for a real hardware-level race. The retry loop, scoped rescan, and error surfacing are well designed. The only issue is the test timing oversight above.

    Review by Claude Code

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.
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

LGTM — clean fix for a real production race. The retry loop, targeted SCSI rescan, and test seam all follow the existing patterns (CustomFileExists / CustomEvalSymlinks). Error handling properly surfaces rescan failures on timeout via lastRescanErr.

Review by Claude Code

@ezekiel-alexrod
ezekiel-alexrod merged commit 489f1f1 into main Jul 20, 2026
8 checks passed
@ezekiel-alexrod
ezekiel-alexrod deleted the bugfix/megaraid-rescan-scsi-after-create branch July 20, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants