Add SSD parity warning for unformatted array devices#2535
Add SSD parity warning for unformatted array devices#2535
Conversation
WalkthroughAdds SSD parity warning detection and messaging plus finer per-member filesystem error details to the array_offline device handling by introducing two helper functions and injecting an ssdWarning into the offline warning output when applicable. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 722-723: Remove the redundant duplicate PHP closing tag '?>' (the
second occurrence with a leading space) so the file only ends with a single PHP
closing tag; locate the duplicate '?>' near the end of the file (the two
consecutive closing tags shown in the diff) and delete the extra one to avoid
emitting unexpected whitespace.
- Around line 111-116: The needs_parity_ssd_warning function currently flags any
Parity/Data SSD or NVMe regardless of format; update it to also check the disk's
fsStatus and only return true for devices whose fsStatus indicates
unformatted/unmountable (e.g. starts with "Unmountable"). Inside
needs_parity_ssd_warning (and using existing helpers like _var and the $disk
param), read fsStatus (default to empty) and if it does not start with
"Unmountable" return false before the rotational/transport checks so the warning
appears only for unformatted devices.
| function needs_parity_ssd_warning($disk) { | ||
| $type = _var($disk,'type'); | ||
| if (!in_array($type, ['Parity','Data'])) return false; | ||
| if (!_var($disk,'device')) return false; | ||
| return !_var($disk,'rotational',1) || _var($disk,'transport')=='nvme'; | ||
| } |
There was a problem hiding this comment.
Missing unformatted device check as described in PR objectives.
The PR summary states the warning should be shown for "unformatted devices" by checking fsStatus values that start with Unmountable. However, this function shows the warning for all SSDs/NVMe devices in Parity or Data slots regardless of their format status.
If the warning should only appear for unformatted devices, consider adding the fsStatus check:
,
Proposed fix
function needs_parity_ssd_warning($disk) {
$type = _var($disk,'type');
if (!in_array($type, ['Parity','Data'])) return false;
if (!_var($disk,'device')) return false;
+ // Only warn for unformatted devices
+ $fsStatus = _var($disk,'fsStatus','');
+ if ($fsStatus !== '' && !str_starts_with($fsStatus, 'Unmountable')) return false;
return !_var($disk,'rotational',1) || _var($disk,'transport')=='nvme';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function needs_parity_ssd_warning($disk) { | |
| $type = _var($disk,'type'); | |
| if (!in_array($type, ['Parity','Data'])) return false; | |
| if (!_var($disk,'device')) return false; | |
| return !_var($disk,'rotational',1) || _var($disk,'transport')=='nvme'; | |
| } | |
| function needs_parity_ssd_warning($disk) { | |
| $type = _var($disk,'type'); | |
| if (!in_array($type, ['Parity','Data'])) return false; | |
| if (!_var($disk,'device')) return false; | |
| // Only warn for unformatted devices | |
| $fsStatus = _var($disk,'fsStatus',''); | |
| if ($fsStatus !== '' && !str_starts_with($fsStatus, 'Unmountable')) return false; | |
| return !_var($disk,'rotational',1) || _var($disk,'transport')=='nvme'; | |
| } |
🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/nchan/device_list` around lines 111 - 116, The
needs_parity_ssd_warning function currently flags any Parity/Data SSD or NVMe
regardless of format; update it to also check the disk's fsStatus and only
return true for devices whose fsStatus indicates unformatted/unmountable (e.g.
starts with "Unmountable"). Inside needs_parity_ssd_warning (and using existing
helpers like _var and the $disk param), read fsStatus (default to empty) and if
it does not start with "Unmountable" return false before the
rotational/transport checks so the warning appears only for unformatted devices.
| ?> | ||
| ?> No newline at end of file |
There was a problem hiding this comment.
Remove duplicate closing PHP tag.
Line 723 contains a duplicate ?> tag with a leading space. This will cause unexpected whitespace to be output and the second closing tag is redundant.
Proposed fix
?>
- ?>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ?> | |
| ?> | |
| ?> |
🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/nchan/device_list` around lines 722 - 723, Remove the
redundant duplicate PHP closing tag '?>' (the second occurrence with a leading
space) so the file only ends with a single PHP closing tag; locate the duplicate
'?>' near the end of the file (the two consecutive closing tags shown in the
diff) and delete the extra one to avoid emitting unexpected whitespace.
1 participant
Motivation
Description
fsStatusfor values starting withUnmountableand detect SSD/NVMe devices by checkingrotationalandtransport == 'nvme'inemhttp/plugins/dynamix/nchan/device_listinsidearray_offline().The array does not support parity with SSDs due to TRIM. For maximum speed, assign SSDs to a pool instead.and append it to the existing overwrite/warning message except for devices of typeCache.Testing
emhttp/plugins/dynamix/nchan/device_listwas updated successfully; no automated tests were run.Codex Task
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.