-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Install to nvme: place root volume to separate subvolume on btrfs #8108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change updates the Armbian installation script to modify how Btrfs filesystems are handled during installation. The script now creates a dedicated Btrfs subvolume named Possibly related PRs
Suggested labels
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bsp/common/usr/bin/armbian-install (2)
131-131: Consider adding a comment about compression settingsThe mount option
compress-force=zlibis used when remounting, which could benefit from a brief comment explaining the choice of compression algorithm.- mount -o compress-force=zlib,subvol=@ "$2" "${TempDir}"/rootfs 2> /dev/null + # Mount with zlib compression for better compatibility and reasonable compression ratio + mount -o compress-force=zlib,subvol=@ "$2" "${TempDir}"/rootfs 2> /dev/null
124-132: Suggestion: Add error handling for Btrfs operationsThe Btrfs subvolume creation and management operations don't include error handling. Consider adding checks to ensure each operation succeeds before proceeding.
# make separate subvolume for rootfs on btrfs if [[ $eMMCFilesystemChoosen =~ ^(btrfs)$ ]]; then - btrfs subvolume create "${TempDir}"/rootfs/@ + if ! btrfs subvolume create "${TempDir}"/rootfs/@ > /dev/null 2>&1; then + echo "Error: Failed to create Btrfs subvolume" >> $logfile + exit 15 + fi sync - btrfs subvolume list ${TempDir}/rootfs/ | grep 'path @$' | cut -d' ' -f2 \ - | xargs -I{} btrfs subvolume set-default {} ${TempDir}/rootfs/ + SUBVOL_ID=$(btrfs subvolume list ${TempDir}/rootfs/ | grep 'path @$' | cut -d' ' -f2) + if [ -z "$SUBVOL_ID" ]; then + echo "Error: Failed to find created subvolume" >> $logfile + exit 15 + fi + btrfs subvolume set-default $SUBVOL_ID ${TempDir}/rootfs/ umount "${TempDir}"/rootfs - mount -o compress-force=zlib,subvol=@ "$2" "${TempDir}"/rootfs 2> /dev/null + if ! mount -o compress-force=zlib,subvol=@ "$2" "${TempDir}"/rootfs 2> /dev/null; then + echo "Error: Failed to mount Btrfs subvolume" >> $logfile + exit 15 + fi fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bsp/common/usr/bin/armbian-install(2 hunks)
🔇 Additional comments (3)
packages/bsp/common/usr/bin/armbian-install (3)
99-99: Updated Btrfs mount options to include subvolume supportThe mount options for Btrfs have been modified to include
subvol=@and replacednoatimewith better defaults. This change supports the new subvolume structure for the root filesystem.
124-132: Enhanced Btrfs support with dedicated root subvolumeThe added code correctly implements the feature to place the root filesystem on a dedicated Btrfs subvolume named
@. This is a good practice for Btrfs filesystems as it enables proper snapshot management and follows the standard layout used by tools like Snapper.The implementation follows these steps:
- Creates a new subvolume named
@within the root filesystem- Syncs the filesystem to ensure persistence
- Sets the newly created subvolume as the default
- Remounts the filesystem with the appropriate subvolume option
124-132:✅ Verification successful
Verify Btrfs subvolume naming convention consistency
The implementation uses
@as the subvolume name, which is common in some distros (like Ubuntu), but ensure this is consistent with Armbian's conventions and documented appropriately.
🏁 Script executed:
#!/bin/bash # Check for any existing Btrfs subvolume naming patterns in the codebase grep -r "btrfs subvolume create" --include="*.sh" . grep -r "subvol=" --include="*.sh" .Length of output: 2610
Btrfs subvolume naming convention confirmed
Verified that Armbian’s existing scripts (e.g.lib/functions/image/partitioning.sh) already use@,@home,@varetc., so the use of@here matches the established convention. No changes required.
|
Perfect, thanks. Perhaps adding few words on this here: |
|
|
Realisation #8067 during installation from sdcard to nvme internal storage.
It makes possible to work with snapshots of a root filesystem on btrfs.
Using snapshots of a rootfs on btrfs needs to place root on a btrfs subvolume, not on root of btrfs volume itself.
Sometimes some of directories have to be placed on separate subvolumes.
After this change snapper can be installed, and do auto-snapshots on timer and apt operations.