Fix smartpqi arcconf parsing for mixed SSD/HDD arrays with 4K block sizes#137
Fix smartpqi arcconf parsing for mixed SSD/HDD arrays with 4K block sizes#137andaaron wants to merge 1 commit intoproject-machine:mainfrom
Conversation
andaaron
commented
Feb 27, 2026
andaaron
commented
- Handle "4K Bytes" and other SI-suffixed block size tokens that caused strconv.Atoi failures on newer hardware.
- Split multiple logical devices within a single arcconf output chunk.
- Map new InterfaceType variants ("SATA SSD", "SAS 4K").
- Parse Channel and Protocol from physical device output.
- Fix GetDiskType returning HDD for the first non-matching drive.
…izes
1. Handle "4K Bytes" and other SI-suffixed block size tokens that caused
strconv.Atoi failures on newer hardware.
2. Split multiple logical devices within a single arcconf output chunk.
3. Map new InterfaceType variants ("SATA SSD", "SAS 4K").
4. Parse Channel and Protocol from physical device output.
5. Fix GetDiskType returning HDD for the first non-matching drive.
Signed-off-by: Andrei Aaron <andaaron@cisco.com>
raharper
left a comment
raharper
left a comment
There was a problem hiding this comment.
Thanks for adding some additional support and with tests!
I left some minor items; I think mostly around the parseBlockSize; I'd like to simplify and make it clear we're looking at the Disk "geometry" or "sector size" or "native block"; rather than anything related to capacity of a disk.
Please take a look.
| s = s[:len(s)-1] | ||
| case "g": | ||
| multiplier = 1024 * 1024 * 1024 | ||
| s = s[:len(s)-1] |
There was a problem hiding this comment.
Lets add a default section to return an error for unsupported multiplier
| logDevs := []LogicalDevice{} | ||
| // parseBlockSize parses a block size token that may use SI suffixes | ||
| // (e.g. "512", "4K", "1M"). Returns the value in bytes. | ||
| func parseBlockSize(raw string) (int, error) { |
There was a problem hiding this comment.
I'd appreciate a comment here that this is related to Disk "native" BlockSize (or SectorSize); which are typically 512, 4K, and I guess 1M;
we don't expect to see 1G or even larger size;
We should probably drop support for G in the multiplier.
This function should not be used for converting "human size" fields into bytes, like the "SizeMB" field; since the int return type is not large enough (we'd need an int64 or the size of disks we find in ND etc).
| case toks[0] == "Block Size of member drives": // 512 Bytes] | ||
| bs, err := strconv.Atoi(strings.Fields(toks[1])[0]) | ||
| case toks[0] == "Block Size of member drives": // 512 Bytes OR 4K Bytes] | ||
| bs, err := parseBlockSize(strings.Fields(toks[1])[0]) |
There was a problem hiding this comment.
We might instead simplify this using a switch on the input string; it looks like we have exactly two values:
func parseBlockSize(raw string) (int, error) {
switch raw {
case "512":
return 512, nil
case "4K":
return 4096, nil
default:
return 0, fmt.Errorf("Unknown Block Size value %q", raw)
}
2 participants
