#2786 PR `closed`: Print error if UUID not updated¶

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

bwelterl opened issue at 2022-04-07 11:47:¶

Relax-and-Recover (ReaR) Pull Request Template¶

Please fill in the following items before submitting a new pull request:

Pull Request Details:¶

Type: Enhancement
Impact: Normal
Reference to related issue (URL):
https://github.com/rear/rear/issues/2785
How was this pull request tested?
internally
Brief description of the changes in this pull request:

Instead of just checking the return code of sed (which is 1 even if no patch is done)

    sed  "$sed_script" "$restored_file" || LogPrintError "Migrating filesystem UUIDs in $restored_file to current UUIDs failed";

I suggest to check if the file was really modified and print an error if not:

    LogPrint "Patching filesystem UUIDs in $restored_file to current UUIDs"
    # Do not error out at this late state of "rear recover" (after the backup was restored) but inform the user:
    sed -i.prerear "$sed_script" "$restored_file" || LogPrintError "Migrating filesystem UUIDs in $restored_file to current UUIDs failed";

    # sed does not print an error if the pattern is not found in the file, thus we keep a backup and compare it to patched file:
    if test -f "$restored_file".prerear; then
        diff "$restored_file" "$restored_file".prerear  >&2 && LogPrintError "WARNING: File $restored_file has not been modified during UUID migration, please check if the old UUID is correct: sed script: *$sed_script *"
        rm -f "$restored_file".prerear
    fi

Please note: This will not cover the case when multiple UUID must be updated and only one is not...
And also, we will have false warning because some files should not be updated (/etc/default/grub for example).
A deeper modification might be necessary to have a sed script for each UUID, thus we will be able to detect which one is missing when it should be (but even in this case, false warning will appear).

jsmeix commented at 2022-04-07 11:56:¶

@bwelterl
thank you for your fix!
I will have a look tomorrow (if nothing gets in my way).

pcahyna commented at 2022-04-07 12:44:¶

I believe the selinux changes should not be there, they belong to a different PR.

bwelterl commented at 2022-04-07 12:49:¶

I believe the selinux changes should not be there, they belong to a different PR.

ooops thanks, the PR is only on usr/share/rear/finalize/GNU/Linux/280_migrate_uuid_tags.sh ...

pcahyna commented at 2022-04-07 12:53:¶

Thanks for the PR, but I am afraid this will not work as expected. What if the file does not contain the UUID at all, therefore does not require patching? You mention /etc/default/grub, but there are other cases (there is no requirement to mount devices by UUID in fstab, you can mount them by label or by device path), so in many cases it is entirely OK that sed does not patch anything. Lots of false warning will be printed and either scare the user, or will be ignored.

pcahyna commented at 2022-04-07 13:04:¶

I have another idea for detecting these kinds of problems. Could mkrescue save checksums of all the files that need to be patched, recover check them before patching them, and if any checksum differs, warn?

jsmeix commented at 2022-04-07 14:47:¶

Regarding checksums of (config) files, cf.
layout/compare/default/510_compare_files.sh

This is currently only run in case of "rear checklayout",
cf. lib/checklayout-workflow.sh
but we could do something like that also for "rear mkrescue".

Furthermore we could clean up things by using only the
already existing CHECK_CONFIG_FILES array
(or a more general similar array) in default.conf
everywhere instead of hardcoded file lists in scripts like
finalize/GNU/Linux/280_migrate_uuid_tags.sh

This would also provide "final power to the user"
when the user can specify what files to check and migrate
if the default array setting is not what he needs.

pcahyna commented at 2022-04-07 15:33:¶

The larger issue is what to do when a checksum mismatch is detected - the recovered system is likely broken then and it is too late to produce a correct/consistent backup.
Of course, any "Print error" approach, such as the one in this PR, will suffer from the same problem.

jsmeix commented at 2022-04-08 13:25:¶

I think what @bwelterl intends is to only detect
when the recovered system could be broken
so he could manually fix things after "rear recover"
in the still running recovery system
in contrast to now where all looks well
but then the recreated system fails to boot.

jsmeix commented at 2022-05-05 10:41:¶

This pull request should be obsoleted by
https://github.com/rear/rear/pull/2795
which implements a more generic check to detect
when restored basic system files do not match the recreated system.

@bwelterl
if possible for you please test our current ReaR GitHub master code,
see the section
"Testing current ReaR upstream GitHub master code" in
https://en.opensuse.org/SDB:Disaster_Recovery

[Export of Github issue for rear/rear.]

#2786 PR closed: Print error if UUID not updated¶