Skip to content

BCF-7410: Backup and restore Btrfs options - #99

Merged
Anastasiya Kapskaya (NastyaKapskaya) merged 3 commits into
developfrom
feature/BCF-7410-backup-and-restore-btrfs-options
Aug 6, 2026
Merged

BCF-7410: Backup and restore Btrfs options#99
Anastasiya Kapskaya (NastyaKapskaya) merged 3 commits into
developfrom
feature/BCF-7410-backup-and-restore-btrfs-options

Conversation

@svlv

Copy link
Copy Markdown

Backup and restore Btrfs nodesize, sectorsize and filesystem features

Copilot AI review requested due to automatic review settings July 29, 2026 12:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for backing up and restoring Btrfs filesystem creation parameters (nodesize, sectorsize, and enabled filesystem features) so recovered systems can recreate Btrfs filesystems more faithfully.

Changes:

  • Added helper functions to detect Btrfs version, available/enabled features, and to construct appropriate mkfs.btrfs feature flags.
  • Extended filesystem layout saving to record nodesize=, sectorsize=, and features= for Btrfs filesystems, and updated filesystem recreation to apply those parameters.
  • Added unit tests for the new filesystem helper functions and added comm as a required binary.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
usr/share/rear/lib/filesystems-functions.sh Adds Btrfs feature/version detection and mkfs option construction helpers.
usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh Persists Btrfs nodesize/sectorsize/features into the saved layout.
usr/share/rear/layout/prepare/GNU/Linux/131_include_filesystem_code.sh Applies saved Btrfs nodesize/sectorsize/features during filesystem creation.
usr/share/rear/conf/default.conf Includes comm to support feature list comparisons.
tests/COVE/005_filesystems_functions.bats Adds unit tests covering the new Btrfs helper logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Comment thread tests/COVE/005_filesystems_functions.bats Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

usr/share/rear/lib/filesystems-functions.sh:171

  • Runtime feature extraction uses grep -P with a negative lookbehind. This adds a hard dependency on PCRE grep; using ERE with grep -E is sufficient here and avoids portability issues.
        for runtime_feature in free-space-tree quota; do
            local found
            if found=$(echo "$features" | grep -oP '(?<![a-z])\^?'"$runtime_feature"',?'); then
                features="${features/$found/}"
                runtime_features+=",${found%,}"
            fi

usr/share/rear/lib/filesystems-functions.sh:51

  • get_btrfs_version relies on grep -P (PCRE lookbehind). grep -P is not guaranteed to be available (e.g., BusyBox grep) and can cause version detection to fail even when btrfs is present. Consider using POSIX tools (awk) to extract the version, and also capture stderr to handle implementations that write version info there.

This issue also appears on line 166 of the same file.

function get_btrfs_version() {
    if ! has_binary btrfs; then
        return 127
    fi

    btrfs version | grep -oP '(?<=btrfs-progs v)[\d.]+'
}

@svlv
Andrus Suvalau (svlv) force-pushed the feature/BCF-7410-backup-and-restore-btrfs-options branch 2 times, most recently from 9d02dc0 to dc9038c Compare July 31, 2026 09:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

usr/share/rear/lib/filesystems-functions.sh:152

  • The error message here refers to “mkfs.btrfs”, but the new unit test expects “btrfs-progs” and (more importantly) the feature support depends on the btrfs-progs version as a whole. This mismatch will fail the added Bats test and is inconsistent terminology.
        LogPrintError "'$unsupported_feature' is an unsupported Btrfs filesystem feature in the version of mkfs.btrfs used by the rescue system and cannot be recovered."

usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh:268

  • If get_enabled_btrfs_features returns an empty string (but exits 0), the layout will still emit features=. During restore, create_fs() treats an empty features value as “disable all features” (see get_btrfs_features_option_for_mkfs), which can create a filesystem with unintended feature flags. Only persist the features option when it is non-empty.
                if features=$(get_enabled_btrfs_features "$uuid" ); then
                    echo -n " features=$features"
                else

usr/share/rear/lib/filesystems-functions.sh:50

  • This uses grep -P (PCRE) with lookbehind, which is a GNU grep feature that is not consistently available across all environments (and is not used elsewhere in this repo). Parsing the version with awk avoids introducing a PCRE dependency while preserving the current “exit 1 on unexpected output” behavior.
    btrfs version | grep -oP '(?<=btrfs-progs v)[\d.]+'

@svlv
Andrus Suvalau (svlv) force-pushed the feature/BCF-7410-backup-and-restore-btrfs-options branch 2 times, most recently from 960e645 to 6d6cda7 Compare July 31, 2026 11:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

usr/share/rear/layout/save/default/950_verify_disklayout_file.sh:230

  • broken_btrfs_errors is appended to in this loop and later iterated, but it is never initialized like the other broken_*_errors arrays. If nounset is enabled or the variable is set in the environment, this can cause failures or leakage between runs. Initialize it to an empty local array before the loop.
Log "Verifying that the Btrfs filesystem entries in $DISKLAYOUT_FILE are correct"
while read -r _ device _ _ _ _ options; do

usr/share/rear/layout/prepare/GNU/Linux/131_include_filesystem_code.sh:287

  • This backward-compat fallback hard-codes -R ^free-space-tree, but your own logic elsewhere notes that -R is deprecated since btrfs-progs 6.3. On newer rescue media where -R is removed/unsupported, filesystem creation would fail for older sessions. Prefer selecting -R only for the 5.7–6.2 window and use -O otherwise.
            if [ -z "$features" ] && [ "$BACKUP" = "COVE" ] && [[ ! "$options" == *"space_cache=v2"* ]]; then
                features=" -R ^free-space-tree"
            fi

tests/COVE/005_filesystems_functions.bats:540

  • This test case is named as a nodesize validation test, but it calls is_btrfs_sectorsize_valid, so it doesn't actually test the nodesize validator against non-numeric input.
@test "Is Btrfs nodesize valid: nodesize containing non-numeric characters is invalid" {
    run -1 is_btrfs_sectorsize_valid "16384;rm -rf /"
}

@svlv
Andrus Suvalau (svlv) force-pushed the feature/BCF-7410-backup-and-restore-btrfs-options branch 2 times, most recently from c72feeb to 4bbac32 Compare July 31, 2026 13:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

usr/share/rear/lib/filesystems-functions.sh:234

  • is_btrfs_sectorsize_valid has the same arithmetic-parsing issue as is_btrfs_nodesize_valid: non-numeric input triggers a bash arithmetic syntax error to stderr. Guarding for numeric-only input avoids noisy error output during disklayout verification/recovery.
function is_btrfs_sectorsize_valid() {
    local sectorsize=$1
    (( sectorsize > 0 && (sectorsize & (sectorsize - 1)) == 0 ))
}

usr/share/rear/lib/filesystems-functions.sh:50

  • get_btrfs_version relies on grep -P with a lookbehind. grep -P (PCRE) is not guaranteed to be available in all rescue environments, which can cause version detection to fail and then break the feature handling logic that depends on it. Consider parsing the version using awk/sed without PCRE so it works with non-GNU grep implementations too.
    if ! has_binary btrfs; then
        return 127
    fi

    btrfs version | grep -oP '(?<=btrfs-progs v)[\d.]+'

usr/share/rear/lib/filesystems-functions.sh:177

  • get_btrfs_features_option_for_mkfs uses grep -P with a negative lookbehind to extract runtime features. This makes the feature split dependent on PCRE support, even though the same parsing can be done reliably in bash by splitting on commas. Avoiding grep -P here reduces portability risk and removes a relatively complex regex.
        local runtime_features=""
        for runtime_feature in free-space-tree quota; do
            local found
            if found=$(echo "$features" | grep -oP '(?<![a-z])\^?'"$runtime_feature"',?'); then
                features="${features/$found/}"

usr/share/rear/lib/filesystems-functions.sh:78

  • get_available_btrfs_features silently skips adding runtime features when get_btrfs_version fails (e.g., unexpected btrfs version output). On btrfs-progs 5.7–6.2 this can lead to an incomplete feature list (missing e.g. quota/free-space-tree) without any error, which then affects restore-time mkfs options.
    # We need to get runtime features using mkfs.btrfs -R list-all for versions
    # between 5.7 and 6.2. Since 6.3, the -R option has been deprecated,
    # and all features have been merged into the -O option.
    if printf '%s\n' "5.7" "$(get_btrfs_version)" "6.2" | sort -V -C; then
        if ! buffer="$(mkfs.btrfs -R list-all 2>&1)"; then

usr/share/rear/lib/filesystems-functions.sh:228

  • is_btrfs_nodesize_valid uses bash arithmetic directly on the input. When the value contains non-numeric characters, bash emits an arithmetic syntax error to stderr, which can leak confusing messages into logs. Adding a simple numeric guard avoids that noise and keeps validation behavior predictable.

This issue also appears on line 231 of the same file.

function is_btrfs_nodesize_valid() {
    local nodesize=$1
    # The nodesize must be not larger than 64KiB and a power of 2
    (( nodesize > 0 && nodesize <= 65536 && (nodesize & (nodesize - 1)) == 0 ))
}

tests/COVE/005_filesystems_functions.bats:550

  • Test title says "nodesize" but the test actually validates sectorsize (calls is_btrfs_sectorsize_valid). Updating the description avoids confusion when reading test output.
@test "Is Btrfs nodesize valid: sectorsize must be a power of 2" {
    run -1 is_btrfs_sectorsize_valid 1023

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

usr/share/rear/lib/filesystems-functions.sh:133

  • get_enabled_btrfs_features currently errors out if get_available_btrfs_features fails. That means a system without mkfs.btrfs (or with a version-detection issue) will not save any Btrfs feature list, even though the enabled features can still be read from sysfs. It’s safer to keep the sysfs-derived list and only filter it when the available-features list is actually obtainable.
    # Filter out filesystem features that can't be passed to the mkfs.btrfs -O option 
    local available_features
    if ! available_features=$(get_available_btrfs_features); then
        LogPrintError "Failed to get the list of available Btrfs features to filter out enabled features for the filesystem with UUID $uuid."
        return 1

usr/share/rear/lib/filesystems-functions.sh:51

  • get_btrfs_version currently hard-requires the btrfs binary, but the feature detection code primarily depends on mkfs.btrfs. If mkfs.btrfs is present but btrfs is not, version detection fails and feature handling degrades unnecessarily. Consider falling back to mkfs.btrfs --version when btrfs is missing.

This issue also appears on line 129 of the same file.

function get_btrfs_version() {
    if ! has_binary btrfs; then
        return 127
    fi

    btrfs version | grep -oP '(?<=btrfs-progs v)[\d.]+'
}

Backup and restore Btrfs nodesize, sectorsize and filesystem features

Jira-Ref: BCF-7410: Backup and restore Btrfs options
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Jira-Ref: BCF-7410: Backup and restore Btrfs options

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

usr/share/rear/lib/filesystems-functions.sh:77

  • The version range check for deciding when to use mkfs.btrfs -R list-all incorrectly excludes patch releases like 6.2.1 (because it compares against the literal upper bound 6.2). For 6.2.x, this will skip the -R query and miss runtime features, leading to incomplete feature detection.
    # We need to get runtime features using mkfs.btrfs -R list-all for versions
    # between 5.7 and 6.2. Since 6.3, the -R option has been deprecated,
    # and all features have been merged into the -O option.
    local btrfs_version
    if ! btrfs_version=$(get_btrfs_version); then
        LogPrintError "Failed to determine the Btrfs version to check whether mkfs.btrfs -R list-all must be used."
        return 1
    fi

    if printf '%s\n' "5.7" "$btrfs_version" "6.2" | sort -V -C; then
        if ! buffer="$(mkfs.btrfs -R list-all 2>&1)"; then
            LogPrintError "Failed to get the list of available Btrfs runtime features using mkfs.btrfs -R list-all."
            return 1
        fi
        features+=$'\n'
        features+="$(echo "$buffer" | awk 'NR>1 {print $1}')"
    fi

tests/COVE/005_filesystems_functions.bats:29

  • This test uses run -127 get_btrfs_version. In bats-core, run does not take an expected-exit-code argument; passing -127 is treated as an option and will make the test fail to execute. Other tests in tests/COVE/*.bats use run <cmd> and then assert on $status.
@test "Check Btrfs version: btrfs is missing" {
    function has_binary() {
        [ "$1" != "btrfs" ]
    }

    run -127 get_btrfs_version
}

usr/share/rear/lib/filesystems-functions.sh:180

  • The version range check for splitting runtime features into -R options has the same issue as above: 6.2.1 and other 6.2.x versions will not be treated as needing -R, even though the -R deprecation starts at 6.3. This can generate the wrong mkfs option set for 5.7 <= version < 6.3.
    # For versions 5.7 through 6.2, runtime features must be controlled using the -R option.
    local btrfs_version
    if ! btrfs_version=$(get_btrfs_version); then
        LogPrintError "Failed to determine the Btrfs version to check whether mkfs.btrfs -R list-all must be used."
        return 1
    fi
    if printf '%s\n' "5.7" "$btrfs_version" "6.2" | sort -V -C; then
        local runtime_features=""
        for runtime_feature in free-space-tree quota; do
            local found
            if found=$(echo "$features" | grep -oP '(?<![a-z])\^?'"$runtime_feature"',?'); then
                features="${features/$found/}"
                runtime_features+=",${found%,}"
            fi
        done

        if [ -n "$runtime_features" ]; then
            result+=" -R ${runtime_features#,}"
        fi

        features=${features%,}
    fi

Jira-Ref: BCF-7410: Backup and restore Btrfs options

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

usr/share/rear/lib/filesystems-functions.sh:161

  • When removing unsupported features, the sed substitution can leave consecutive commas when the unsupported feature is in the middle of the comma-separated list (e.g. "extref,mixed-bg,no-holes" -> "extref,,no-holes"). That can later result in an invalid mkfs.btrfs feature list being emitted (empty feature between commas). Clean up duplicate/leading/trailing commas after the removal.
    for unsupported_feature in $unsupported_features; do
        LogPrintError "'$unsupported_feature' is an unsupported Btrfs filesystem feature in the version of mkfs.btrfs used by the rescue system and cannot be recovered."
        # Remove unsupported features caused by using an older mkfs.btrfs on the rescue system than mkfs.btrfs used to create a filesystem.
        features=$(echo "$features" | sed -E "s/(^|,)${unsupported_feature}(,|$)/\1/;s/,$//")
    done

@NastyaKapskaya
Anastasiya Kapskaya (NastyaKapskaya) merged commit 118d140 into develop Aug 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants