Skip to content

Fix smartpqi arcconf parsing for mixed SSD/HDD arrays with 4K block sizes#137

Merged
raharper merged 1 commit intoproject-machine:mainfrom
andaaron:parse
Mar 1, 2026
Merged

Fix smartpqi arcconf parsing for mixed SSD/HDD arrays with 4K block sizes#137
raharper merged 1 commit intoproject-machine:mainfrom
andaaron:parse

Conversation

@andaaron
Copy link
Contributor

  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.

@andaaron andaaron marked this pull request as ready for review February 27, 2026 21:17
Copy link
Collaborator

@raharper raharper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@raharper raharper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the parseBlockSize. I'm +1 once squashed and CI passes.

…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>
@andaaron
Copy link
Contributor Author

andaaron commented Mar 1, 2026

@raharper, I have fixed the issue, please run CI again.

@codecov
Copy link

codecov bot commented Mar 1, 2026

Codecov Report

❌ Patch coverage is 90.27778% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.97%. Comparing base (d8386ce) to head (ceee8f8).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
smartpqi/arcconf.go 90.27% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   49.40%   49.97%   +0.56%     
==========================================
  Files          27       27              
  Lines        3633     3666      +33     
==========================================
+ Hits         1795     1832      +37     
+ Misses       1624     1621       -3     
+ Partials      214      213       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@raharper raharper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@raharper raharper merged commit 83a9a74 into project-machine:main Mar 1, 2026
4 checks passed
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