feat(storcli2): cache options setter and JBOD setters#71
Open
g-carre wants to merge 2 commits into
Open
Conversation
Implement the storcli2/perccli2 write path for ARTESCA-17649: - SetLVCacheOptions on the logicalvolumemanager StorCLI2: diffs current vs desired and emits only the changed rdcache/wrcache flags as separate "set" commands (storcli2 rejects the combined syntax and dropped the IO policy). - EnableJBOD / DisableJBOD on the physicaldrivegetter StorCLI2 via "set jbod" / "set uconf" (storcli's "delete jbod" no longer parses). Both surface in-JSON failures through storcli2.Decode regardless of exit code. Widen the shared envelope's DetailedStatus.ErrCd to "any": storcli2 reports it as an int on failure and the string "-" on success, mirroring PID/VD. Issue: ARTESCA-17649 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| return errors.Wrapf(err, "failed to get logical volume %s", metadata.ID) | ||
| } | ||
|
|
||
| options, err := storcli2CacheOptions(current.CacheOptions, desired) |
There was a problem hiding this comment.
current.CacheOptions is a pointer field on LogicalVolume. If the getter ever returns a volume without cache metadata (e.g. a volume type that doesn't support caching), this will nil-pointer panic. A guard before the call would make the setter robust against upstream getter changes.
Suggested change
| options, err := storcli2CacheOptions(current.CacheOptions, desired) | |
| if current.CacheOptions == nil { | |
| return errors.Errorf("logical volume %s has no cache options", metadata.ID) | |
| } | |
| options, err := storcli2CacheOptions(current.CacheOptions, desired) |
— Claude Code
| // LogicalVolumesGetter so setters only emit the flags that actually change. | ||
| type StorCLI2 struct { | ||
| ports.LogicalVolumesGetter | ||
|
|
There was a problem hiding this comment.
The exported field StorCLI2 shadows the struct type name, which makes usage sites (e.g. s.StorCLI2.Run(...)) ambiguous to read. The existing physicaldrivegetter.StorCLI2 uses an unexported runner field for the same role — matching that convention would be more consistent and keep the command runner out of the public API surface.
Suggested change
| runner commandrunner.CommandRunner |
— Claude Code
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Write path of the storcli2/perccli2 adapter (ARTESCA-17649): the logical-volume cache setter and the per-drive JBOD setters.
SetLVCacheOptions— reads the current state through the logical-volume getter and emits only the policies that changed, as two independentset rdcache=/set wrcache=commands (storcli2 rejects the combined syntax and dropped the IO policy).EnableJBOD/DisableJBOD—/cx/ex/sx set jbodandset uconf, surfacing the in-JSON failure payload.storcli2envelope adjustment to support the setter error paths.Testing
go build,go vet,gofmt, and the package test suite pass. Tests cover the only-changed-flag behavior (rdcache-only / wrcache-only / both / none), getter-error and command-error paths, and the JBOD enable/disable paths.Issue: ARTESCA-17649
🤖 Generated with Claude Code