-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
If rootfs placed on btrfs filesystem, place root on @ subvolume #8067
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
It makes possible to work with snapshots of a root filesystem
WalkthroughThe changes update the Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ 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 (
|
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)
lib/functions/image/partitioning.sh (2)
317-342: Btrfs Subvolume Creation and Root RemountingThis new block introduces btrfs‐specific logic by:
• Setting the mount option for btrfs to"commit=120"(note that this now omits the leading comma compared to its earlier default, so please ensure consistency across mount option concatenations).
• Creating the root subvolume “@” withbtrfs subvolume create $MOUNT/@.
• Retrieving the subvolume ID using a pipeline that includesgrep 'path @'andcutbefore setting it as the default withbtrfs subvolume set-default.Please verify that the filtering with
grep 'path @'is robust across different btrfs versions and that only the intended subvolume is matched. In addition, consider whether additional error handling or logging should be added in case the subvolume creation or default setting fails.
345-363: Btrfs fstab Entries for Additional SubvolumesThis section creates and mounts additional btrfs subvolumes (namely for
/home,/var,/var/log,/var/cache, and/srv) and appends corresponding entries to fstab. The use of thecall_extension_methodhook to encapsulate these operations is a good approach for modularity. Ensure that:
• Directory creation is idempotent so that repeated runs do not result in errors.
• The fstab entries are unique and nonconflicting if the image is rebuilt or reconfigured.
• Appropriate logging or error checking is in place for these mount operations.A brief review in various environments may help verify that these extra mounts integrate well with the overall mounting scheme.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/functions/image/partitioning.sh(2 hunks)
🔇 Additional comments (1)
lib/functions/image/partitioning.sh (1)
247-247: Minor Formatting/Whitespace ChangeA new line appears to be added right after the partitioning command output. This seems intended to improve readability or visually separate stages in the script. Verify that this extra newline was intentional and does not affect downstream processing.
Good health! Maybe it's better to implement this feature in armbian-install? |
igorpecovnik
left a comment
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.
LGTM.
I also agree that it would be nice to add this also to armbian-install.
But there is ongoing RFC of it. I did some front-end changed armbian/configng#384 and @adeepn was also looking into integrating (fresh modular design) armbian-install into armbian-config.
|
I'll try to add this to armbian-install. |
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
snappercan be installed, and do auto-snapshots on timer and apt operations.This PR: