#3059 Issue open: Make "rear recover" robust against problematic mount options

Labels: enhancement

jsmeix opened issue at 2023-10-27 08:04:

For the reason behind see in particular
https://github.com/rear/rear/issues/2777#issuecomment-1077600797

Currently during "rear recover" it mounts filesystems
with all mount options that have been reported by
mount -t ... and/or
findmnt -mnrv -o SOURCE,TARGET,FSTYPE,OPTIONS -t ...
at the time when "rear mkrescue/mkbackup" was run on the original system
except for 'vfat', see
https://github.com/rear/rear/issues/576

In some cases mounting with all those mount options fails
and then "rear recover" also fails, e.g. see
https://github.com/rear/rear/issues/2777

Probably in most cases all those mount options are not needed
to mount the recreated filesystems during "rear recover"
because all what is needed during "rear recover" is to get
the filesystems mounted "sufficiently OK" so that it works
to restore the files from backup into the empty recreated
filesystems, for an example see
https://github.com/rear/rear/issues/576

After "rear recover" the recreated system will be rebooted
and then its filesystems get mounted according to what is
stored in the restored config files from the backup.

So a possible way to make "rear recover" robust
against problematic mount options could be to
first try to mount the recreated filesystems
with all those mount options
but when this fails
then try to mount it without any options as fallback
and only if also this fails stop with an error,
cf. "Dirty hacks welcome" in
https://github.com/rear/rear/wiki/Coding-Style

So in the mount_fs() function in
layout/prepare/GNU/Linux/133_include_mount_filesystem_code.sh
we may add a separated case for each $fstype
for example like (totally untested offhanded proposal):

(<fstype>)
    ( echo "mkdir -p $TARGET_FS_ROOT$mountpoint"
      echo "if ! mount $mountopts $device $TARGET_FS_ROOT$mountpoint ; then"
      echo "    # Trying to mount without mount options as fallback"
      echo "    mount $device $TARGET_FS_ROOT$mountpoint"
      echo "fi"
    ) >> "$LAYOUT_CODE"
    ;;

I cannot imagine what could go worse with such a fallback attempt.
If the fallback works to get the backup restored things should be OK,
if not "rear recover" behaves as before i.e. it also fails
(only later at a different place).

Perhaps some standard mount options might be useful
also in the fallback case?
For example mount options to ensure mandatory behaviour
like 'rw' because write access is needed to restore the backup.
On the other hand it might be perhaps better to leave out 'rw'
('rw' is the default) and proceed "bona fide" in the fallback case.
When later the backup cannot be restored, "rear recover" would fail
there at the right place where the actual error happened.
Perhaps some filesystem gets mounted 'ro' in the fallback case
but no backup files will be restored into that filesystem?

pcahyna commented at 2023-10-27 09:31:

👍 , at the same time I would remove the special handling of VFAT introduced because of #576

jsmeix commented at 2023-10-27 09:44:

Yes of course I planned to remove the special handling of VFAT
(except for VFAT it never works with those mount options).

jsmeix commented at 2023-10-27 13:46:

@pcahyna
while looking at the mount_fs() function in
layout/prepare/GNU/Linux/133_include_mount_filesystem_code.sh
I noticed

(ext2 | ext3 | ext4)
    (
    echo "mkdir -p $TARGET_FS_ROOT$mountpoint"
    echo "mount $mountopts $device $TARGET_FS_ROOT$mountpoint"
    # try remount for xattr
    # mount and then try remount for systems supporting xattr
    # add a second mount for extended attr (selinux) 
    # the first mount will do perform the basic mount, the second mount (remount) will try for xattr
    # if xattr are not support, the mount will fail, however, the first mount will still be in effect and not cause any errors
    echo "mount $mountopts,remount,user_xattr $device $TARGET_FS_ROOT$mountpoint"
    ) >> "$LAYOUT_CODE"
    ;;

where I think that in contrast to what the comment states
"rear recover" will fail when the second mount (remount) fails
(i.e. when the second mount results a non-zero exit code)
because LAYOUT_CODE="$VAR_DIR/layout/diskrestore.sh"
runs with 'set -e' see
layout/prepare/default/540_generate_device_code.sh

pcahyna commented at 2023-10-27 13:51:

@jsmeix good catch, I looked at this code, but I have not realized this. Note that we never had any report about the problem occurring in practice, IIRC. So I suspect that this special handling is unnecessary, but we need to check why it was introduced.

pcahyna commented at 2023-10-27 13:54:

commit 2011f0af2cffa02be5c6889d806664cfcfeb40bd PR #2142

jsmeix commented at 2023-10-27 14:24:

From the comment at that code place follows
that also 'user_xattr' should be considered
as a "problematic" mount option (i.e. a
mount option that could let 'mount' fail)
so this issue also applies to that part.

pcahyna commented at 2023-10-27 20:13:

@jsmeix but you have found that the code is not as fail-safe as it seems, due to use of set -e. So, given the notable lack of actual reports of failures at this code spot, I suspect that the implied judgement that 'user_xattr' should be considered
as a "problematic" mount option in your sense was simply a wrong judgement from the code's author. Note also the lack of explanation of what actual problem was the code supposed to solve, or even why is it necessary to enable 'user_xattr' even if it has not been present among the original mount options (one would normally assume that if the original system has not had it, it is not needed - this is general assumption of the whole ReaR operation, we attempt to reproduce the original system faithfully, not to improve it).

pcahyna commented at 2023-10-27 20:16:

IOW: if 'user_xattr' were really as problematic as the code implies, we would have seen reports of failures due to set -e already.

[Export of Github issue for rear/rear.]