feat(flash): change flash to boot device#2564
Merged
limetech merged 7 commits intounraid:masterfrom Mar 7, 2026
Merged
Conversation
🔧 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
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughTerminology throughout the codebase was changed from "Flash" / "USB flash" to "Boot device" / "Boot drive", including UI labels, help text, scripts, comments, link targets, and small UI behavior adjustments; one function signature added an optional parameter. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
emhttp/plugins/dynamix/scripts/rsyslog_config (1)
52-56:⚠️ Potential issue | 🟠 MajorAdd
*.debug ?flashindependently of template creation.On Line 53, the new debug mirror rule is gated by the template existence check, so systems that already have
$template flashnever get the new*.debug ?flashrule.🔧 Proposed fix
if [[ -n $syslog_flash ]]; then if ! grep -q '^\$template flash,' $ETC; then sed -ri '/^#\$UDPServerRun [0-9]+.*$/a$template flash,"/boot/logs/syslog"' $ETC - sed -ri '/^\*\.debug .*syslog$/a*.debug ?flash' $ETC fi + grep -q '^\*\.debug \?flash$' $ETC || \ + sed -ri '/^\*\.debug .*syslog$/a*.debug ?flash' $ETC else sed -ri '/^\$template flash,"\/boot\/logs\/syslog"$/d;/^\*\.debug \?flash/d' $ETC fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/scripts/rsyslog_config` around lines 52 - 56, The new "*.debug ?flash" insertion is incorrectly guarded by the template-existence check so systems that already have "$template flash" never receive the debug rule; update the rsyslog_config logic so that when [[ -n $syslog_flash ]] the script always ensures the '*.debug ?flash' line is present: keep the existing grep '^\$template flash,' test and its sed that adds the template only when missing, but move or duplicate the sed -ri '/^\*\.debug .*syslog$/a*.debug ?flash' invocation outside that conditional (or run a separate presence check) so the '*.debug ?flash' rule is added regardless of whether "$template flash" already exists, referencing the variables/strings syslog_flash, $template flash, the grep check and the sed command in the current diff.emhttp/plugins/dynamix/nchan/device_list (1)
495-513:⚠️ Potential issue | 🟠 MajorBoot/flash rows now incorrectly get spin-toggle controls.
$sourceis now'Boot'for flash/boot devices, but the spin-control guard still excludes only'Flash'. This enablestoggle_state('Boot', ...)for boot rows, which is a behavior regression.🔧 Proposed fix
- if (_var($var,'fsState')=='Started' && $source!='Flash' && !str_contains($disk_status,'_NP')) { + if (_var($var,'fsState')=='Started' && !$flash && !$boot && !str_contains($disk_status,'_NP')) { $ctrl = " style='cursor:pointer' onclick=\"toggle_state('$source','$name','$action')\""; $help .= "<br>"._("Click to spin $action device"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/nchan/device_list` around lines 495 - 513, The spin-toggle control guard incorrectly only excludes 'Flash' so rows with $source set to 'Boot' still get toggle_state controls; update the conditional that sets $ctrl (the if using _var($var,'fsState')=='Started' && $source!='Flash' && !str_contains($disk_status,'_NP')) to also exclude 'Boot' (e.g., require $source!='Flash' && $source!='Boot' or check $source==='Device') so boot/flash rows do not render the onclick toggle_state('$source',...) control.
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/BootParameters.page (1)
52-52: Consider preserving legacy anchor compatibility.Renaming the section id can break existing deep links/bookmarks that still use
#boot-parameters.♻️ Optional compatibility shim
- <section class="section" id="vm-parameters"> + <span id="boot-parameters" style="display:none"></span> + <section class="section" id="vm-parameters">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/BootParameters.page` at line 52, The section id was changed to "vm-parameters", which breaks legacy deep-links using "#boot-parameters"; add a compatibility shim by preserving or reintroducing the legacy anchor (e.g., add an element with id="boot-parameters" or give the same section both ids/anchors) so both "#vm-parameters" and "#boot-parameters" resolve to the same section; update the section with a non-visual anchor (e.g., an empty <a> or aria-hidden element) named "boot-parameters" to avoid layout changes while restoring legacy bookmark compatibility.emhttp/plugins/dynamix/nchan/device_list (1)
1101-1102: Remove redundant boot label assignment.
$bootLabelis assigned fromucfirst($bootPoolName)and then immediately overwritten with"Internal Boot". Keep only the final assignment for clarity.Also applies to: 1223-1224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/nchan/device_list` around lines 1101 - 1102, The variable $bootLabel is redundantly set from ucfirst($bootPoolName) and then immediately overwritten with _("Internal Boot"); remove the first assignment (the ucfirst($bootPoolName) line) and keep only the final $bootLabel = _("Internal Boot"); do the same cleanup for the duplicate occurrence around the $bootLabel/$bootPoolName usage at the later block (the other pair at lines referenced in the review).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/languages/en_US/helptext.txt`:
- Around line 106-109: Unify terminology in the help paragraph by replacing the
phrase "USB Flash device" with "boot device" so the paragraph consistently uses
"boot device" (note the existing link text "[Boot
Device](/Main/Boot?name=flash)" and the phrases "boot device" in the first two
lines); update the sentence "Since the USB Flash device is formatted using FAT
file system, it may only be exported using SMB protocol." to use "boot device"
and ensure capitalization matches surrounding text (i.e., "boot device" or "Boot
Device" depending on style).
In `@emhttp/plugins/dynamix.plugin.manager/scripts/plugin`:
- Around line 108-112: Replace the incorrect possessive phrase "users boot
device" with "user's boot device" in the plugin help text where that string
appears (the two occurrences in the plugin script), ensuring both instances in
the help/documentation text are updated exactly to "user's boot device".
In `@emhttp/plugins/dynamix/BootInfo.page`:
- Line 37: The restore logic only resets the button text but leaves the button
disabled; update the same element selection used for the text change (the input
matched by the selector 'input[value="_(Creating boot device backup)_..."]') to
also re-enable the control after failure by clearing its disabled state (e.g.
remove the disabled attribute or set its disabled property to false) — ensure
you obtain the element once (assign to a variable like $btn) and call both the
.val(...) update and the .prop('disabled', false) / .removeAttr('disabled') on
that element so retry is possible without a page reload.
In `@emhttp/plugins/dynamix/scripts/monitor`:
- Around line 294-299: The boot-drive read/write check uses an awk pattern that
only matches if "rw" is the first mount option, causing false alerts; update the
awk command string in the exec call that sets $warn (the exec("awk
'$2==\"/boot\" && $4 ~ /^rw/ {print \"rw\"}' /proc/mounts") invocation) to use a
token-safe regex like /(^|,)rw(,|$)/ so "rw" is matched regardless of position
in the option list, leaving the rest of the logic using $warn, $last and $saved
unchanged.
---
Outside diff comments:
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 495-513: The spin-toggle control guard incorrectly only excludes
'Flash' so rows with $source set to 'Boot' still get toggle_state controls;
update the conditional that sets $ctrl (the if using
_var($var,'fsState')=='Started' && $source!='Flash' &&
!str_contains($disk_status,'_NP')) to also exclude 'Boot' (e.g., require
$source!='Flash' && $source!='Boot' or check $source==='Device') so boot/flash
rows do not render the onclick toggle_state('$source',...) control.
In `@emhttp/plugins/dynamix/scripts/rsyslog_config`:
- Around line 52-56: The new "*.debug ?flash" insertion is incorrectly guarded
by the template-existence check so systems that already have "$template flash"
never receive the debug rule; update the rsyslog_config logic so that when [[ -n
$syslog_flash ]] the script always ensures the '*.debug ?flash' line is present:
keep the existing grep '^\$template flash,' test and its sed that adds the
template only when missing, but move or duplicate the sed -ri '/^\*\.debug
.*syslog$/a*.debug ?flash' invocation outside that conditional (or run a
separate presence check) so the '*.debug ?flash' rule is added regardless of
whether "$template flash" already exists, referencing the variables/strings
syslog_flash, $template flash, the grep check and the sed command in the current
diff.
---
Nitpick comments:
In `@emhttp/plugins/dynamix/BootParameters.page`:
- Line 52: The section id was changed to "vm-parameters", which breaks legacy
deep-links using "#boot-parameters"; add a compatibility shim by preserving or
reintroducing the legacy anchor (e.g., add an element with id="boot-parameters"
or give the same section both ids/anchors) so both "#vm-parameters" and
"#boot-parameters" resolve to the same section; update the section with a
non-visual anchor (e.g., an empty <a> or aria-hidden element) named
"boot-parameters" to avoid layout changes while restoring legacy bookmark
compatibility.
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 1101-1102: The variable $bootLabel is redundantly set from
ucfirst($bootPoolName) and then immediately overwritten with _("Internal Boot");
remove the first assignment (the ucfirst($bootPoolName) line) and keep only the
final $bootLabel = _("Internal Boot"); do the same cleanup for the duplicate
occurrence around the $bootLabel/$bootPoolName usage at the later block (the
other pair at lines referenced in the review).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1a7fc2d1-ae1b-4843-8056-61dc31511d40
📒 Files selected for processing (24)
emhttp/languages/en_US/helptext.txtemhttp/plugins/dynamix.plugin.manager/scripts/pluginemhttp/plugins/dynamix.vm.manager/VMSettings.pageemhttp/plugins/dynamix.vm.manager/scripts/libvirt_initemhttp/plugins/dynamix/ArrayOperation.pageemhttp/plugins/dynamix/BootInfo.pageemhttp/plugins/dynamix/BootParameters.pageemhttp/plugins/dynamix/DashStats.pageemhttp/plugins/dynamix/DeviceInfo.pageemhttp/plugins/dynamix/ManagementAccess.pageemhttp/plugins/dynamix/Notifications.pageemhttp/plugins/dynamix/SyslogSettings.pageemhttp/plugins/dynamix/include/DefaultPageLayout/MainContent.phpemhttp/plugins/dynamix/nchan/device_listemhttp/plugins/dynamix/nchan/update_1emhttp/plugins/dynamix/scripts/diagnosticsemhttp/plugins/dynamix/scripts/flash_backupemhttp/plugins/dynamix/scripts/monitoremhttp/plugins/dynamix/scripts/rsyslog_configemhttp/update.phpetc/rc.d/rc.6etc/rc.d/rc.S.contetc/rc.d/rc.sshdetc/rc.d/rc.udev
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
emhttp/languages/en_US/helptext.txt (1)
2177-2177: Consider standardizing “boot drive” to “boot device” for consistency.These updated lines use “boot drive” while nearby updated content uses “boot device.” Keeping one canonical term will make help text more consistent.
Suggested text-only cleanup
-Notifications may be stored permanently on the boot drive under folder '/boot/config/plugins/dynamix' instead. +Notifications may be stored permanently on the boot device under folder '/boot/config/plugins/dynamix' instead.-Click the edit button to add/modify/delete any modprobe.d config file in the config/modprobe.d directory on the boot drive. +Click the edit button to add/modify/delete any modprobe.d config file in the config/modprobe.d directory on the boot device.Also applies to: 2723-2723
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/languages/en_US/helptext.txt` at line 2177, Replace the inconsistent phrase "boot drive" with the canonical term "boot device" in this help text: specifically change "Notifications may be stored permanently on the boot drive under folder '/boot/config/plugins/dynamix' instead." to use "boot device"; also update the same phrase found later (around the other occurrence referenced) so all instances of "boot drive" are standardized to "boot device" while preserving the path '/boot/config/plugins/dynamix' and surrounding punctuation.emhttp/plugins/dynamix/nchan/device_list (2)
524-525: URL-encode thenamequery parameter when building device links.Line 525 interpolates
$realNamedirectly into the query string. Encoding it avoids malformed links for names with special characters.Proposed change
- ? "<a href=\"".htmlspecialchars("/Main/$source?name=$realName")."\">$fancy</a>" + ? "<a href=\"".htmlspecialchars("/Main/$source?name=".urlencode($realName), ENT_QUOTES)."\">$fancy</a>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/nchan/device_list` around lines 524 - 525, The link construction in the $link assignment interpolates $realName directly into the query string, which can produce malformed URLs for names with special characters; fix this by URL-encoding $realName (e.g., use rawurlencode($realName) or urlencode($realName)) when building the "/Main/$source?name=..." segment in the $link expression so the href remains valid, and keep the existing htmlspecialchars() call around the full URL.
1102-1105: Avoid double-translatingInternal Boot.Lines 1102 and 1223 translate the label first, then pass it again through
_($bootLabel,3). Keep a raw key and translate once to avoid inconsistent lookup behavior across locales.Proposed change
- $bootLabel = _("Internal Boot"); + $bootLabel = "Internal Boot"; $bootDeviceUrl = "/Main/Boot?name=".urlencode($bootPoolName); $bootLink = "<a href=\"$bootDeviceUrl\">"._($bootLabel,3)."</a>";Also applies to: 1223-1230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/nchan/device_list` around lines 1102 - 1105, The code double-translates the label by first assigning a translated string to $bootLabel and then passing it again into _() when building $bootLink; change to keep a raw key (e.g., $bootLabelKey = "Internal Boot") and call _($bootLabelKey) only once when creating $bootLink (or assign $bootLabel = _($bootLabelKey) and then use $bootLabel directly), ensuring $bootDeviceUrl, $bootLink and $summaryName use the single translated value; apply the same fix for the second occurrence around $bootLabel/$bootLink in the 1223-1230 block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 1064-1065: The linkHref is hardcoded to '/Main/Boot?name=flash'
but should use the actual boot target for the current row built from $bootDisk;
update the assignment that sets 'linkHref' (the array key 'linkHref' in the
device row construction) to interpolate the boot target from the $bootDisk
variable (e.g. the boot pool/name field on $bootDisk) instead of the literal
"flash", and ensure the value is URL-encoded when inserting into the query
string.
---
Nitpick comments:
In `@emhttp/languages/en_US/helptext.txt`:
- Line 2177: Replace the inconsistent phrase "boot drive" with the canonical
term "boot device" in this help text: specifically change "Notifications may be
stored permanently on the boot drive under folder '/boot/config/plugins/dynamix'
instead." to use "boot device"; also update the same phrase found later (around
the other occurrence referenced) so all instances of "boot drive" are
standardized to "boot device" while preserving the path
'/boot/config/plugins/dynamix' and surrounding punctuation.
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 524-525: The link construction in the $link assignment
interpolates $realName directly into the query string, which can produce
malformed URLs for names with special characters; fix this by URL-encoding
$realName (e.g., use rawurlencode($realName) or urlencode($realName)) when
building the "/Main/$source?name=..." segment in the $link expression so the
href remains valid, and keep the existing htmlspecialchars() call around the
full URL.
- Around line 1102-1105: The code double-translates the label by first assigning
a translated string to $bootLabel and then passing it again into _() when
building $bootLink; change to keep a raw key (e.g., $bootLabelKey = "Internal
Boot") and call _($bootLabelKey) only once when creating $bootLink (or assign
$bootLabel = _($bootLabelKey) and then use $bootLabel directly), ensuring
$bootDeviceUrl, $bootLink and $summaryName use the single translated value;
apply the same fix for the second occurrence around $bootLabel/$bootLink in the
1223-1230 block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7466d6c3-e74b-4997-b2f7-48e8d0fe9f17
📒 Files selected for processing (7)
.codex/coderabbit-fixes-wip.md.codex/coderabbit-review-rest.jsonemhttp/languages/en_US/helptext.txtemhttp/plugins/dynamix.plugin.manager/scripts/pluginemhttp/plugins/dynamix/BootInfo.pageemhttp/plugins/dynamix/nchan/device_listemhttp/plugins/dynamix/scripts/monitor
✅ Files skipped from review due to trivial changes (1)
- .codex/coderabbit-fixes-wip.md
🚧 Files skipped from review as they are similar to previous changes (2)
- emhttp/plugins/dynamix.plugin.manager/scripts/plugin
- emhttp/plugins/dynamix/BootInfo.page
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 1106-1108: The Boot page links are built from $bootPoolName which
can route incorrectly; update the link construction where $bootDeviceUrl and
$bootLink are created (variables $bootPoolName, $bootLabel, $bootDeviceUrl,
$bootLink) to use the fixed identifier "flash" instead of
urlencode($bootPoolName) — i.e., hardcode the URL to "/Main/Boot?name=flash" in
both places where these variables are set (the blocks around the current
$bootDeviceUrl/$bootLink assignments at the two occurrences).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24c3b50a-3645-4d19-a34c-c33e6234894a
📒 Files selected for processing (2)
emhttp/plugins/dynamix/nchan/device_listemhttp/plugins/dynamix/scripts/rsyslog_config
elibosley
approved these changes
Mar 5, 2026
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
3 participants
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.
Fix spin down of pools from main
Change flash to boot for text.
Restrict number of pool members to 2 if a boot pool
Fix monitoring of a boot device going offline if internal boot.
Change in rsyslog to check for entry being already in the config.
Summary by CodeRabbit
Documentation & UI Updates
UI Changes