LowMem extension: Fix swapfile creation on btrfs; Fix permission race - #10298
LowMem extension: Fix swapfile creation on btrfs; Fix permission race#10298dlitz wants to merge 2 commits into
Conversation
Signed-off-by: Darsey Litzenberger <dlitz@dlitz.net>
📝 WalkthroughWalkthroughThe swapfile setup script now sets a restrictive umask, truncates the target, and attempts to disable copy-on-write before allocation and activation. ChangesSwapfile initialization hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/bsp/armbian-lowmem/lowmem-mkswap.sh`:
- Around line 29-35: Update the swapfile creation flow around SWAPFILE_PATH so
failures after truncate, including chattr, fallocate, mkswap, or swapon, remove
the incomplete swapfile before exiting. Use transactional staging or equivalent
validation/repair for existing incomplete files, while preserving valid existing
swapfile handling.
- Around line 34-35: The swapfile setup currently ignores failures from chattr
in the low-memory swap creation flow. Update the chattr +C step after
REAL_SWAPFILE_PATH is resolved to stop immediately with an actionable error when
it fails, before preallocation, mkswap, or swapon proceed; retain normal
continuation when No_COW setup succeeds.
- Around line 32-35: Update the swapfile setup flow to use REAL_SWAPFILE_PATH,
the canonical path resolved by readlink, for free-space checks, df/dirname
lookups, and swapfile allocation operations; retain SWAPFILE_PATH only as the
configurable input and ensure all filesystem calculations target the resolved
path.
🪄 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 Plus
Run ID: 7f9feebd-0b10-4195-9ab1-9e9715e4f913
📒 Files selected for processing (1)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh
This ensures the swapfile is created with the No_COW attribute.
Otherwise, swapon fails with "Invalid argument" and the kernel logs a
warning:
BTRFS warning (device mmcblk1p1): swapfile must not be copy-on-write
Signed-off-by: Darsey Litzenberger <dlitz@dlitz.net>
0aecfa5 to
61a7dd0
Compare
|
✅ This PR has been reviewed and approved — all set for merge! |
So after merge global rootfs recreation seems necessary |
Description
This PR includes a fix for the creation of the
/swapfilewhen building withENABLE_EXTENSIONS=lowmemand a btrfs root filesystem. Specifically, it ensures that the swapfile is created with the No_COW attribute (chattr +C). Otherwise, swapon fails.This PR also includes a one-line fix (as a separate commit) to set the umask so that there is no moment where
/swapfileis world-readable.How Has This Been Tested?
BOARD=qemu-uefi-x86without this patchBOARD=qemu-uefi-x86with this patchBOARD=nanopi-r3s-ltswithout this patchBOARD=nanopi-r3s-ltswith this patchCommon build options:
./compile.sh RELEASE=trixie BRANCH=edge BUILD_MINIMAL=yes KERNEL_CONFIGURE=no ROOTFS_TYPE=btrfs BTRFS_COMPRESSION=zstd ENABLE_EXTENSIONS=lowmem CLEAN_LEVEL=debsNotes:
CLEAN_LEVEL=debsseems to be needed to prevent the build process from including an old, cached copy of thelowmem-mkswap.shscript.BTRFS_COMPRESSION=zstdorBTRFS_COMPRESSION=noneseems to be needed onnanopi-r3s-ltsbecause u-boot apparently can't properly read /boot/armbianEnv.txt from a zlib-compressed btrfs.Checklist:
Summary by CodeRabbit
umask 077).