feat(extensions): add netboot extension for TFTP+NFS diskless boot#9656
Draft
iav wants to merge 11 commits intoarmbian:mainfrom
Draft
feat(extensions): add netboot extension for TFTP+NFS diskless boot#9656iav wants to merge 11 commits intoarmbian:mainfrom
iav wants to merge 11 commits intoarmbian:mainfrom
Conversation
iav
requested review from
a team,
EvilOlaf,
TRSx80,
igorpecovnik and
littlecxm
as code owners
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new netboot mode ( Changes
Sequence DiagramsequenceDiagram
participant Builder as Builder Host
participant Ext as Netboot Extension
participant Staging as TFTP/NFS Staging
participant Remote as Remote Server
participant Target as Target U‑Boot
Builder->>Ext: build with ROOTFS_TYPE=nfs-root
Ext->>Staging: validate NETBOOT_*, normalize paths, stage kernel/DTB/uInitrd, generate PXE configs
Ext->>Staging: create rootfs archive or rsync export (preserve owners/xattrs/ACLs)
Ext->>Builder: emit netboot_artifacts_ready (TFTP tree, PXE config, ROOTFS_ARCHIVE)
Builder->>Remote: rsync TFTP payload + pxelinux configs
Builder->>Remote: upload & extract ROOTFS_ARCHIVE (optional, preserve metadata)
Target->>Remote: DHCP/PXE -> download kernel/DTB/initrd via TFTP
Target->>Remote: mount root via NFS or use DHCP option 17 (initramfs-assisted)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
05
![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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/netboot.sh`:
- Around line 38-42: The function
extension_prepare_config__netboot_force_nfs_rootfs sets ROOTFS_TYPE too late (it
runs during do_extra_configuration) so NFS-specific branches in
do_main_configuration never see it; move the ROOTFS_TYPE assignment earlier by
ensuring extension_prepare_config__netboot_force_nfs_rootfs (or its logic) runs
before do_main_configuration—either call that function in the init/startup hook
that executes prior to main configuration (e.g., extension_init or top-level
script before do_main_configuration) or export/set declare -g ROOTFS_TYPE="nfs"
at script initialization so the global variable is in effect for
do_main_configuration and the NFS setup in
lib/functions/configuration/main-config.sh executes.
- Around line 147-149: The PXE staging copies ${MOUNT}/boot/uInitrd but never
references it in the generated extlinux.cfg, so U-Boot won't load the initramfs;
modify the netboot stanza generation to set an initrd_line variable when the
file exists (mirror how fdt_line is created — e.g. check for
"${MOUNT}/boot/uInitrd", set initrd_line="INITRD ${tftp_prefix_dir}/uInitrd")
and then include ${initrd_line} in the emitted PXE stanza alongside fdt_line so
the INITRD directive is present for U-Boot.
In `@extensions/netboot/README.md`:
- Around line 137-155: The README has multiple unlabeled fenced code blocks
(e.g., the directory tree block showing "/srv/netboot/" and other blocks around
lines 159-164, 171-175, 465-489, 544-547); update each triple-backtick fence to
include an appropriate info string (for example use sh, ini, or text as
appropriate) so markdownlint stops flagging them—locate the fences by searching
for the blocks that display the directory tree and configuration snippets and
add the matching language tag to each opening ``` line.
In `@lib/functions/image/rootfs-to-image.sh`:
- Around line 82-93: The rsync into ROOTFS_EXPORT_DIR can leave stale files
behind when the export tree is reused; update the rsync invocation in
rootfs-to-image.sh (the run_host_command_logged rsync call) to include --delete
(and optionally --delete-excluded) so files removed from the source are removed
from "${ROOTFS_EXPORT_DIR}" as well; add the flag either into $rsync_ea or
directly in the rsync command invocation to ensure exported NFS root mirrors the
built image.
- Around line 69-76: The archive creation pipeline (tar … | pv … |
${archive_filter} > "${ROOTFS_ARCHIVE_PATH}") must be guarded with pipefail so
failures in tar or pv aren't masked by a later stage; wrap that pipeline in a
shell context with set -o pipefail (or enable set -o pipefail locally before
running the pipeline) so a nonzero exit from tar or pv will cause the script to
fail and propagate the error from creating ROOTFS_ARCHIVE_PATH; update the block
around display_alert/ tar / pv / ${archive_filter} to ensure pipefail is active
for that pipeline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fca3bef5-36a8-4aaa-81dd-645853e7579e
📒 Files selected for processing (5)
config/templates/nfs-boot.cmd.templateextensions/netboot/README.mdextensions/netboot/netboot.shlib/functions/configuration/main-config.shlib/functions/image/rootfs-to-image.sh
iav
added a commit
to iav/armbian
that referenced
this pull request
Apr 12, 2026
`nfs-root` is a new rootfs type distinct from the existing `nfs` hybrid mode. Selecting it wires the `netboot` extension from the core `ROOTFS_TYPE` dispatch in `do_main_configuration`, so callers no longer need a separate `ENABLE_EXTENSIONS=netboot`. The legacy `nfs` branch (kernel+DTB on local boot partition, `/` over NFS) is untouched — both paths coexist until the hybrid mode's future is decided. Core plumbing mirrors the `nfs` branch for all paths where local root storage would be meaningless: partition layout skip (`prepare_partitions`), archive/export gate and version suffix (`rootfs-to-image.sh`), and the host-side filesystem compatibility check in `main-config.sh`. Extension hooks now key on `ROOTFS_TYPE=nfs-root` instead of guessing from `nfs`, removing the `force ROOTFS_TYPE=nfs` shim that ran too late relative to `do_main_configuration`. Also folded in from CodeRabbit review on PR armbian#9656: - pipefail around tar|pv|compressor so truncated archives no longer slip through on an intermediate stage failure - `rsync --delete` on `ROOTFS_EXPORT_DIR` so stale files from a previous build don't linger in the NFS export tree - explicit `INITRD` directive in extlinux when `uInitrd` is staged; U-Boot only loads an initramfs when the stanza names it - README updated to document `ROOTFS_TYPE=nfs-root` as the single switch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
Very nice. With recent u-boot (2026.04, thanks to Kwiboo), one can enable LWIP networking stack for better TFTP performance (loading kernel and initramfs) -- there's also some TFTP Window Size stuff that can be tuned. U-boot with LWIP + u-boot's |
Contributor
Author
|
Thanks for the pointer! Let's get this merged first — iterating on something that works is the easy part. |
iav
force-pushed
the
enh/netboot-extension
branch
from
a76e4b2 to
fec99ac
Compare
iav
force-pushed
the
enh/netboot-extension
branch
5 times, most recently
from
a10feb4 to
4ff6016
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
Author
|
Pull my Helios64 to v2026.04 in #9675 |
iav
force-pushed
the
enh/netboot-extension
branch
4 times, most recently
from
a61ff64 to
dac19c8
Compare
Contributor
Author
|
@coderabbitai thanks! |
Contributor
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extensions/netboot/netboot.sh (1)
317-350: ⚡ Quick winSkip the root-path plumbing when
NETBOOT_SERVERis already fixed.When
NETBOOT_SERVERis set, the PXE stanza uses a literalnfsroot=${NETBOOT_SERVER}:${NETBOOT_NFS_PATH}and never consumes DHCP option 17. This hook still rewritesdhcpcd.conf, installs both hooks, and rebuilds initramfs on everynfs-rootbuild, which is just extra churn on the explicit-server path.♻️ Suggested simplification
function post_customize_image__netboot_request_root_path() { [[ "${ROOTFS_TYPE}" == "nfs-root" ]] || return 0 + [[ -z "${NETBOOT_SERVER}" ]] || return 0 declare conf="${SDCARD}/etc/dhcpcd.conf" declare dhcpcd_hook="${SDCARD}/usr/share/initramfs-tools/dhcpcd-hooks/71-netboot-rootpath" declare initramfs_hook="${SDCARD}/etc/initramfs-tools/hooks/netboot-rootpath" declare changed=no🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/netboot.sh` around lines 317 - 350, The post_customize_image__netboot_request_root_path function always applies root_path plumbing for ROOTFS_TYPE == "nfs-root" even when NETBOOT_SERVER is set; update the function to early-return when NETBOOT_SERVER is non-empty (i.e., skip editing ${SDCARD}/etc/dhcpcd.conf, installing ${dhcpcd_hook} and ${initramfs_hook}, and rebuilding initramfs) so the explicit-server PXE path is not modified or rebuilt; locate the function post_customize_image__netboot_request_root_path and add a guard like [[ -n "${NETBOOT_SERVER}" ]] && return 0 (or equivalent) immediately after checking ROOTFS_TYPE.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/netboot.sh`:
- Around line 220-235: host_pre_docker_launch__netboot_bindmount_export_symlink
currently skips the external-export safety checks so a symlink at
${SRC}/output/netboot-export can point to arbitrary host paths; update this
function to call the same validation used elsewhere
(_netboot_validate_external_export_dir) for the resolved symlink target (and
validate ROOTFS_EXPORT_DIR semantics), reject targets that are blacklisted or
empty, and only proceed to append DOCKER_EXTRA_ARGS when validation passes;
ensure you still enforce the comma-free path check and surface the same
exit_with_error messages when validation fails.
---
Nitpick comments:
In `@extensions/netboot/netboot.sh`:
- Around line 317-350: The post_customize_image__netboot_request_root_path
function always applies root_path plumbing for ROOTFS_TYPE == "nfs-root" even
when NETBOOT_SERVER is set; update the function to early-return when
NETBOOT_SERVER is non-empty (i.e., skip editing ${SDCARD}/etc/dhcpcd.conf,
installing ${dhcpcd_hook} and ${initramfs_hook}, and rebuilding initramfs) so
the explicit-server PXE path is not modified or rebuilt; locate the function
post_customize_image__netboot_request_root_path and add a guard like [[ -n
"${NETBOOT_SERVER}" ]] && return 0 (or equivalent) immediately after checking
ROOTFS_TYPE.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abfb312f-a24b-4817-8d8e-8c05f4551ae1
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
Contributor
Author
|
Re: review nitpick about skipping the DHCP option 17 plumbing when Cost of keeping it: one line in Reverse cost (skip path): the rootfs becomes implicitly tied to the literal nfsroot at build time; switching strategies later requires rebuild + re-deploy. Removing config you don't need is always easier than adding plumbing you don't know about later. |
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 `@extensions/netboot/netboot.sh`:
- Around line 204-212: The current safety check uses presence of
"${path}/etc/os-release" which is too generic; change the guard in the block
that exits via exit_with_error (uses variables path, EXTENSION,
NETBOOT_EXPORT_FORCE) to look for an Armbian-specific marker instead (e.g.,
"${path}/etc/armbian-release" or a dedicated stamp like
"${path}/.netboot_export_marker"), and ensure the export routine stamps that
marker when creating an export tree so subsequent runs recognize a genuine
Armbian export; update the error message to mention the specific marker checked
and keep NETBOOT_EXPORT_FORCE behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36928a16-449f-41fd-ae70-5d8e256c0296
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extensions/netboot/README.md (1)
199-217: ⚡ Quick winClarify
pxelinux.cfg/01-<mac>vs tagged per-MAC configs.The directory layout snippet shows:
pxelinux.cfg/defaultpxelinux.cfg/01-aa-bb-cc-dd-ee-ffBut earlier the README describes tagged per-MAC PXE configs like
01-<mac>.<board>-<branch>-<release>[-<hostname>].example, with01-<mac>being the U-Boot lookup name that should be symlinked to the tagged file.Consider a small wording tweak so readers don’t think the tagged per-MAC config file is literally named
01-<mac>.🛠️ Suggested doc tweak
/srv/netboot/tftp pxelinux.cfg/ - default # the file U-Boot pxe get downloads - 01-aa-bb-cc-dd-ee-ff # per-MAC override (lowercase, dashes) + default # the active config file U-Boot downloads + 01-aa-bb-cc-dd-ee-ff # per-MAC override *symlink* (U-Boot lookup name) armbian/ <family>/<board>/<branch>-<release>/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` around lines 199 - 217, The README snippet misleadingly suggests the per-MAC PXE file is literally named "pxelinux.cfg/01-<mac>"; update the wording to clarify that U-Boot looks up a bare name like pxelinux.cfg/01-<mac> which should be a symlink pointing to a tagged filename such as 01-<mac>.<board>-<branch>-<release>[-<hostname>].example (and keep examples of both the lookup name "pxelinux.cfg/default" and the tagged target pattern to make the relationship explicit).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/README.md`:
- Around line 609-618: The README omits the one-time activation step for the
catch-all pxelinux config when NETBOOT_CLIENT_MAC is unset: after rsyncing the
generated pxelinux.cfg/*-netboot-tftp/ files (which include
pxelinux.cfg/<board>-<branch>-<release>.example), document that you must promote
the generated default example into the DHCP-expected catch-all by renaming or
symlinking pxelinux.cfg/default.example to pxelinux.cfg/default (i.e., a
one-time mv or ln -sfn from pxelinux.cfg/default.example → pxelinux.cfg/default)
before relying on the board-level symlink such as default-arm-rk3399-helios64 to
take effect.
---
Nitpick comments:
In `@extensions/netboot/README.md`:
- Around line 199-217: The README snippet misleadingly suggests the per-MAC PXE
file is literally named "pxelinux.cfg/01-<mac>"; update the wording to clarify
that U-Boot looks up a bare name like pxelinux.cfg/01-<mac> which should be a
symlink pointing to a tagged filename such as
01-<mac>.<board>-<branch>-<release>[-<hostname>].example (and keep examples of
both the lookup name "pxelinux.cfg/default" and the tagged target pattern to
make the relationship explicit).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adf2e355-0ae7-4ed3-aed2-4e2f0e3c477e
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
✅ Files skipped from review due to trivial changes (1)
- extensions/netboot/netboot.sh
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 `@extensions/netboot/netboot.sh`:
- Around line 238-243: The function
host_pre_docker_launch__netboot_bindmount_export_symlink currently runs for
nfs-root even when ROOTFS_EXPORT_DIR is unset; update it to also check that
ROOTFS_EXPORT_DIR is non-empty before proceeding (e.g. return 0 if
ROOTFS_EXPORT_DIR is unset/empty) so the bind-mount is only performed for
explicit export usage; keep the existing checks for ROOTFS_TYPE, the link
variable ("${SRC}/output/netboot-export") and the readlink call intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 875deb73-1a00-4804-969a-009cc101fcc2
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
extensions/netboot/README.md (1)
729-731:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the second dhcpcd hook path in troubleshooting.
The second fallback path is off: it should be
/usr/lib/dhcpcd/dhcpcd-hooks/(not/usr/lib/dhcpcd-hooks/), otherwise readers can get a false negative while debugging option 17 plumbing.Suggested doc patch
- `/usr/libexec/dhcpcd-hooks/` or `/usr/lib/dhcpcd-hooks/` (the + `/usr/libexec/dhcpcd-hooks/` or `/usr/lib/dhcpcd/dhcpcd-hooks/` (the🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` around lines 729 - 731, Update the troubleshooting text that refers to the dhcpcd hook fallback path: replace the incorrect path "/usr/lib/dhcpcd-hooks/" with the correct "/usr/lib/dhcpcd/dhcpcd-hooks/" so the sentence about "71-netboot-rootpath" correctly lists `/usr/libexec/dhcpcd-hooks/` or `/usr/lib/dhcpcd/dhcpcd-hooks/`; adjust the string in the README entry referencing "71-netboot-rootpath" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/netboot.sh`:
- Around line 149-183: _netboot_normalize_export_dir currently skips the
non-empty-target / .netboot_export_marker guard for under-base exports (e.g.
ROOTFS_EXPORT_DIR="my-host") because it only calls
_netboot_validate_external_export_dir for absolute-external paths or after
symlink resolution; to fix, invoke _netboot_validate_external_export_dir for any
export that will resolve under the netboot base (both the branch that
canonicalizes to "${SRC}/output/netboot-export/${ROOTFS_EXPORT_DIR}" and the
branch that rewrites from "${host_src}/output/netboot-export" into
"${SRC}/output/netboot-export${rest}"), passing the final computed path (or set
_NETBOOT_EXPORT_DIR_EXTERNAL and run validation) so the .netboot_export_marker /
non-empty-target checks are enforced for under-base exports as well; keep the
existing resolved symlink validation intact.
---
Duplicate comments:
In `@extensions/netboot/README.md`:
- Around line 729-731: Update the troubleshooting text that refers to the dhcpcd
hook fallback path: replace the incorrect path "/usr/lib/dhcpcd-hooks/" with the
correct "/usr/lib/dhcpcd/dhcpcd-hooks/" so the sentence about
"71-netboot-rootpath" correctly lists `/usr/libexec/dhcpcd-hooks/` or
`/usr/lib/dhcpcd/dhcpcd-hooks/`; adjust the string in the README entry
referencing "71-netboot-rootpath" accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b2640b7-2305-4723-a48b-8aeb66a96d30
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
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 `@extensions/netboot/netboot.sh`:
- Around line 234-252: The hook
host_pre_docker_launch__netboot_bindmount_export_symlink should skip
bind-mounting when export mode is "external" — i.e. when ROOTFS_EXPORT_DIR is
set to an absolute external path — to avoid exposing arbitrary host paths; add a
guard after the existing ROOTFS_EXPORT_DIR check that returns early if
ROOTFS_EXPORT_DIR is an absolute path (e.g. starts with "/") or points outside
the repo (not under "${SRC}/output/netboot-export"), and apply the same
early-return guard to the corresponding bind-mount function handling the export
dir (the one around lines 270-278) so neither function binds the symlink target
when export is external.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd0ca0a4-5ea2-4053-9d39-49f8aa461034
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
extensions/netboot/README.md (3)
500-502: ⚡ Quick winAdd a “one-time activation” reminder to the “Default: pxelinux.cfg/default” pattern.
In Multi-board patterns, Pattern
#1says:
- “Default:
pxelinux.cfg/defaultpoints at it.” (Line 501)But
pxelinux.cfg/defaultis not automatically correct until the operator performs the one-time promotion/activation step(s) described earlier. A short reminder here would prevent confusion.Suggested doc tweak
-1. **One board, one image.** Default: `shared/<family>/<board>/<branch>-<release>/`, - `pxelinux.cfg/default` points at it. +1. **One board, one image.** Default: `shared/<family>/<board>/<branch>-<release>/`, + and (after the one-time activation step) `pxelinux.cfg/default` points at it.Based on learnings, the extension intentionally writes a
*.examplefile (notdefault) in some cases, and the operator must perform a one-timemv/lnactivation so U-Boot’sbootfile=defaultlookup actually lands on the generated config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` around lines 500 - 502, Add a one-line reminder to Pattern `#1` stating that pxelinux.cfg/default is not created automatically and requires the one-time activation/promotion step described earlier (the extension writes a *.example file by default, so the operator must mv/ln the generated *.example to pxelinux.cfg/default or perform the documented activation) — reference the pattern path shared/<family>/<board>/<branch>-<release>/, the pxelinux.cfg/default lookup, and the *.example filename so readers know exactly which files to promote.
407-417: ⚡ Quick winMake the builder-as-NFS symlink route idempotent (
ln -sfn).The symlink-route command is:
ln -s /srv/netboot/rootfs "$PWD/output/netboot-export"(Lines 416-417)This can fail or leave users in a confusing state if
output/netboot-exportalready exists from a previous run (especially when iterating on images/configs).Prefer
ln -sfnto overwrite/update the symlink safely.Suggested doc tweak
-ln -s /srv/netboot/rootfs "$PWD/output/netboot-export" +ln -sfn /srv/netboot/rootfs "$PWD/output/netboot-export"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` around lines 407 - 417, The README's one-time symlink command (the line creating output/netboot-export pointing at /srv/netboot/rootfs) is not idempotent and will fail or produce confusing results if output/netboot-export already exists; update the example to use a force/replace symlink invocation so the link is overwritten safely (use ln with the equivalent of -sfn) when recreating or updating the builder-as-NFS symlink (the example command that currently reads ln -s /srv/netboot/rootfs "$PWD/output/netboot-export").
718-720: 💤 Low valueFix minor formatting glitch (“mountd port”) in troubleshooting.
There’s a visible extra whitespace in:
- “randomized mountd port” (Line 718)
Minor, but it stands out in a troubleshooting checklist. Remove the double spaces.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` around lines 718 - 720, Update the README troubleshooting text that currently contains the double space in the phrase "randomized mountd port" — locate the checklist item mentioning "Server firewall is blocking TCP/2049 (or the randomized mountd port for NFSv3). Pin mountd and open the port." and change the triple-space sequence to a single space so it reads "randomized mountd port". Ensure no other unintended whitespace changes are introduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/netboot.sh`:
- Around line 122-133: The script is normalizing/validating ROOTFS_EXPORT_DIR
and always setting up host bindmount hooks even when ROOTFS_COMPRESSION is an
archive mode; restrict those operations to the no-export case by wrapping the
_netboot_normalize_export_dir call and the two
host_pre_docker_launch__netboot_bindmount_* hook definitions/registrations in a
guard that only runs when ROOTFS_COMPRESSION == "none" (use the same test style
already used elsewhere), and update the error/exit logic accordingly so archive
modes (gzip|zstd|zst) do not validate or bind-mount ROOTFS_EXPORT_DIR or
register the host bindmount hooks.
In `@extensions/netboot/README.md`:
- Around line 68-69: Update the NETBOOT_CLIENT_MAC table row to clarify that
NETBOOT_CLIENT_MAC is the raw user-provided value (accepting aa:bb:cc:dd:ee:ff
or aa-bb-cc-dd-ee-ff forms), but the per-MAC PXE filename and lookup use a
normalized/cleaned MAC; point readers to the hook context variable
NETBOOT_PXE_FILE which contains the exact generated filename used for 01-<mac>
symlinks (i.e., instruct operators to use NETBOOT_PXE_FILE or the normalized MAC
form when creating the 01-<mac> symlink).
---
Nitpick comments:
In `@extensions/netboot/README.md`:
- Around line 500-502: Add a one-line reminder to Pattern `#1` stating that
pxelinux.cfg/default is not created automatically and requires the one-time
activation/promotion step described earlier (the extension writes a *.example
file by default, so the operator must mv/ln the generated *.example to
pxelinux.cfg/default or perform the documented activation) — reference the
pattern path shared/<family>/<board>/<branch>-<release>/, the
pxelinux.cfg/default lookup, and the *.example filename so readers know exactly
which files to promote.
- Around line 407-417: The README's one-time symlink command (the line creating
output/netboot-export pointing at /srv/netboot/rootfs) is not idempotent and
will fail or produce confusing results if output/netboot-export already exists;
update the example to use a force/replace symlink invocation so the link is
overwritten safely (use ln with the equivalent of -sfn) when recreating or
updating the builder-as-NFS symlink (the example command that currently reads ln
-s /srv/netboot/rootfs "$PWD/output/netboot-export").
- Around line 718-720: Update the README troubleshooting text that currently
contains the double space in the phrase "randomized mountd port" — locate the
checklist item mentioning "Server firewall is blocking TCP/2049 (or the
randomized mountd port for NFSv3). Pin mountd and open the port." and change
the triple-space sequence to a single space so it reads "randomized mountd
port". Ensure no other unintended whitespace changes are introduced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf8dc942-2b89-4786-8fa1-26fa535f46e8
📒 Files selected for processing (2)
extensions/netboot/README.mdextensions/netboot/netboot.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/README.md`:
- Around line 563-565: The README currently implies SSH host keys change on
every reboot by saying "per boot"; update the wording to reflect the actual
behavior of armbian-firstrun.service by replacing that phrase with a clear
statement like "on first boot of a fresh rootfs" or "on first boot (then
persisted)" so it references armbian-firstrun.service and clarifies keys are
regenerated only once for a fresh rootfs and then persist in the NFS tree.
- Around line 3-7: The README text describing the netboot flow implies a fully
storage-less boot, but the nfs-root mode still requires a pre-existing
PXE-capable bootloader (e.g., U-Boot) on the board to fetch kernel/DTB via TFTP;
update the paragraphs mentioning "nfs-root" and the boot flow (the sentences
describing kernel+DTB over TFTP and mounting root over NFS) to explicitly state
that a PXE-capable bootloader must already be present on the target device (or
alternative boot ROM) before TFTP/NFS will work, and add a brief note about
operator responsibility to provision that bootloader (reference the term
"nfs-root" and the description lines mentioning kernel+DTB over TFTP / mounts
root over NFS).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93beb213-5d73-493a-bf32-1d6c1a2f102f
📒 Files selected for processing (3)
extensions/netboot/README.mdextensions/netboot/netboot-deploy.shextensions/netboot/netboot.sh
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
extensions/netboot/README.md (1)
178-186:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to this fenced block to satisfy markdownlint (MD040).
This unlabeled fence can fail docs lint; use a neutral tag like
text.Proposed fix
-``` +```text <ver>-netboot-tftp/ pxelinux.cfg/ <board>-<branch>-<release>[-<hostname>].example # tagged file (or 01-<mac>.<...> with NETBOOT_CLIENT_MAC) <NETBOOT_TFTP_PREFIX>/ # default: armbian/<family>/<board>/<branch>-<release>/ Image # or zImage on armhf dtb/<...>/*.dtb uInitrd # only if /boot/uInitrd was produced</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@extensions/netboot/README.mdaround lines 178 - 186, The fenced code block
in the README (the unlabeled triple-backtick block showing the netboot-tftp
layout) needs a language tag to satisfy markdownlint MD040; update the opening
fence fromtotext (or another neutral tag) so the block becomes
labelled, leaving the contents unchanged and keeping the closing ``` as-is.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@extensions/netboot/README.md:
- Around line 178-186: The fenced code block in the README (the unlabeled
triple-backtick block showing the netboot-tftp layout) needs a language tag to
satisfy markdownlint MD040; update the opening fence fromtotext (or
another neutral tag) so the block becomes labelled, leaving the contents
unchanged and keeping the closing ``` as-is.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `c2c15ddc-c379-4970-b592-cf91789be6fc` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e675c1c559c772cd7c18ffd0599bb5f537b0d5b2 and efa2488efa3b08ca35a130a4fecfc9345fda293c. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `extensions/netboot/README.md` * `extensions/netboot/netboot-deploy.sh` * `extensions/netboot/netboot.sh` </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * extensions/netboot/netboot.sh </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * extensions/netboot/netboot-deploy.sh </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Contributor
Author
|
Re: markdownlint MD040 duplicate from review 4209238665 — the only unlabeled fenced block I newly introduced ("Build artifacts matrix" TFTP-tree layout) now carries a |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
extensions/netboot/README.md (2)
77-77: 💤 Low valueDe-emphasize the overused “exactly”.
You use “that’s exactly what U-Boot’s … resolution requests” in the
NETBOOT_CLIENT_MACrow. It’s correct, but the word is called out by static analysis as overused; removing “exactly” improves readability without changing meaning.Suggested doc wording
-... that’s exactly what U-Boot's PXELINUX per-MAC fallback resolution requests. +... that’s what U-Boot's PXELINUX per-MAC fallback resolution requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` at line 77, In the NETBOOT_CLIENT_MAC README row update the wording to remove the overused adverb "exactly" — change the phrase "that's exactly what U-Boot's PXELINUX per-MAC fallback resolution requests" to a simpler "that's what U-Boot's PXELINUX per-MAC fallback resolution requests" (preserve the surrounding explanation about lowercase + dashes and the NETBOOT_PXE_FILE exposure).
79-80: ⚡ Quick winClarify when
ROOTFS_EXPORT_DIRmust pre-exist on the host.The
ROOTFS_EXPORT_DIRtable row says: “The directory must exist on the host before the build…”. For external absolute paths, that’s correct; but for relative values confined under${SRC}/output/netboot-export/<value>, the build is typically able to create/populate paths under the build output.Consider qualifying the sentence so users don’t create unnecessary directories for the relative (confined) case.
Suggested doc fix
-| Relative** value (e.g. `shared/rockchip64/helios64/edge-trixie`) is confined under `${SRC}/output/netboot-export/<value>` so `rsync --delete` cannot escape that subtree. **Absolute path outside the build tree** (e.g. `/srv/netboot/rootfs/shared/<board>/<branch>-<release>` or `/nfsroot`) is kept as-is and bind-mounted into the container at the same path; rsync writes straight into the host export tree. The directory must exist on the host before the build (typically `sudo mkdir -p` for root-owned NFS roots). Primary use: builder host is also the NFS server — single-step `build → boot` loop, no tar/unpack/rsync hop. System roots (`/`, `/etc`, `/usr`, ...) and `..` segments are rejected. The build stamps a `.netboot_export_marker` at the root of every export tree it writes; a non-empty target without that marker is refused (so `rsync --delete` cannot wipe an unrelated Linux tree at the same path) unless `NETBOOT_EXPORT_FORCE=yes`. +| Relative** value (e.g. `shared/rockchip64/helios64/edge-trixie`) is confined under `${SRC}/output/netboot-export/<value>` so `rsync --delete` cannot escape that subtree. **Absolute path outside the build tree** (e.g. `/srv/netboot/rootfs/shared/<board>/<branch>-<release>` or `/nfsroot`) is kept as-is and bind-mounted into the container at the same path; rsync writes straight into the host export tree. For absolute/external paths, the directory must exist on the host before the build (typically `sudo mkdir -p` for root-owned NFS roots). Primary use: builder host is also the NFS server — single-step `build → boot` loop, no tar/unpack/rsync hop. System roots (`/`, `/etc`, `/usr`, ...) and `..` segments are rejected. The build stamps a `.netboot_export_marker` at the root of every export tree it writes; a non-empty target without that marker is refused (so `rsync --delete` cannot wipe an unrelated Linux tree at the same path) unless `NETBOOT_EXPORT_FORCE=yes`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/netboot/README.md` around lines 79 - 80, The README text for ROOTFS_EXPORT_DIR is ambiguous about when the directory must pre-exist; update the table row for ROOTFS_EXPORT_DIR to clearly state that only absolute external paths (e.g. /srv/netboot/... or /nfsroot) must exist on the host before the build and will be bind-mounted as-is, whereas relative values confined under ${SRC}/output/netboot-export/<value> are created/populated by the build process and do not require pre-creation; keep the note about the .netboot_export_marker and NETBOOT_EXPORT_FORCE behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/README.md`:
- Around line 703-713: The sample boot log is inconsistent: the APPEND/kernel
cmdline example contains "rw" (the line starting with "append: root=/dev/nfs ...
rw") while the following kernel output shows "VFS: Mounted root (nfs filesystem)
readonly on device 0:17."; update the README so these two lines match—either
change the APPEND example to use "ro" (if the root was actually mounted
read-only) or change the VFS line in the sample to "rw"/remove "readonly" to
reflect a read-write mount; ensure the two literal strings ("append:
root=/dev/nfs ... rw" and "VFS: Mounted root (nfs filesystem) readonly on device
0:17.") are consistent.
---
Nitpick comments:
In `@extensions/netboot/README.md`:
- Line 77: In the NETBOOT_CLIENT_MAC README row update the wording to remove the
overused adverb "exactly" — change the phrase "that's exactly what U-Boot's
PXELINUX per-MAC fallback resolution requests" to a simpler "that's what
U-Boot's PXELINUX per-MAC fallback resolution requests" (preserve the
surrounding explanation about lowercase + dashes and the NETBOOT_PXE_FILE
exposure).
- Around line 79-80: The README text for ROOTFS_EXPORT_DIR is ambiguous about
when the directory must pre-exist; update the table row for ROOTFS_EXPORT_DIR to
clearly state that only absolute external paths (e.g. /srv/netboot/... or
/nfsroot) must exist on the host before the build and will be bind-mounted
as-is, whereas relative values confined under
${SRC}/output/netboot-export/<value> are created/populated by the build process
and do not require pre-creation; keep the note about the .netboot_export_marker
and NETBOOT_EXPORT_FORCE behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2afa6525-7b8b-4e73-8c55-cb66d068cc49
📒 Files selected for processing (3)
extensions/netboot/README.mdextensions/netboot/netboot-deploy.shextensions/netboot/netboot.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- extensions/netboot/netboot-deploy.sh
- extensions/netboot/netboot.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/netboot/netboot.sh`:
- Around line 386-405: The install logic for dhcpcd_hook and initramfs_hook
should unconditionally overwrite the target hooks and force the initramfs
rebuild instead of only installing when the file is missing; remove the [[ ! -f
"${dhcpcd_hook}" ]] and [[ ! -f "${initramfs_hook}" ]] guards so
run_host_command_logged install -D -m 0755
"${EXTENSION_DIR}/files/dhcpcd-hooks/71-netboot-rootpath" "${dhcpcd_hook}" and
run_host_command_logged install -D -m 0755
"${EXTENSION_DIR}/files/initramfs-hooks/netboot-rootpath" "${initramfs_hook}"
always execute, ensure changed is set to "yes" after each install, and keep the
chroot_sdcard update-initramfs -u call guarded by if [[ "${changed}" == "yes" ]]
so the new hooks are always included in the uInitrd.
In `@extensions/netboot/README.md`:
- Around line 611-655: The Helios64 example block is inconsistent with the
captured log; update the sample so all values match (or clearly mark the log as
an older capture). Ensure NETBOOT_SERVER, ROOTFS_EXPORT_DIR, ROOTFS_TYPE,
BUILD_MINIMAL and the BUILD BRANCH/RELEASE (e.g., edge/trixie) in the compile.sh
invocation match the values shown in the log (remove mismatched "edge-noble",
path-only "nfsroot=", and bootserver/rootserver=192.168.1.65), or add a short
note before the log stating it is from a different run with its exact settings;
keep the compile.sh invocation and the log consistent and self-contained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 577ff317-7d1a-4d1d-ac15-f148feca461e
📒 Files selected for processing (12)
config/templates/nfs-boot.cmd.templateextensions/netboot/README.mdextensions/netboot/files/dhcpcd-hooks/71-netboot-rootpathextensions/netboot/files/initramfs-hooks/netboot-rootpathextensions/netboot/files/initramfs-scripts/nfs-bottom/zz-netboot-watchdog-cancelextensions/netboot/netboot-deploy.shextensions/netboot/netboot.shlib/functions/configuration/main-config.shlib/functions/image/partitioning.shlib/functions/image/rootfs-to-image.shlib/functions/rootfs/create-cache.shlib/functions/rootfs/rootfs-create.sh
✅ Files skipped from review due to trivial changes (3)
- lib/functions/configuration/main-config.sh
- lib/functions/rootfs/rootfs-create.sh
- lib/functions/image/partitioning.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- extensions/netboot/files/initramfs-hooks/netboot-rootpath
- lib/functions/rootfs/create-cache.sh
- extensions/netboot/netboot-deploy.sh
- lib/functions/image/rootfs-to-image.sh
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 `@extensions/netboot/netboot.sh`:
- Around line 438-440: The script currently silently skips copying DTB from
"${MOUNT}/boot/dtb" to "${tftp_prefix_dir}/dtb/" but still emits a PXE FDTDIR
pointing at "${NETBOOT_TFTP_PREFIX}/dtb", causing broken PXE configs; modify the
logic around the DTB staging (check existence of "${MOUNT}/boot/dtb" and/or
"${tftp_prefix_dir}/dtb") to fail fast or omit the FDTDIR fallback: either (A)
if no DTB tree is staged, log an error and exit non-zero so the build fails
early, or (B) when producing the PXE config, only emit the FDTDIR
${NETBOOT_TFTP_PREFIX}/dtb line if the destination dtb directory actually exists
and contains files; update the blocks that reference MOUNT, tftp_prefix_dir,
NETBOOT_TFTP_PREFIX and the FDTDIR emission to implement one of these two
behaviors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c2a396a-6903-4c51-b156-2826d8b7ff24
📒 Files selected for processing (13)
config/templates/nfs-boot.cmd.templateextensions/netboot/README.mdextensions/netboot/files/dhcpcd-hooks/71-netboot-rootpathextensions/netboot/files/initramfs-hooks/netboot-rootpathextensions/netboot/files/initramfs-scripts/init-premount/zz-netboot-watchdogextensions/netboot/files/initramfs-scripts/nfs-bottom/zz-netboot-watchdog-cancelextensions/netboot/netboot-deploy.shextensions/netboot/netboot.shlib/functions/configuration/main-config.shlib/functions/image/partitioning.shlib/functions/image/rootfs-to-image.shlib/functions/rootfs/create-cache.shlib/functions/rootfs/rootfs-create.sh
✅ Files skipped from review due to trivial changes (2)
- lib/functions/rootfs/rootfs-create.sh
- config/templates/nfs-boot.cmd.template
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/functions/rootfs/create-cache.sh
- lib/functions/image/rootfs-to-image.sh
- extensions/netboot/netboot-deploy.sh
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 `@extensions/netboot/README.md`:
- Around line 437-444: The README incorrectly states that rsync performs kernel
reflinks on btrfs/xfs; update the text near ROOTFS_EXPORT_DIR to remove or
reword the claim that "the kernel reflinks the rootfs tree" and instead explain
that while bind-mounting and rsync preserve ownership, rsync will copy data
unless explicit reflink tooling or filesystem-level copy-on-write (e.g., cp
--reflink or filesystem snapshot mechanisms) is used; reference
ROOTFS_EXPORT_DIR and the rsync-in-container behavior so readers understand the
export remains fast only when reflink/cow mechanisms are explicitly employed,
not automatically by rsync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e89d51b-0cb5-41e2-936a-6b5ab8d6bd19
📒 Files selected for processing (13)
config/templates/nfs-boot.cmd.templateextensions/netboot/README.mdextensions/netboot/files/dhcpcd-hooks/71-netboot-rootpathextensions/netboot/files/initramfs-hooks/netboot-rootpathextensions/netboot/files/initramfs-scripts/init-premount/zz-netboot-watchdogextensions/netboot/files/initramfs-scripts/nfs-bottom/zz-netboot-watchdog-cancelextensions/netboot/netboot-deploy.shextensions/netboot/netboot.shlib/functions/configuration/main-config.shlib/functions/image/partitioning.shlib/functions/image/rootfs-to-image.shlib/functions/rootfs/create-cache.shlib/functions/rootfs/rootfs-create.sh
✅ Files skipped from review due to trivial changes (3)
- extensions/netboot/files/initramfs-scripts/nfs-bottom/zz-netboot-watchdog-cancel
- lib/functions/image/partitioning.sh
- config/templates/nfs-boot.cmd.template
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/functions/rootfs/rootfs-create.sh
- extensions/netboot/netboot-deploy.sh
- extensions/netboot/netboot.sh
- lib/functions/image/rootfs-to-image.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@extensions/netboot/README.md`:
- Around line 659-660: The example uses a glob "output/images/*-netboot-tftp/"
which can accidentally merge multiple build outputs into "/srv/netboot/tftp/";
update the README example to point at a specific build directory instead of the
wildcard (e.g., replace "output/images/*-netboot-tftp/" with the specific build
directory name for the target board/release) and keep the destination
"/srv/netboot/tftp/"; update the example text so readers must substitute their
exact build directory (referencing the string "output/images/*-netboot-tftp/"
and the destination "/srv/netboot/tftp/").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7bdbb458-1561-4a19-9941-b926c8ce368d
📒 Files selected for processing (14)
config/templates/nfs-boot.cmd.templateextensions/netboot/README.mdextensions/netboot/files/dhcpcd-hooks/71-netboot-rootpathextensions/netboot/files/initramfs-hooks/netboot-rootpathextensions/netboot/files/initramfs-scripts/init-premount/zz-netboot-watchdogextensions/netboot/files/initramfs-scripts/nfs-bottom/zz-netboot-watchdog-cancelextensions/netboot/netboot-deploy.shextensions/netboot/netboot.shlib/functions/configuration/main-config.shlib/functions/host/host-utils.shlib/functions/image/partitioning.shlib/functions/image/rootfs-to-image.shlib/functions/rootfs/create-cache.shlib/functions/rootfs/rootfs-create.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/functions/rootfs/rootfs-create.sh
- config/templates/nfs-boot.cmd.template
- lib/functions/image/rootfs-to-image.sh
- extensions/netboot/netboot-deploy.sh
2 tasks
Make the network-boot u-boot template work for both arm64 (Image/booti) and armv7 (zImage/bootz). Load kernel/initrd/dtb from the local boot partition; mount root over NFS. Take console settings from DTB `/chosen/stdout-path` instead of hardcoded baud — boards like helios64 (1500000) and others (115200) work without per-board overrides. Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
Allow the rootfs stage to produce a compressed archive, an exported directory tree, or both. Compression is configurable. When `ROOTFS_COMPRESSION=none` is set without `ROOTFS_EXPORT_DIR`, fail fast — there would be no rootfs artifact otherwise. Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
Add opt-in netboot extension for diskless U-Boot PXE/NFS boot. Generates a TFTP tree (kernel + DTB + uInitrd + per-board pxelinux.cfg with extlinux APPEND for NFS root) alongside or instead of a flashable image. Supports per-host MAC-tagged configs, builder-as-NFS-server via ROOTFS_EXPORT_DIR, ROOTSERVER discovery from DHCP siaddr in initramfs, and a `netboot_artifacts_ready` post-hook for deploy automation. Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
New rootfs type for full network boot: the only thing on the device's local storage is U-Boot itself. Kernel, DTB, optional uInitrd and PXE config come from TFTP; rootfs is mounted over NFS. A new case branch in do_main_configuration auto-enables the netboot extension, symmetric with existing fs-f2fs-support / fs-btrfs wiring. The legacy ROOTFS_TYPE=nfs (hybrid: kernel on local storage, only / over NFS) is untouched — both paths coexist. - nfs-root case branch in ROOTFS_TYPE dispatch calls enable_extension "netboot" - prepare_partitions skips root partition creation and SD-size sanity check - check_filesystem_compatibility_on_host skipped for nfs-root - create_image_from_sdcard_rootfs early-returns for nfs-root after the pre_umount hook has grabbed TFTP artifacts from /boot: SDCARD.raw is dropped, the .img pipeline (mv to DESTIMG, write-to-SD, fingerprint, compress) is skipped. For nfs-root the only deliverables are the rootfs archive / ROOTFS_EXPORT_DIR tree and the TFTP staging dir — producing a boot-partition .img would be misleading (nothing on the device reads it). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
Documents the netboot extension: artifact server setup (tftpd-hpa + nfs-kernel-server), TFTP tree layout, DHCP options 66/67 on the network DHCP server, userpatches.conf knobs, the netboot_artifacts_ready hook, a full end-to-end helios64 walkthrough, and a troubleshooting section. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
In the rootfs cache and image pipelines, switch tar/rsync calls to:
rsync -aHWh -XAXS --numeric-ids ...
tar cp --xattrs --xattrs-include='security.*' --acls --selinux --sparse ...
Without this, xattrs (notably `security.capability` for setcap'd binaries
like /usr/bin/ping) and POSIX ACLs were stripped during the rootfs →
tarball → image (or → NFS export) round-trip.
The tar xattr include pattern defaults to `security.*` (file capabilities
and SELinux contexts) — broader patterns also pick up source-fs internals
(`bcachefs_*`, `btrfs.*`, `zfs.*` from the build host) that have no
meaning on the target and produce extract-time errors. Tunable via:
- `ROOTFS_TAR_XATTR_INCLUDE` (env, replaces default include pattern)
- `ROOTFS_TAR_EXTRA_FLAGS` (bash array, appended to tar args)
- `ROOTFS_RSYNC_XATTR_FLAGS` (env, replaces rsync xattr/ACL flags)
- `pre_create_rootfs_archive` extension hook (set the above lazily,
e.g. depending on ARCH/RELEASE)
Drive-by: scope the `post_create_rootfs_archive` hook dispatch to
`ROOTFS_TYPE=nfs|nfs-root` (the only types that produce an archive)
and add `zst` to the `expected:` list in the unknown-compression
error message.
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
Reference implementation of the `netboot_artifacts_ready` post-hook:
rsyncs the TFTP tree and unpacks the rootfs archive into an NFS export
on a remote server over SSH.
- Setting `NETBOOT_DEPLOY_SSH=user@host` implies `ROOTFS_TYPE=nfs-root`
so the operator does not have to repeat it on the command line.
- An early probe hook (`extension_prepare_config__060_…`) tries
`touch && rm` on the target TFTP root before the long build, so a
misconfigured SSH key / sudo / known_hosts fails fast with a clear
diagnostic instead of after `./compile.sh` has run for 40 minutes.
- Host identity is explicit: `known_hosts` file by default
(`NETBOOT_DEPLOY_SSH_KNOWN_HOSTS`), optional TOFU
(`NETBOOT_DEPLOY_SSH_TOFU=yes`) for first-time deployments.
- SSH key passed via `NETBOOT_DEPLOY_SSH_KEY`; `sudo` on the
target is opt-in via `NETBOOT_DEPLOY_SUDO=yes`.
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
Add troubleshooting entries for the most common netboot failure modes: 'NFS over TCP not available from <gateway>' (DHCP boot-server / siaddr unset or wrong) and the corresponding dnsmasq/OpenWRT 'dhcp-boot' fix. Show how to verify with /proc/net/pnp on a booted client. Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
… watchdog
Replace the `nfsroot=auto` (DHCP option 17) discovery path with
`nfsroot=<path>,tcp,v3` baked into the extlinux APPEND, and let the
kernel resolve the NFS server from DHCP siaddr at boot. Per-host rootfs
paths live in per-board pxelinux.cfg files; the server is a single
network-wide value. DHCP option 17 is no longer consulted in this mode
— it doesn't scale to multi-board setups (one path for all clients).
Add a 10-minute mount-stall safety net via two initramfs hooks:
- `init-premount/zz-netboot-watchdog` forks a background shell that
sleeps 600 s and then triggers an immediate reboot via
`echo b > /proc/sysrq-trigger` if the NFS root mount has not
completed by then.
- `nfs-bottom/zz-netboot-watchdog-cancel` kills that background
shell after a successful mount.
Active only for `ROOTFS_TYPE=nfs-root`. Without the watchdog, a
misconfigured server or transient network failure would hang the target
in initramfs forever; with it, the board self-recovers and tries again.
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
…uid_owner netboot extension exports an unpacked rootfs tree under output/netboot-export/<suffix>/ and reset_uid_owner runs over it. Debian/Ubuntu rootfs trees routinely contain dangling symlinks (e.g. /etc/systemd/system/multi-user.target.wants/ entries referring to services from packages that aren't installed). GNU chown without -h follows the symlink and fails on a missing target with 'cannot dereference', so post-docker cleanup returns exit 2 even though the build itself succeeded. chown -h sets the owner of the symlink inode itself rather than its target; it is a no-op for regular files and directories. This is the semantically correct choice when walking a filesystem tree, regardless of the netboot use case. Surfaced by armbian#9656 (netboot extension); applies to any extension that performs reset_uid_owner over a real rootfs tree on host. Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
The default NETBOOT_TFTP_PREFIX and (shared-mode) NETBOOT_NFS_PATH now
include a build-flavor suffix so CLI vs minimal vs desktop builds for
the same board/branch/release coexist side by side without colliding
on the staging tree, the deployed TFTP layout, or the NFS root.
• CLI: armbian/<family>/<board>/<branch>-<release>
• min: armbian/<family>/<board>/<branch>-<release>-min
• desktop: armbian/<family>/<board>/<branch>-<release>-desktop
Flavor is computed once in extension_prepare_config and stored as the
global ${_NETBOOT_FLAVOR}, so the post-build hook that composes pxe_tag
picks it up too — pxelinux.cfg/<board>-<branch>-<release><flavor>.example
becomes a unique tagged file per flavor, and the deploy/symlink workflow
keeps working without changes.
Per-host NFS path (NETBOOT_HOSTNAME set) is left untouched — host names
are already disambiguating.
User-supplied NETBOOT_TFTP_PREFIX / NETBOOT_NFS_PATH are honored verbatim,
flavor is only added to the defaults.
README updated to document the new <flavor> segment in both default
patterns.
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>
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
2 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.
Summary
Adds an opt-in
netbootextension that produces a full network-boot payload — kernel, DTB, optionaluInitrd,pxelinux.cfgand an NFS-exportable rootfs — from a regular Armbian build. For netboot the only thing that has to live on the device's local storage is U-Boot itself: the kernel and DTB come from TFTP,/is mounted over NFS, nothing else on mmc/eMMC/USB is touched during early boot.Today
ROOTFS_TYPE=nfsalone only gives a hybrid image (kernel+DTB still on SD/eMMC, only/over NFS). This PR keeps that path working and layers a real netboot flow on top via a newROOTFS_TYPE=nfs-root.What changes
lib/functions/configuration/main-config.sh— newROOTFS_TYPE=nfs-rootcase branch, symmetric with the existingfs-f2fs-support/fs-btrfswiring: the branch auto-enables thenetbootextension soROOTFS_TYPE=nfs-rootis the single switch that selects the full netboot flow.check_filesystem_compatibility_on_hostis skipped for bothnfsandnfs-root— the host-side check is a sanity net for block-device targets and falsely rejects valid NFS configurations.extensions/netboot/netboot.sh— new, directory-based extension. Directory layout (rather than a singleextensions/netboot.shfile) exists so the long-form usage guide can live next to the code asREADME.md; inlining documentation of that size into the script body would be unwieldy. Hooks:extension_prepare_config— validate variables, compute defaults forNETBOOT_TFTP_PREFIX/NETBOOT_NFS_PATH(shared byLINUXFAMILY/BOARD/BRANCH/RELEASE, or per-host whenNETBOOT_HOSTNAMEis set), normalizeNETBOOT_CLIENT_MACto the01-<mac>PXELINUX form, fail fast on badROOTFS_COMPRESSION/ROOTFS_EXPORT_DIRcombinations before debootstrap.custom_kernel_config— enableROOT_NFS,NFS_FS,NFS_V3,IP_PNP,IP_PNP_DHCPsoroot=/dev/nfs ip=dhcpworks without an initrd.post_customize_image— droparmbian-resize-filesystem.service(meaningless on NFS root) and/root/.not_logged_in_yet(thearmbian-firstlogininteractive wizard blocks bring-up when there is no interactive console).armbian-firstrun.servicestays — it only regenerates SSH host keys.host_pre_docker_launch— bind-mountROOTFS_EXPORT_DIRinto the build container when it lives outside${SRC}, so the single-step builder-as-NFS-server workflow works the same inside and outside Docker.pre_umount_final_image— assemble the TFTP tree (Image/zImage,dtb/,uInitrd), writepxelinux.cfg/{default.example | 01-<mac>}with the rightFDT/FDTDIRline and an explicitINITRDdirective whenuInitrdis present (U-Boot's PXE parser only loads an initramfs when the stanza names it), expose anetboot_artifacts_readyhook for userpatches that deploy to a real server.lib/functions/image/rootfs-to-image.sh— two new controls for NFS-rootfs builds (bothROOTFS_TYPE=nfsandnfs-root):ROOTFS_COMPRESSION=zstd|gzip|none(defaultzstd).zstd→.tar.zst,gzip→.tar.gz,none→ no archive at all. Rejected early ifnoneis set withoutROOTFS_EXPORT_DIR. Thetar | pv | compressorpipeline now runs in aset -o pipefailsubshell so a broken archive step actually fails the build instead of producing a silently truncated file.ROOTFS_EXPORT_DIR=/abs/path— also (or only) rsync the rootfs tree into this directory. Can point anywhere on the build host's filesystem — inside or outside the Armbian tree (the extension bind-mounts external paths into the Docker container automatically). If the path is an NFS mount or any other network filesystem, the build writes directly to the remote server with no intermediate archive. The rsync uses--deleteso a re-used export dir stays in sync with the new build instead of accumulating stale files from previous runs. Combined withROOTFS_COMPRESSION=nonethis turns deploy-to-NFS-server into a single build step.ROOTFS_ARCHIVE_PATHis exported so downstream hooks (the newnetboot_artifacts_ready) can reference the produced archive.lib/functions/image/partitioning.sh—prepare_partitionsskips the root partition entry fornfs-rootthe same way it does fornfs, and the SD-size sanity check no longer applies to either (there is no image file to size).config/templates/nfs-boot.cmd.template— previously sunxi/arm32-only (zImage+bootz). Made arch-agnostic so the hybridROOTFS_TYPE=nfspath builds for arm64 boards too.extensions/netboot/README.md— long-form guide: variable reference, server setup (tftpd-hpa+nfs-kernel-server), DHCP 66/67 config (OpenWRT + others), per-host / per-MAC deployments, firstrun vs firstlogin, end-to-end helios64 example, troubleshooting.Variables (quick reference)
All optional.
ROOTFS_TYPE=nfs-rootalone gives you a shared-rootfs build with apxelinux.cfg/default.examplefile.NETBOOT_SERVERnfsroot=keeps${serverip}literal for U-Boot to fill from DHCP siaddr.NETBOOT_TFTP_PREFIXarmbian/${LINUXFAMILY}/${BOARD}/${BRANCH}-${RELEASE}NETBOOT_NFS_PATH/srv/netboot/rootfs/shared/...or/srv/netboot/rootfs/hosts/<hostname>nfsroot=.NETBOOT_HOSTNAMEhosts/<hostname>/— each machine owns its own writable copy.NETBOOT_CLIENT_MAC01-<mac>(per-MAC PXELINUX override). Accepts:or-, normalized to lowercase.ROOTFS_COMPRESSIONzstdzstd/gzip/none.ROOTFS_EXPORT_DIRTest plan
Validated end-to-end on helios64 (
rockchip64,edge/resolute,ttyS2 @ 1500000), Armbian 26.05:ROOTFS_TYPE=nfs-root NETBOOT_SERVER=192.168.1.125, shared NFS path, FDTDIR fallback for boards withoutBOOT_FDT_FILE, no hardcodedconsole=). 6:57 min, all artifacts correct.NETBOOT_HOSTNAME=helios64-a NETBOOT_CLIENT_MAC=aa:bb:cc:dd:ee:ff).pxelinux.cfg/01-aa-bb-cc-dd-ee-ffcontainsnfsroot=.../hosts/helios64-a. 7:44 min.ROOTFS_COMPRESSION=gzip— produces.tar.gz(defaultzstd→.tar.zst). On the helios64 rootfs: 503 MB gzip vs 460 MB zstd.ROOTFS_COMPRESSION=none ROOTFS_EXPORT_DIR=...— single-step tree workflow, 1.5 GB tree in export dir, no archive produced. 5:17 min.ROOTFS_COMPRESSION=nonewithoutROOTFS_EXPORT_DIR— fails fast inextension_prepare_configbefore debootstrap, not hours later.tftpd-hpa+ NFS rootfs via rsync tonfs-kernel-server. DHCP options 66/67 on OpenWRT. Helios64 cold boot → U-Bootbootflow scan -lb→ TFTP →booti→ kernel →ip_auto_config→ NFS mount → systemdgraphical.target→ login prompt onttyS2@1500000. Noarmbian-firstloginwizard, noresize2fserrors,armbian-firstrun.serviceregenerates SSH host keys normally. Verified twice: once with archive workflow + manual unpack, once with tree workflow + rsync deploy.mvebu,armhf, U-Boot v2025.10) — second SoC family. PXE netboot: DHCP → TFTP (zImage + DTB) →bootz→ kernel → NFS root → systemd → login prompt. No initrd needed (see notes below).BOOT_FDT_FILEset (the FDTDIR vs FDT code path is exercised by helios64's missingBOOT_FDT_FILE, but a board with it set should also work).Known issues found during helios4 testing (not blockers for this PR):
ramdisk_addr_r(0x2880000) is only 8 MB abovekernel_addr_r(0x2080000), but zImage 6.12 is 8.1 MB → U-Boot refuses to boot with initrd. Workaround: omitINITRDfrom PXE config — kernel withCONFIG_ROOT_NFS=ymounts NFS root directly without initrd.ip=dhcpwrites DNS to/proc/net/pnpbutsystemd-resolveddoesn't read it → "No DNS servers known". Workaround:resolvectl dns <iface> <gateway>. Needs a networkd.networkfile or oneshot service inpost_customize_image(future improvement).Post-review update: architecture reworked after CodeRabbit #1 — the extension no longer forces
ROOTFS_TYPEfrom insideextension_prepare_config(too late in the config lifecycle); instead a newROOTFS_TYPE=nfs-rootcase branch inmain-config.shenables the extension, symmetric with existing filesystem-support extensions.pipefailon the archive pipeline (#4),rsync --deleteon the export dir (#5), and an explicitINITRDdirective whenuInitrdis present (#2) are all in.Post-review update (round 3): archive creation moved from the early
if/elseblock to afterpre_umount_final_imagehooks so the tarball/export tree captures the fully finalized rootfs (includesupdate_initramfsoutput and all hook edits). Split out as a separate pre-feature commit — also fixes the same bug in the originalROOTFS_TYPE=nfshybrid flow. Additionally: the netboot extension now setsBOOTSIZE=0fornfs-root, preventing the phantom/bootfstab entry at its source; the compensatingpre_umount_final_image__100_netboot_clean_fstabhook is gone.Post-review update (round 4):
config/templates/nfs-boot.cmd.templatenow wraps the DTB load in anif load ... ; then true; else echo FATAL; reset; fiblock so a missing/wrongdtb/${fdtfile}aborts the boot with a clear message instead of continuing with an invalid${fdt_addr_r}— consistent with the ramdisk and kernel handling on the following lines.Post-review update (round 5): also includes a small core-fix in
lib/functions/host/host-utils.sh—reset_uid_ownernow useschown -hto tolerate dangling symlinks in the unpacked rootfs tree this extension exports (see the commit for the full rationale).Related work
A companion PR to armbian/documentation will add
Developer-Guide_Netboot.mdwith the short overview + variable reference; this extension'sREADME.mdis the long-form guide and is linked from that page.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Behavior