From 73e66b703258270a3d688a51dd90ed2f24401568 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Tue, 23 Jun 2026 21:05:17 -0400 Subject: refactor(installer): extract parse_btrfs_subvol_opts helper mount_btrfs_subvolumes and generate_btrfs_fstab each carried an identical block that composed a subvolume's mount options from BTRFS_OPTS plus the per-subvol extra flags. The two could drift out of sync. Extracted the logic into parse_btrfs_subvol_opts (pure string transform), preserving the exact behavior, and called it from both. Added bats cases covering the default, compress=no, nodatacow, nosuid, and combined paths. --- installer/lib/btrfs.sh | 69 +++++++++++++++++++++++----------------------- tests/unit/test_btrfs.bats | 35 +++++++++++++++++++++++ 2 files changed, 69 insertions(+), 35 deletions(-) diff --git a/installer/lib/btrfs.sh b/installer/lib/btrfs.sh index 0a34be0..67c96a0 100644 --- a/installer/lib/btrfs.sh +++ b/installer/lib/btrfs.sh @@ -340,6 +340,36 @@ create_btrfs_subvolumes() { # Btrfs Mount Functions ############################# +# Compose the mount-option string for a single subvolume: the shared +# BTRFS_OPTS prefixed with subvol=, then the per-subvol extra +# flags applied. compress=no and nodatacow both drop the default +# compress=zstd; nodatacow also appends nodatacow; nosuid appends +# nosuid,nodev. Pure string transform — no I/O. Shared by +# mount_btrfs_subvolumes and generate_btrfs_fstab so the two stay in sync. +# Usage: parse_btrfs_subvol_opts NAME EXTRA +parse_btrfs_subvol_opts() { + local name="$1" extra="$2" + local opts="subvol=$name,$BTRFS_OPTS" + + if [[ -n "$extra" ]]; then + # compress=no: drop the default compression, don't add anything + if [[ "$extra" == *"compress=no"* ]]; then + opts=$(echo "$opts" | sed 's/,compress=zstd//') + fi + # nodatacow implies no compression (incompatible), so drop it too + if [[ "$extra" == *"nodatacow"* ]]; then + opts="$opts,nodatacow" + opts=$(echo "$opts" | sed 's/,compress=zstd//') + fi + # nosuid,nodev hardening for tmp subvolumes + if [[ "$extra" == *"nosuid"* ]]; then + opts="$opts,nosuid,nodev" + fi + fi + + echo "$opts" +} + mount_btrfs_subvolumes() { local partition="$1" @@ -356,25 +386,8 @@ mount_btrfs_subvolumes() { # Skip root, already mounted [[ "$name" == "@" ]] && continue - # Build mount options - local opts="subvol=$name,$BTRFS_OPTS" - - # Apply extra options (override defaults where specified) - if [[ -n "$extra" ]]; then - # Handle compress=no by removing compress from opts and not adding it - if [[ "$extra" == *"compress=no"* ]]; then - opts=$(echo "$opts" | sed 's/,compress=zstd//') - fi - # Handle nodatacow - if [[ "$extra" == *"nodatacow"* ]]; then - opts="$opts,nodatacow" - opts=$(echo "$opts" | sed 's/,compress=zstd//') - fi - # Handle nosuid,nodev for tmp - if [[ "$extra" == *"nosuid"* ]]; then - opts="$opts,nosuid,nodev" - fi - fi + local opts + opts=$(parse_btrfs_subvol_opts "$name" "$extra") info "Mounting $name -> $MNTPOINT$mountpoint" mkdir -p "$MNTPOINT$mountpoint" @@ -412,22 +425,8 @@ EOF for subvol_spec in "${BTRFS_SUBVOLS[@]}"; do IFS=':' read -r name mountpoint extra <<< "$subvol_spec" - # Build mount options - local opts="subvol=$name,$BTRFS_OPTS" - - # Apply extra options - if [[ -n "$extra" ]]; then - if [[ "$extra" == *"compress=no"* ]]; then - opts=$(echo "$opts" | sed 's/,compress=zstd//') - fi - if [[ "$extra" == *"nodatacow"* ]]; then - opts="$opts,nodatacow" - opts=$(echo "$opts" | sed 's/,compress=zstd//') - fi - if [[ "$extra" == *"nosuid"* ]]; then - opts="$opts,nosuid,nodev" - fi - fi + local opts + opts=$(parse_btrfs_subvol_opts "$name" "$extra") echo "UUID=$uuid $mountpoint btrfs $opts 0 0" >> $MNTPOINT/etc/fstab done diff --git a/tests/unit/test_btrfs.bats b/tests/unit/test_btrfs.bats index 890bba2..15bf141 100644 --- a/tests/unit/test_btrfs.bats +++ b/tests/unit/test_btrfs.bats @@ -54,3 +54,38 @@ setup() { [ "$status" -eq 0 ] [ -z "$output" ] } + +############################# +# parse_btrfs_subvol_opts +############################# +# Composes the mount-option string for one subvolume from the shared +# BTRFS_OPTS plus the per-subvol extra flags. Pure string transform, +# shared by mount_btrfs_subvolumes and generate_btrfs_fstab. BTRFS_OPTS +# is set at the top of btrfs.sh (sourced in setup), so these pin behavior +# against the real default option string. + +@test "parse_btrfs_subvol_opts: no extra flags keeps the default opts" { + run parse_btrfs_subvol_opts "@home" "" + [ "$status" -eq 0 ] + [ "$output" = "subvol=@home,noatime,compress=zstd,space_cache=v2,discard=async" ] +} + +@test "parse_btrfs_subvol_opts: compress=no drops compress=zstd" { + run parse_btrfs_subvol_opts "@media" "compress=no" + [ "$output" = "subvol=@media,noatime,space_cache=v2,discard=async" ] +} + +@test "parse_btrfs_subvol_opts: nodatacow adds nodatacow and drops compress=zstd" { + run parse_btrfs_subvol_opts "@vms" "nodatacow" + [ "$output" = "subvol=@vms,noatime,space_cache=v2,discard=async,nodatacow" ] +} + +@test "parse_btrfs_subvol_opts: nosuid adds nosuid,nodev and keeps compression" { + run parse_btrfs_subvol_opts "@tmp" "nosuid" + [ "$output" = "subvol=@tmp,noatime,compress=zstd,space_cache=v2,discard=async,nosuid,nodev" ] +} + +@test "parse_btrfs_subvol_opts: nodatacow and nosuid combine" { + run parse_btrfs_subvol_opts "@x" "nodatacow,nosuid" + [ "$output" = "subvol=@x,noatime,space_cache=v2,discard=async,nodatacow,nosuid,nodev" ] +} -- cgit v1.2.3