partitioning: Add BTRFS_ROOT_SUBVOLUME switch #8144
Conversation
This allows changing the default root subvolume from @ to any user defined subvolume name
This allows changes to the default subvolume in `btrfs_root_add_subvolumes` to propagate as needed into the final image.
This simplifies the code, removing the need to grep/cut the output of btrfs subvolume list
WalkthroughThis change updates the logic for handling btrfs root subvolumes in the Possibly related PRs
Suggested labels
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
05
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/functions/image/partitioning.sh (1)
355-355: Optional: Quote variables and use explicit-osyntax in remount.The remount line drops the explicit
subvol=@, which is great, but for improved robustness:-run_host_command_logged mount -odefaults,${mountopts[$ROOTFS_TYPE]} $rootdevice $MOUNT/ +run_host_command_logged mount -o "defaults,${mountopts[$ROOTFS_TYPE]}" "$rootdevice" "$MOUNT/"This change avoids word-splitting issues and makes the
-ousage clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/functions/image/partitioning.sh(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lib/functions/image/partitioning.sh (1)
lib/functions/logging/runners.sh (1)
run_host_command_logged(192-194)
🔇 Additional comments (1)
lib/functions/image/partitioning.sh (1)
333-339:❓ Verification inconclusive
Parameterize and robustify BTRFS root subvolume creation and default setting.
You’ve centralized the subvolume name via
btrfs_root_subvolume="${BTRFS_ROOT_SUBVOLUME:-@}"and simplified default‐setting withbtrfs subvolume set-default. To support nested subvolumes (e.g.,"a/b") and avoid path or whitespace issues, please:
- Ensure parent directories are created before
btrfs subvolume create, e.g.:+run_host_command_logged mkdir -p "$(dirname "$MOUNT/$btrfs_root_subvolume")" -run_host_command_logged btrfs subvolume create $MOUNT/$btrfs_root_subvolume +run_host_command_logged btrfs subvolume create "$MOUNT/$btrfs_root_subvolume"- Quote all variable expansions for safety, including in
set-default:-run_host_command_logged "btrfs subvolume set-default $MOUNT/$btrfs_root_subvolume" +run_host_command_logged btrfs subvolume set-default "$MOUNT/$btrfs_root_subvolume"- Verify that the installed
btrfs-progsversion on target systems supports passing a path directly toset-default(older versions expect a numeric ID).Would you like me to help with these adjustments?
To be sure we address all quoting and directory‐creation issues, let’s first pinpoint how these commands are used in the script:
🏁 Script executed:
#!/bin/bash set -e echo "## btrfs subvolume create lines" rg -n "btrfs subvolume create" lib/functions/image/partitioning.sh echo "## btrfs subvolume set-default lines" rg -n "btrfs subvolume set-default" lib/functions/image/partitioning.sh echo "## mkdir calls" rg -n "mkdir" lib/functions/image/partitioning.shLength of output: 1260
Parameterize and Robustify BTRFS Subvolume Creation (lib/functions/image/partitioning.sh)
Please update the BTRFS subvolume creation and default‐setting to handle nested names and whitespace safely, and ensure parent directories exist:
- Quote all path expansions in both
createandset-defaultcalls.- Pre-create the parent directory of each subvolume before calling
btrfs subvolume create.- Apply the same quoting to the other hardcoded subvolume creates at lines 346–350.
- Verify that your target systems’
btrfs-progsversion supports passing a path tobtrfs subvolume set-default(older versions may require a numeric subvolume ID).Suggested diff:
--- a/lib/functions/image/partitioning.sh +++ b/lib/functions/image/partitioning.sh @@ -333,7 +333,9 @@ btrfs_root_subvolume="${BTRFS_ROOT_SUBVOLUME:-@}" mountopts[$ROOTFS_TYPE]='commit=120' - run_host_command_logged btrfs subvolume create $MOUNT/$btrfs_root_subvolume + run_host_command_logged mkdir -p "$(dirname "$MOUNT/$btrfs_root_subvolume")" + run_host_command_logged btrfs subvolume create "$MOUNT/$btrfs_root_subvolume" + # getting the subvolume id of the newly created volume @ to install it # as the default volume for mounting without explicit reference @@ -339,7 +341,7 @@ - run_host_command_logged "btrfs subvolume set-default $MOUNT/$btrfs_root_subvolume" + run_host_command_logged btrfs subvolume set-default "$MOUNT/$btrfs_root_subvolume"Applicable locations:
• lib/functions/image/partitioning.sh: lines 335 (root subvol), 346–350 (other subvol creates), 339 (set-default)
Docstrings generation was requested by @iav. * #8144 (comment) The following files were modified: * `lib/functions/image/partitioning.sh`
|
Note Generated docstrings for this pull request at #8145 |
iav
left a comment
iav
left a comment
There was a problem hiding this comment.
I try to build jammy, noble, plucky.
It works.
|
Please add (copy/paste + AI) to https://docs.armbian.com/Developer-Guide_Build-Switches/ |
3 participants
Description
This allows changing the default root subvolume from @ to any user
defined subvolume name
We currently ship an image using @root/[revision], which allows us to ship offline updates using btrfs send/receive. It would be easy enough to just add our root subvolume in
btrfs_root_add_subvolumes, however the subsequent "Remounting rootfs"mountcall specifies the subvolume by name instead of relying on the previously set default. To work around this for our needs, I would have to abusebtrfs_root_add_subvolumes_fstabtoumount/mountto change the volume name there as well.To this end, I've changed the remounting rootfs call to not explicitly pass a subvolume, mounting the current default. This allows users to customize subvolumes further in
btrfs_root_add_subvolumes(for example, creating a/b rootfs subvolumes).Documentation summary for feature / change
How Has This Been Tested?
A test extension was used to create a nested custom root subvolumes
Then I mounted the output images and verified the changed default was used during remounting and image build
bash compile.sh BOARD=uefi-x86 BUILD_MINIMAL=yes RELEASE=trixie BRANCH=current KERNEL_CONFIGURE=no ROOTFS_TYPE=btrfs BTRFS_ROOT_SUBVOLUME=@root ENABLE_EXTENSIONS=test-btrfs-subvolChecklist: