#3223 PR closed: In 400_save_directories.sh mkdir $VAR_DIR/recovery/ when not existing

Labels: bug, won't fix / can't fix / obsolete

jsmeix opened issue at 2024-05-15 14:35:

In prep/default/400_save_directories.sh
create $VAR_DIR/recovery/ when not existing

jsmeix commented at 2024-05-16 07:42:

I added my somewhat unrelated looking improvement
https://github.com/rear/rear/pull/3223/commits/2a65eab4c254eae4c405836eaad4f09541497575
just here "by the way" because while debugging it in
https://github.com/rear/rear/pull/3175#issuecomment-2112594449
I got a bit confused because it was not clear to me
if ReaR's TMP_DIR was used versus the system's TMPDIR
so I inspected the code in usr/sbin/rear which was

    if ! [[ "$RECOVERY_MODE" || "$PORTABLE" ]] ; then
        ...
            tmpdir_debug_info="Setting TMPDIR to '$TMP_DIR' (was unset when ReaR was launched)"
        ...
    else
        ...
            tmpdir_debug_info="Setting TMPDIR to '$TMPDIR' (was unset when ReaR was launched)"

i.e. same message text like

Setting TMPDIR to '/var/tmp' (was unset when ReaR was launched)

for two different cases so it was not possible
to see from the message text which case it was.
The '/var/tmp' indicates it was the second case
but that is not 100% clear.
So that I had to reverse-engineer things
to understand what actually goes on.
With the improvement it looks like

Setting TMPDIR to ReaR's '/var/tmp/rear.3pc7RyD4Ki2uoMC/tmp' (was unset when ReaR was launched)

versus

Setting TMPDIR to '/var/tmp' (was unset when ReaR was launched)

jsmeix commented at 2024-05-16 09:00:

@rear/contributors
I would be happy if one of you could review it
because I would like to merge it today afternoon
i.e. before the long Pentecost weekend
which starts already tomorrow for me.

schlomo commented at 2024-05-16 09:25:

Good catch, can I suggest putting that "create recovery dir" topic into a very early (potentially new) script in the prep stage and not into the script that currently is the first one to require it?

jsmeix commented at 2024-05-16 10:15:

It seems the whole
usr/share/rear/prep/default/400_save_directories.sh
needs to be moved to a 'layout' stage because
usr/share/rear/prep/README reads (excerpt)

You should not put scripts into this 'prep' stage that modify things
in ROOTFS_DIR or in VAR_DIR/recovery and VAR_DIR/layout because
scripts for ROOTFS_DIR belong to the 'rescue' stage and scripts
for VAR_DIR/recovery and VAR_DIR/layout belong to the 'layout' stages.

This is something for later...

The problem is that what 400_save_directories.sh implements
does not belong to what is meant with "disk layout"
(i.e. storage volumes, partitions, filesystems, mountpoints)
so 400_save_directories.sh should not be in a 'layout' stage.

jsmeix commented at 2024-05-16 10:38:

I separated the unrelated part
https://github.com/rear/rear/pull/3223#issuecomment-2114294271
out from here into the new
https://github.com/rear/rear/pull/3225

I will close this one because it is the wrong fix.

jsmeix commented at 2024-05-16 10:40:

Next week - as time permits - I will try to make
a proper fix that obsoletes this one.

jsmeix commented at 2024-05-16 11:27:

An offhanded thought:

Perhaps what 400_save_directories.sh implements
could even belong to what is meant with "disk layout"
(i.e. storage volumes, partitions, filesystems, mountpoints)
because when "mountpoints" belong to "disk layout"
then "basic system directories" (i.e. FHS directories)
could also belong - in a broader sense - to "disk layout".

In particular because 400_save_directories.sh
first and foremost saves directories
that are used as mountpoints at least those belong
to "disk layout" even in the narrow sense
so 400_save_directories.sh could sufficiently well
belong to the 'layout/save' stage.

jsmeix commented at 2024-05-16 11:37:

An even further offhanded thought:

When what 400_save_directories.sh implements
belongs to what is meant with "disk layout"
then what its counterpart 900_create_missing_directories.sh
implements could also belong to recreating "disk layout".

So 900_create_missing_directories.sh could be moved
from after the backup was restored
to the earlier layout/recreate stage
and renamed into e.g. create_basic_directories.sh
(and save_directories.sh renamed into save_basic_directories.sh)
where both ensure that all mountpoint directories
and all FHS directories get properly recreated
before they get used, cf.
https://github.com/rear/rear/pull/3175#issuecomment-2114478980
where /mnt/local/tmp was created with wrong permissions
because of plain

mkdir -p /mnt/local/tmp

in diskrestore.sh

pcahyna commented at 2024-05-16 19:55:

It seems the whole
usr/share/rear/prep/default/400_save_directories.sh
needs to be moved to a 'layout' stage because
usr/share/rear/prep/README reads (excerpt)

I think there is another reason to move it: prep scripts run also during mkbackuponly, but we need to save directories only during the rescue image creation, not backup creation.

pcahyna commented at 2024-05-16 20:05:

Perhaps what 400_save_directories.sh implements could even belong to what is meant with "disk layout" (i.e. storage volumes, partitions, filesystems, mountpoints) because when "mountpoints" belong to "disk layout" then "basic system directories" (i.e. FHS directories) could also belong - in a broader sense - to "disk layout".

In particular because 400_save_directories.sh first and foremost saves directories that are used as mountpoints at least those belong to "disk layout" even in the narrow sense so 400_save_directories.sh could sufficiently well belong to the 'layout/save' stage.

There is prior art for saving and restoring such "fixups" (information that may be missing in backup) to the restored files.

For example rescue/NETFS/default/610_save_capabilities.sh (saves capabilities, restored by restore/NETFS/default/510_set_capabilities.sh ) . I am sure I saw some other, but I don't remember them now. Some common approach would be great.

pcahyna commented at 2024-05-16 20:18:

An even further offhanded thought:

When what 400_save_directories.sh implements belongs to what is meant with "disk layout" then what its counterpart 900_create_missing_directories.sh implements could also belong to recreating "disk layout".

So 900_create_missing_directories.sh could be moved from after the backup was restored to the earlier layout/recreate stage and renamed into e.g. create_basic_directories.sh (and save_directories.sh renamed into save_basic_directories.sh) where both ensure that all mountpoint directories and all FHS directories get properly recreated before they get used, cf. #3175 (comment) where /mnt/local/tmp was created with wrong permissions because of plain

mkdir -p /mnt/local/tmp

in diskrestore.sh

IMO, this won't work. You need to create mountpoints before they are used, but after the containing filesystem is created. E.g. if /var and /var/tmp are separate filesystems, you can't create /var/tmp as a directory in the empty root file system. You need to create it only after the /var filesystem has been created and mouted. Therefore moutpoint creation needs to be interspersed with filesystem creation and mounting in diskrestore.sh.

jsmeix commented at 2024-05-22 14:04:

This one is obsoleted by
https://github.com/rear/rear/pull/3232

[Export of Github issue for rear/rear.]