#3365 PR merged: savelayout: error out on mounted multi-disk BTRFS filesystems

Labels: enhancement, fixed / solved / done

lzaoral opened issue at 2024-12-16 14:57:

Pull Request Details:
  • Type: Bug Fix / "Workaround"

  • Impact: High

  • Reference to related issue (URL): https://github.com/rear/rear.github.com/pull/22#issuecomment-2540763985

  • How was this pull request tested? rear savelayout on a Fedora 41 system with different combinations of (un)mounted single disk and multi disk BTRFS filesystems

  • Description of the changes in this pull request:

Error out on mounted multi-disk BTRFS filesystems because ReaR does not support this scenario at the moment.

jsmeix commented at 2024-12-16 15:31:

Regarding
https://github.com/rear/rear/pull/3365#discussion_r1886997203

Works for me on SLES 12 SP5:

# rpm -qf /usr/sbin/btrfs
btrfsprogs-4.5.3-24.27.x86_64

# btrfs filesystem show /
Label: none  uuid: b3fc4e1d-63bc-4ec7-b131-0c43b8245b68
        Total devices 1 FS bytes used 4.80GiB
        devid    1 size 12.99GiB used 7.02GiB path /dev/sda2

# for mp in $( findmnt -a -n -l -t btrfs -o TARGET ) ; \
  do btrfs filesystem show $mp | grep -q 'Total devices 1' \
     && echo OK for $mp \
     || echo FAILED for $mp ; \
  done
OK for /
OK for /var/lib/pgsql
OK for /var/lib/mailman
OK for /var/lib/named
OK for /tmp
OK for /opt
OK for /boot/grub2/i386-pc
OK for /var/spool
OK for /var/lib/mariadb
OK for /.snapshots
OK for /var/lib/machines
OK for /var/crash
OK for /srv
OK for /var/lib/mysql
OK for /var/cache
OK for /home
OK for /var/lib/libvirt/images
OK for /var/opt
OK for /var/tmp
OK for /var/log
OK for /boot/grub2/x86_64-efi
OK for /usr/local

Works for me for SLES 15 SP6:

# rpm -qf /usr/sbin/btrfs
btrfsprogs-6.5.1-150600.2.4.x86_64

# btrfs filesystem show /
Label: none  uuid: bad9f5d9-6a37-4902-b5fc-ded16e84f357
        Total devices 1 FS bytes used 5.36GiB
        devid    1 size 12.99GiB used 9.07GiB path /dev/vda2

# for mp in $( findmnt -a -n -l -t btrfs -o TARGET ) ; \
  do btrfs filesystem show $mp | grep -q 'Total devices 1' \
     && echo OK for $mp \
     || echo FAILED for $mp ; \
  done
OK for /
OK for /.snapshots
OK for /boot/grub2/i386-pc
OK for /boot/grub2/x86_64-efi
OK for /home
OK for /opt
OK for /root
OK for /srv
OK for /tmp
OK for /usr/local
OK for /var

jsmeix commented at 2024-12-16 15:45:

@lzaoral
perhaps a more failsafe implementation could be
to first grep for "Total devices" (perhaps with ignore case?)
and only if that matches grep for
not "Total devices 1 " (with trailing space)
for example something like

# Only test when 'btrfs filesystem' shows the number of "Total devices":
if grep -qi "total devices" ... ; then
    # Ensure a btrfs filesystem consists of only one device
    # cf. https://github.com/rear/rear/issues/2028
    if ! grep -qi "total devices 1 " ... ; then

jsmeix commented at 2024-12-16 16:09:

@rear/contributors
please have a look here and
if time permits and when you have a system with btrfs
check if this also works for you
because currently it is intended to be included
in ReaR 2.8 according to
https://github.com/rear/rear.github.com/pull/22#issuecomment-2540763985

When issues appear with the change here
we can easily postpone it to ReaR 3.0 because
when we do not do the check in this pull request
it is not worse than it was all the time up to ReaR 2.7

schlomo commented at 2024-12-16 16:12:

When issues appear with the change here we can easily postpone it to ReaR 3.0 because when we do not do the check in this pull request it is not worse than it was all the time up to ReaR 2.7

I'd rather include this check now as we know about the problem and I always prefer direct and clear communication to our users over "just failing".

jsmeix commented at 2024-12-16 16:21:

I fully agree with
https://github.com/rear/rear/pull/3365#issuecomment-2546046917
in particular during "rear mkrescue" because
even if some Error() exit would be false in some cases,
it can be relatively easily removed in the code by the user and
we get (hopefully) informed by the user via a GitHub issue,
in contrast to "somehow failing elsewhere" where "elsewhere"
is likely during "rear recover" which is much more annoying
for the user and debugging is much more complicated
both for us and the user.

jsmeix commented at 2024-12-18 08:37:

@pcahyna
could you also have a look here and tell your opinion
whether or not it should be included in ReaR 2.8
provided @lzaoral commits something like his
https://github.com/rear/rear/pull/3365#discussion_r1887046610
or if this pull request should be postponed to ReaR 3.0
because without this pull request it is not worse
than it had been all the time up to ReaR 2.7
cf. https://github.com/rear/rear/pull/3365#issuecomment-2546039387
and https://github.com/rear/rear/pull/3365#discussion_r1887100637

jsmeix commented at 2024-12-18 11:36:

@rear/contributors
I intend to merge it in one hour or so
unless severe objections appear against the currrently implemented

grep -qE 'Total devices ([2-9]|[1-9][0-9]+) '

method which is tested on Red Hat / Fedora and SLES 12 / 15.

schlomo commented at 2024-12-18 12:56:

Tested is always a very hard arguments, so let's go ahead with it. We can improve it when the output changes.

jsmeix commented at 2024-12-18 15:29:

FYI an addendum regarding
https://github.com/rear/rear/pull/3365#discussion_r1887100637
that reads (excerpt)

any chance to have something more robust
that [sic] matching on the english text output of the tool?

The english therein made me wonder if ReaR
actually enforces English text output of what it calls
and yes, it does because sbin/rear enforces C locale

# Make sure that we use only English:
export LC_CTYPE=C LC_ALL=C LANG=C

Phew!
;-)


[Export of Github issue for rear/rear.]