Skip to content

partitioning: fix malformed fstab double comma and switch to relatime #9227

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.

Sign up for GitHub

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

Open
NickAlilovic wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −1
Open

partitioning: fix malformed fstab double comma and switch to relatime #9227

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/functions/image/partitioning.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ function prepare_partitions() {
run_host_command_logged mount -odefaults,${mountopts[$ROOTFS_TYPE]} ${fscreateopt} $rootdevice $MOUNT/
fi
rootfs="UUID=$(blkid -s UUID -o value $rootdevice)"
echo "$rootfs / ${mkfs[$ROOTFS_TYPE]} defaults,${mountopts[$ROOTFS_TYPE]} 0 1" >> $SDCARD/etc/fstab
echo "$rootfs / ${mkfs[$ROOTFS_TYPE]} defaults,relatime${mountopts[$ROOTFS_TYPE]} 0 1" >> $SDCARD/etc/fstab
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the actual file to understand the context
cd /tmp && git ls-files | grep -E "lib/functions/image/partitioning.sh"

Repository: armbian/build

Length of output: 125

🏁 Script executed:

# Get the file and examine the critical lines
cat -n lib/functions/image/partitioning.sh | sed -n '320,400p'

Repository: armbian/build

Length of output: 5076

🏁 Script executed:

# Check the git history to see what changed and the original code
git log --oneline -p lib/functions/image/partitioning.sh | head -200

Repository: armbian/build

Length of output: 8518

🏁 Script executed:

# Look for the double-comma issue mentioned in the review
git diff HEAD~1 lib/functions/image/partitioning.sh || git show HEAD:lib/functions/image/partitioning.sh | cat -n | sed -n '340,390p'

Repository: armbian/build

Length of output: 3766

🏁 Script executed:

# Look for other versions or branches of this file
find . -name "partitioning.sh" -type f

Repository: armbian/build

Length of output: 94

🏁 Script executed:

# Check if this is a brand new file or if it's been modified
# by examining the full directory structure
ls -la lib/functions/image/ 2>/dev/null | head -20

Repository: armbian/build

Length of output: 634

🏁 Script executed:

# Let's trace the mountopts variable for btrfs throughout the code
echo "=== Initial mountopts declarations ==="
sed -n '80,90p' lib/functions/image/partitioning.sh

echo -e "\n=== Btrfs block that overwrites mountopts ==="
sed -n '343,367p' lib/functions/image/partitioning.sh

echo -e "\n=== Line 369 that uses mountopts ==="
sed -n '369p' lib/functions/image/partitioning.sh

echo -e "\n=== Subvolume fstab entries that use mountopts ==="
sed -n '370,388p' lib/functions/image/partitioning.sh

Repository: armbian/build

Length of output: 3671

🏁 Script executed:

# Let's create a test to trace what the actual fstab output would be
python3 << 'PYTESTEOF'
import re

# Simulate the btrfs code path
mountopts = {}

# Line 85
mountopts['btrfs'] = ',commit=120'
print(f"Line 85: mountopts[btrfs] = '{mountopts['btrfs']}'")

# Line 345 - for btrfs case
ROOTFS_TYPE = 'btrfs'
mountopts[ROOTFS_TYPE] = 'commit=120'
print(f"Line 345 (btrfs case): mountopts[ROOTFS_TYPE] = '{mountopts[ROOTFS_TYPE]}'")

# Line 369 - fstab entry
rootfs = "UUID=12345678-1234-1234-1234-123456789abc"
mkfs = {'btrfs': 'btrfs'}
fstab_entry = f"{rootfs} / {mkfs[ROOTFS_TYPE]} defaults,relatime{mountopts[ROOTFS_TYPE]} 0 1"
print(f"\nLine 369 output for btrfs:")
print(f"  {fstab_entry}")

# Check if mount options are valid (must have comma-separated values)
opts = fstab_entry.split()[3]  # Get the mount options field
print(f"\nMount options field: '{opts}'")
if ',relatimecommit' in opts:
    print("  ERROR: Found 'relatimecommit' - MISSING COMMA between relatime and commit!")
    print("  This is MALFORMED and will likely cause mount failures")
else:
    print("  OK: Mount options are properly formatted")

print("\n" + "="*60)
print("Comparison with ext4:")
print("="*60)

# Simulate ext4 path
mountopts['ext4'] = ',commit=120,errors=remount-ro'
ROOTFS_TYPE = 'ext4'
# ext4 does NOT have the overwrite at line 345
print(f"\nmountopts[ext4] = '{mountopts['ext4']}'")

fstab_entry_ext4 = f"{rootfs} / {mkfs.get('ext4', 'ext4')} defaults,relatime{mountopts['ext4']} 0 1"
print(f"\nLine 369 output for ext4:")
print(f"  {fstab_entry_ext4}")
opts_ext4 = fstab_entry_ext4.split()[3]
print(f"\nMount options field: '{opts_ext4}'")
if ',,' in opts_ext4:
    print("  ERROR: Found double comma ',,' - MALFORMED")
else:
    print("  OK: Mount options are properly formatted")

PYTESTEOF

Repository: armbian/build

Length of output: 143

🏁 Script executed:

# Let's verify the mount command on line 366 (inside btrfs block)
echo "=== Line 366 mount command (inside btrfs block) ==="
sed -n '366p' lib/functions/image/partitioning.sh
echo ""
echo "Expected: mount -odefaults,commit=120 ... (no comma before commit, comma provided before it)"
echo ""

# And the subvolume mounts at line 373 onwards (inside extension method)
echo "=== Lines 373, 376, 379, 382, 385 (subvolume mounts) ==="
sed -n '373p; 376p; 379p; 382p; 385p' lib/functions/image/partitioning.sh
echo ""
echo "Expected: all use -odefaults,commit=120,subvol=... (comma provided AFTER mountopts)"

Repository: armbian/build

Length of output: 1008

Critical: btrfs filesystem produces malformed fstab entry (missing comma between relatime and commit).

At line 345, the btrfs block overwrites mountopts[$ROOTFS_TYPE]='commit=120' without a leading comma. This is correct for lines 366 and 373+ (which provide the comma after), but line 369 concatenates directly: defaults,relatime${mountopts[$ROOTFS_TYPE]} produces defaults,relatimecommit=120.

Result:

  • ext4: defaults,relatime,commit=120,errors=remount-ro ✓
  • btrfs: defaults,relatimecommit=120 ✗ (malformed, missing comma)

Impact: btrfs systems will fail to boot or mount due to invalid fstab syntax.

Recommended fix

Add a leading comma to the btrfs mountopts at line 345, then update lines 366, 373, 376, 379, 382, 385 to accommodate:

 mountopts[$ROOTFS_TYPE]=',commit=120'
-run_host_command_logged mount -odefaults,${mountopts[$ROOTFS_TYPE]} ${fscreateopt}
+run_host_command_logged mount -odefaults${mountopts[$ROOTFS_TYPE]} ${fscreateopt}

Apply similar changes to all subsequent mount commands and fstab entries in the btrfs subvolume section.

🤖 Prompt for AI Agents 
In @lib/functions/image/partitioning.sh at line 369, The btrfs mount options
lack a leading comma so concatenation in fstab entries produces invalid tokens
(e.g., defaults,relatimecommit=120); fix by ensuring mountopts[$ROOTFS_TYPE] for
btrfs is set with a leading comma (e.g., ",commit=120") and update all places
that append mountopts[$ROOTFS_TYPE] (the echo lines that write fstab and any
mount commands creating subvolume fstab entries) so they rely on the leading
comma instead of inserting one before the variable; specifically adjust the
assignment that sets mountopts for btrfs and verify every use site (the echo to
$SDCARD/etc/fstab and the btrfs subvolume mount/elements) produce
"defaults,relatime,commit=120" rather than "defaults,relatimecommit=120".

if [[ $ROOTFS_TYPE == btrfs ]]; then
call_extension_method "btrfs_root_add_subvolumes_fstab" <<- 'BTRFS_ROOT_ADD_SUBVOLUMES_FSTAB'
run_host_command_logged mkdir -p $MOUNT/home
Expand Down