Skip to content

Commit

Permalink
fix: Handle Drives with existing config from another RAID adapter
Browse files Browse the repository at this point in the history
When querying state from the controller with drives that have existing
RAID config, the PhysicalDrive.DG field has the letter 'F' specified.
Handle this case by assigning another negative number (-2).  Also
Allocate a new DGUnknown value (-99) and use this when we encounter
another string value that we don't know about rather than crashing.

Add a test case for this scenario.

Additional changes
- Dropped commented out tests
- Created const values for the DG properties

Signed-off-by: Ryan Harper <[email protected]>
  • Loading branch information
raharper committed Feb 6, 2025
1 parent f0dbf37 commit 3d4a4b9
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 109 deletions.
27 changes: 19 additions & 8 deletions mpi3mr/storcli2.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,14 +306,19 @@ func (pd *PhysicalDrive) UnmarshalJSON(data []byte) error {
case int:
pd.DG = v.(int)
case string:
if v == "-" {
pd.DG = -1
} else {
switch v {
case "-": // UConfigured Good
pd.DG = DriveDGUConf
case "F": // Foreign Config
pd.DG = DriveDGForeign
default:
i, err := strconv.Atoi(v.(string))
if err != nil {
return err
fmt.Printf("warning: unknown 'DG' value %q, setting DG=%d", v, DriveDGUnknown)
pd.DG = DriveDGUnknown
} else {
pd.DG = i
}
pd.DG = i
}
}
case "Size":
Expand Down Expand Up @@ -522,10 +527,16 @@ func StorCli2() Mpi3mr {
return &storCli2{}
}

const noStorCli2RC = 127
const (
noStorCli2RC = 127

// when successful storcli2 show returns rc=6 ¯\_(ツ)_/¯
const StorCli2ShowRC = 6
// when successful storcli2 show returns rc=6 ¯\_(ツ)_/¯
StorCli2ShowRC = 6

DriveDGUConf = -1
DriveDGForeign = -2
DriveDGUnknown = -99
)

func (sc *storCli2) Query(cID int) (Controller, error) {
// run /c0 show all nolog J
Expand Down
132 changes: 31 additions & 101 deletions mpi3mr/storcli2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,107 +300,6 @@ var testc0VAllShowAllJOut = `
]
}
`

/*
var testParseVirtPropObj = VirtualDrives{
VDInfo: VirtualDrive{
DGVD: "0/1",
Type: "RAID0",
State: "Optl",
Access: "RW",
CurrentCache: "NR,WB",
DefaultCache: "NR,WB",
Size: "893.137 GiB",
Name: "",
},
PhysicalDrives: []PhysicalDrive{
{
EIDSlot: "322:2",
PID: 312,
State: "Conf",
Status: "Online",
DG: 0,
Size: "893.137 GiB",
Interface: "SATA",
Medium: "SSD",
SEDType: "-",
SectorSize: "512B",
Model: "SAMSUNG MZ7L3960HCJR-00AK1",
SP: "U",
LUNSCount: 1,
AltEID: "-",
},
},
VDProperties: VirtualDriveProperties{
StripSize: "64 KiB",
BlockSize: 4096,
NumberOfBlocks: 234130688,
SpanDepth: 1,
NumberOfDrives: 1,
DriveWriteCachePolicy: "Default",
DefaultPowerSavePolicy: "Default",
CurrentPowerSavePolicy: "Default",
AccessPolicyStatus: "VD has desired access",
AutoBGI: "Off",
Secured: "No",
InitState: "No Init",
Consistent: "Yes",
Morphing: "No",
CachePreserved: "No",
BadBlockExists: "No",
VDReadyForOSRequests: "Yes",
ReachedLDBBMFailureThreshold: "No",
SupportedEraseTypes: "Simple, Normal, Thorough",
ExposedToOS: "Yes",
CreationTimeLocalTimeString: "2025/01/29 22:07:51",
DefaultCachebypassMode: "Cachebypass Not Performed For Any IOs",
CurrentCachebypassMode: "Cachebypass Not Performed For Any IOs",
SCSINAAId: "62cf89bd43a7af80679aa6b751349581",
OSDriveName: "/dev/sdg",
CurrentUnmapStatus: "No",
CurrentWriteSameUnmapStatus: "No",
LUNSCountUsedPerPD: 1,
DataFormatForIO: "None",
SerialNumber: "0081953451b7a69a6780afa743bd89cf",
},
}
var testParsePhysicalDrivesObj = PhysicalDrive{
EIDSlot: "322:2",
PID: 312,
State: "Conf",
Status: "Online",
DG: 0,
Size: "893.137 GiB",
Interface: "SATA",
Medium: "SSD",
SEDType: "-",
SectorSize: "512B",
Model: "SAMSUNG MZ7L3960HCJR-00AK1",
SP: "U",
LUNSCount: 1,
AltEID: "-",
}
func TestStorCli2ParseVirtProperties(t *testing.T) {
ctrlID := 0
vdMap, err := parseVirtProperties(ctrlID, []byte(testc0VAllShowAllJOut))
if err != nil {
t.Fatalf("Error parsing virt properties: %s", err)
}
if len(vdMap) != 1 {
t.Fatalf("expected 1 virtual drive, got %d", len(vdMap))
}
want := testParseVirtPropObj.VDProperties
for _, got := range vdMap {
if diff := cmp.Diff(got, want); diff != "" {
t.Errorf("got:\n%+v\nwant:\n+%v", got, want)
}
}
}
*/

var expectedControllerBytes = `
{
"Controller": {
Expand Down Expand Up @@ -516,3 +415,34 @@ func TestStorCli2NewController(t *testing.T) {
t.Errorf("got:\n%+v\nwant:\n+%v", got, want)
}
}

var pdForeignData = `
{
"EID:Slt": "322:5",
"PID": 315,
"State": "UConf",
"Status": "Good",
"DG": "F",
"Size": "1.090 TiB",
"Intf": "SAS",
"Med": "HDD",
"SED_Type": "-",
"SeSz": "512B",
"Model": "AL15SEB120N ",
"Sp": "U",
"LU/NS Count": 1,
"Alt-EID": "-"
}
`

func TestStorCli2ParsePhysicalDriveForeign(t *testing.T) {
pd := PhysicalDrive{}
if err := json.Unmarshal([]byte(pdForeignData), &pd); err != nil {
t.Fatalf("failed to unmarshal PhysicalDrive with Foreign 'DG': %v", err)
}

if pd.DG != DriveDGForeign {
t.Fatalf("expected DG %d, got DG %d", DriveDGForeign, pd.DG)
}

}

0 comments on commit 3d4a4b9

Please sign in to comment.