Skip to content

BCF-6879: Support Btrfs on multiple block devices - #100

Merged
Anastasiya Kapskaya (NastyaKapskaya) merged 8 commits into
developfrom
feature/BCF-6879-support-btrfs-on-multiple-devices
Sep 29, 2026
Merged

Anastasiya Kapskaya (NastyaKapskaya) merged 8 commits into
developfrom
feature/BCF-6879-support-btrfs-on-multiple-devices

Conversation

@svlv

@svlv Andrus Suvalau (svlv) commented Sep 7, 2026 •

Copy link
Copy Markdown
  • Change SELinux mode from enforcing to permissive on Fedora 44.
  • Fix getting grub-editenv path on Fedora 44 where there is /sbin/type.
  • Backup and restore Btrfs device paths, data profile and metadata profile to support a Btrfs filesystem created on top of multiple block devices.
  • Warn at backup time that seeding Btrfs devices will become regular read-write devices at recovery time

Refs:

Copilot AI lite review requested due to automatic review settings September 7, 2026 11:50

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 extends Relax-and-Recover’s Btrfs handling to support filesystems spanning multiple block devices by persisting the device list in the layout, restoring dependencies accordingly, and using that list during filesystem re-creation. It also refactors SELinux-permissive handling into a shared helper and adjusts GRUB editenv path detection.

Changes:

  • Persist and consume a devices= option for Btrfs filesystems to support multi-device layouts end-to-end.
  • Update layout dependency generation to account for additional Btrfs devices.
  • Centralize SELinux “set permissive” logic into a shared library function and reuse it for RHEL/Fedora COVE finalize steps.

Reviewed changes

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

Show a summary per file
File Description
usr/share/rear/lib/linux-functions.sh Adds shared set_selinux_permissive helper for COVE finalize workflows.
usr/share/rear/lib/layout-functions.sh Adds dependency tracking for additional Btrfs devices from devices= mount options.
usr/share/rear/lib/filesystems-functions.sh Adds helpers to extract Btrfs device lists and validate them.
usr/share/rear/lib/bootloader-functions.sh Adjusts how grub-editenv path is determined outside recovery mode.
usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh Saves Btrfs device lists into layout output.
usr/share/rear/layout/save/default/950_verify_disklayout_file.sh Validates the new devices= option in disklayout.conf.
usr/share/rear/layout/prepare/GNU/Linux/131_include_filesystem_code.sh Uses the saved Btrfs device list when running mkfs during recovery.
usr/share/rear/finalize/COVE/RedHatEnterpriseServer/060_set_selinux_permissive.sh Switches to the new shared SELinux helper.
usr/share/rear/finalize/COVE/Fedora/060_set_selinux_permissive.sh Adds Fedora SELinux-permissive finalize step using the shared helper.
tests/COVE/005_filesystems_functions.bats Adds tests for the new Btrfs helper functions and validator.
Suppressed comments (1)

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

  • The device-path validation regex is too restrictive: it rejects common real-world device paths like /dev/mapper/vg-lv or /dev/disk/by-id/... because it disallows -, ., and additional path segments. This will make valid Btrfs multi-device layouts fail verification/creation.
    for path in "${paths[@]}"; do
        if [[ ! "$path" =~ ^/dev/[/0-9a-zA-Z]+$ ]]; then
            return 1
        fi

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh Outdated
Comment thread usr/share/rear/lib/bootloader-functions.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Comment thread tests/COVE/005_filesystems_functions.bats
Comment thread usr/share/rear/lib/linux-functions.sh
Comment thread usr/share/rear/lib/linux-functions.sh 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 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

usr/share/rear/lib/bootloader-functions.sh:1035

  • Using eval here executes a shell string and re-parses it, which is unnecessary for this fixed command and makes the code harder to reason about (and riskier if the command string ever becomes influenced by environment/user input). You can avoid eval by running type -P directly (it already supports ||), keeping behavior identical without re-parsing.
        GRUB_EDITENV_PATH="$(eval "$grub_editenv_path")"

Comment thread usr/share/rear/lib/filesystems-functions.sh

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 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

usr/share/rear/lib/layout-functions.sh:200

  • The loop uses an undeclared device variable, which becomes function-global in bash and can clobber other variables (or leak out of this function). Use a distinct local loop variable for the Btrfs member devices.
    usr/share/rear/lib/linux-functions.sh:359
  • This comment says the function changes the SELinux mode, but the implementation only edits $TARGET_FS_ROOT/etc/selinux/config (it does not run setenforce). Adjust the comment to reflect what actually happens to avoid misleading future changes.

usr/share/rear/lib/bootloader-functions.sh:1035

  • Avoid eval here. Even though the string is currently constant, eval expands and executes arbitrary shell syntax and is a common injection footgun; using the same bash -c approach as the chroot branch is safer and consistent.
        GRUB_EDITENV_PATH="$(eval "$grub_editenv_path")"

@svlv
Andrus Suvalau (svlv) force-pushed the feature/BCF-6879-support-btrfs-on-multiple-devices branch 2 times, most recently from d20b2d7 to 5a554c0 Compare September 9, 2026 12:07

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 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

usr/share/rear/lib/bootloader-functions.sh:1035

  • Using eval here executes shell syntax from a string and is riskier than necessary. Since grub_editenv_path is already a shell command string (it includes ||), execute it via bash -c (consistent with the chroot branch) instead of eval.
        GRUB_EDITENV_PATH="$(eval "$grub_editenv_path")"

Comment thread usr/share/rear/lib/linux-functions.sh 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 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

usr/share/rear/lib/bootloader-functions.sh:1035

  • Avoid using eval here. Even though grub_editenv_path is currently a hardcoded string, eval executes in the current shell context (side effects, harder to audit) and is unnecessary for running type -P ... || type -P .... Use a subshell via bash -c (matching the recovery-mode branch) or invoke type -P directly.
        GRUB_EDITENV_PATH="$(eval "$grub_editenv_path")"

usr/share/rear/lib/linux-functions.sh:373

  • The line-continuation indentation inside the quoted text ends up as literal spaces in the warning message (because the escaped newline is removed but the next line's leading spaces remain). This can make the message render with odd spacing. Prefer a single-line string (or concatenate without indentation) so the output is predictable.
    local text="During the recovery process of ${os}-based systems, SELinux is set to permissive mode to \
    successfully relabel the entire file system. After verifying that your system is functioning \
    correctly, set SELINUX=enforcing in /etc/selinux/config to return SELinux to enforcing mode."

Comment thread usr/share/rear/layout/prepare/GNU/Linux/131_include_filesystem_code.sh 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 10 out of 10 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

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

  • read -a drops a trailing empty field, so a malformed value such as /dev/sda1, passes this validator. create_fs then expands it to one device and silently omits the missing member instead of rejecting the layout; validate the complete comma-separated form before splitting.
    IFS=',' read -ra paths <<< "$1"

Comment thread usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
@svlv Andrus Suvalau (svlv) changed the title BCF-6879: Support Btrfs on multiple disk devices BCF-6879: Support Btrfs on multiple block devices Sep 11, 2026
@svlv
Andrus Suvalau (svlv) force-pushed the feature/BCF-6879-support-btrfs-on-multiple-devices branch from 642816c to eab81f4 Compare September 11, 2026 15:12
Storing the 'type -P grub-editenv || type -P grub2-editenv' command in the
cmd variable does not work because parsing happens before expansion. By
the time $cmd is expanded, '||' can no longer be interpreted as a control
operator and it's treated as a word instead.

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 10 out of 10 changed files in this pull request and generated 3 comments.

Suppressed comments (6)

usr/share/rear/finalize/COVE/RedHatEnterpriseServer/060_set_selinux_permissive.sh:7

  • OS_VENDOR uses the internal value RedHatEnterpriseServer, so this refactor changes the existing user-facing warning from RHEL 10-based systems to RedHatEnterpriseServer 10-based systems. Pass a display name for this caller so the warning remains understandable to RHEL users.
set_selinux_permissive "${OS_VENDOR} ${OS_VERSION%%.*}"

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

  • The primary filesystem device is normalized with get_device_mapping and get_device_name before it is written (lines 163-167), but the additional paths returned by get_btrfs_devices are emitted raw. On a device-mapper or multipath member that Btrfs reports as /dev/dm-N, recovery can use an unstable or wrong member in the generated mkfs command. Normalize every returned member with the same mapping/name functions before serializing devices=.
                if devices=$(get_btrfs_devices "$mountpoint"); then
                    echo -n " devices=$devices"

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

  • When profile discovery fails, omitting dprofile makes recovery use mkfs.btrfs's default data profile rather than the source profile, defeating the new profile-preservation behavior. This should fail the save (or otherwise prevent recovery from silently falling back to a different profile) instead of merely logging the error.
                    LogPrintError "Failed to get Btrfs data profile for $device"

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

  • The same non-fatal handling here omits mprofile, so recovery silently selects mkfs.btrfs's metadata default whenever discovery fails instead of restoring the source metadata profile. Treat this as a layout-save error rather than continuing with an incomplete Btrfs description.
                    LogPrintError "Failed to get Btrfs metadata profile for $device"

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

  • The allocation profile directory names are not consistently lowercase: for example, metadata is exposed as DUP and RAID profiles as RAID1/RAID10, while the mkfs profile names here are lowercase. This loop therefore misses normal metadata DUP and multi-device RAID profiles, omits mprofile/dprofile, and lets recovery silently use mkfs defaults instead of the backed-up profile. Check the sysfs spelling and emit the normalized lowercase profile.
    for profile in "${BTRFS_PROFILES[@]}"; do
        if [ -d "$sysfs_alloc_path/$type/$profile" ]; then
            echo "$profile"

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

  • The primary filesystem device is normalized with get_device_mapping/get_device_name before the fs entry is written (230_filesystem_layout.sh:152-167), but these additional paths are emitted exactly as reported by btrfs filesystem show. For a device-mapper member reported as /dev/dm-*, the saved dependency and mkfs command will not match the /dev/mapper/* component name used elsewhere and recovery can wait forever or target the wrong path. Normalize each member path using the same mapping before joining the list.
    devices="$(echo "$fs_info" | awk '$1 == "devid" && $(NF-1) == "path" {print $NF}')"
    devices=${devices//$'\n'/,}

Comment thread usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh
Save the list of Btrfs devices in disklayout.conf and set filesystem
dependencies on all used devices. Pass the list to mkfs.btrfs when
creating the filesystem.
A profile describes an allocation policy. It is essential to restore the
Btrfs data and metadata profiles when a Btrfs filesystem is created on
top of multiple block devices, as profiles configure data redundancy and
replication, which typically involve multiple devices.
@svlv
Andrus Suvalau (svlv) force-pushed the feature/BCF-6879-support-btrfs-on-multiple-devices branch from 3ef1e8b to 5f829cc Compare September 14, 2026 15:07
Warn at backup time that seeding devices will become regular read-write
devices at recovery time.

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 10 out of 10 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

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

  • The primary filesystem device was normalized with get_device_mapping/get_device_name before the layout line is written, but get_btrfs_devices returns the raw paths from btrfs filesystem show and this option writes them unchanged. During disk migration/recovery an additional device can therefore remain a /dev/sdX or /dev/dm-* source name, while the new dependency and generated mkfs commands need the mapped target name. Normalize every returned path before emitting devices= (or in the helper, while preserving its library-loading contract).
                if devices=$(get_btrfs_devices "$mountpoint"); then
                    echo -n " devices=$devices"

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

  • This branch detects a mixed-mode filesystem, but only returns its data/metadata profile; no mixed-mode attribute is saved, and create_fs always emits separate -d and -m options. A source created with mkfs.btrfs --mixed will therefore be recreated with different allocation semantics. Persist the mixed-mode state and emit mkfs.btrfs -M for it instead of treating it as two independent profiles.
    elif [ -d "$sysfs_alloc_path/mixed" ]; then
        # In mixed mode /sys/fs/btrfs/UUID/allocation/mixed/ should be used
        type=mixed

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

  • This regex permits .. path components, so a value such as /dev/../etc/passwd passes validation even though it escapes /dev. The value is later interpolated into the generated mkfs command, so malformed or edited layout data can make recovery operate on a path outside the device tree; reject parent-directory components (or canonicalize and enforce the /dev prefix).
        if [[ ! "$path" =~ ^/dev/[[:alnum:]/._-]+$ ]]; then

Comment thread usr/share/rear/layout/prepare/GNU/Linux/131_include_filesystem_code.sh Outdated
Comment thread usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Comment thread usr/share/rear/lib/layout-functions.sh
Comment thread usr/share/rear/lib/layout-functions.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh
Comment thread usr/share/rear/lib/filesystems-functions.sh Outdated
Comment thread usr/share/rear/layout/prepare/GNU/Linux/131_include_filesystem_code.sh Outdated
Comment thread usr/share/rear/lib/filesystems-functions.sh

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.

🟡 Changes recommended

Recovery cleanup is skipped for some filesystem types, and older Btrfs layouts fail without the new devices option.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

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

  • The member paths returned by btrfs filesystem show are serialized verbatim, unlike the primary filesystem device which is normalized with get_device_mapping and get_device_name before being written. On layouts involving dm, multipath, or other alternate device names, devices= can therefore refer to components that do not match the normalized disk/part entries; dependency generation and migration then leave those members unrecreated or unreplaced. Normalize each returned member path before writing the list, as is done for RAID component paths.
                if devices=$(get_btrfs_devices "$mountpoint"); then
                    echo -n " devices=$devices"
  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Lite

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.

🔵 Needs a closer look

Device normalization, mixed-mode preservation, and failed device discovery currently allow incorrect Btrfs recovery.

Review details

Suppressed comments (3)

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

  • The mixed allocation directory identifies a mixed data/metadata filesystem, but this branch returns only a normal profile name. The layout writer stores that as dprofile/mprofile, and the recovery code turns those into -d/-m without mkfs.btrfs -M, so mixed-mode filesystems are recreated with separate data and metadata block groups. Preserve the mixed-mode flag separately and emit -M during recovery.
    elif [ -d "$sysfs_alloc_path/mixed" ]; then
        # In mixed mode /sys/fs/btrfs/UUID/allocation/mixed/ should be used
        type=mixed

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

  • These device paths are copied directly from btrfs filesystem show, while the primary $device is canonicalized with get_device_mapping/get_device_name before this code. For device-mapper or multipath members this can preserve an unstable /dev/dm-X or by-id path in devices=, and recovery will then use the wrong/nonexistent member for both dependencies and mkfs. Normalize every returned member with the same mapping sequence before writing the layout entry.
                if devices=$(get_btrfs_devices "$mountpoint"); then
                    echo -n " devices=$devices"

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

  • When device discovery fails, this only logs the problem and omits devices=. create_fs then initializes devices from the single primary $device, so a multi-device filesystem can produce a successful backup whose recovery recreates and formats only one member. Treat failure to obtain the member list as fatal (or otherwise make the layout verifier reject the entry) instead of silently falling back.
                    LogPrintError "Failed to get a list of Btrfs device paths for $mountpoint"
  • Files reviewed: 10/10 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@NastyaKapskaya
Anastasiya Kapskaya (NastyaKapskaya) merged commit 0e895d5 into develop Sep 29, 2026
6 checks passed
@svlv
Andrus Suvalau (svlv) deleted the feature/BCF-6879-support-btrfs-on-multiple-devices branch October 2, 2026 13:15
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.

5 participants