Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skip patching absolute symlinks during finalize stage (related to issue 1338) #2055

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Feb 26, 2019

  • Type: Bug Fix Enhancement Cleanup

  • Impact: Normal

  • Reference to related issue (URL):
    skip patching absolute symlinks during finalize stage
    is only a first step towards actually fixing
    Potentially patching wrong files during recovery #1338

  • How was this pull request tested?
    Not yet tested and not yet complete - some more commits are needed...

  • Brief description of the changes in this pull request:

All finalize scripts that patch restored files within TARGET_FS_ROOT
should do the same symlink handling which means

  1. Skip patching symlink targets that are not within TARGET_FS_ROOT
    (i.e. when it is an absolute symlink)

  2. Skip patching if the symlink target contains /proc/ /sys/ /dev/ or /run/

  3. Skip patching dead symlinks

Skip patching symlink targets that are not within TARGET_FS_ROOT
does not actually fix #1338
but at least it should avoid patching wrong files
so that for now this pull request before the ReaR 2.5 release
is meant only as a first step towards actually fixing
#1338

@jsmeix jsmeix added enhancement Adaptions and new features bug The code does not do what it is meant to do cleanup labels Feb 26, 2019
@jsmeix jsmeix added this to the ReaR v2.5 milestone Feb 26, 2019
@jsmeix jsmeix self-assigned this Feb 26, 2019
…ink handling in 320_migrate_network_configuration_files.sh but that code it too obsure for me
@jsmeix
Copy link
Member Author

jsmeix commented Feb 26, 2019

For me on SLES12-SP4 with UEFI
things still work but that does not mean much
because most of what the finalize scripts can do
is not needed to be done on my particular system.

@gozora
Copy link
Member

gozora commented Feb 26, 2019

@jsmeix

For me on SLES12-SP4 with UEFI
things still work but that does not mean much
because most of what the finalize scripts can do
is not needed to be done on my particular system.

I can test this PR on my Arch Linux where I've observed this behavior for first time (#2047 (comment)), but not sooner that tomorrow (27th Feb) afternoon ...

V.

…here or mess around with udev rules that are created and maintained by systemd/udev
@jsmeix
Copy link
Member Author

jsmeix commented Feb 27, 2019

With my above recent
42b1f0f
that etc/udev/rules.d/70-persistent-net.rules stuff
seems to work o.k. at lest for me for the first time:
Without

BACKUP_RESTORE_MOVE_AWAY_FILES=( /etc/udev/rules.d/70-persistent-net.rules )

I get during finalize stage the restored
/mnt/local/etc/udev/rules.d/70-persistent-net.rules
replaced by the one in the recovery system
but with

BACKUP_RESTORE_MOVE_AWAY_FILES=( /etc/udev/rules.d/70-persistent-net.rules )

I do no longer get a etc/udev/rules.d/70-persistent-net.rules
created in any case in my restored system even when I do not want it
because that ReaR-created etc/udev/rules.d/70-persistent-net.rules
gets in conflict with what systemd/udev normally should do,
i.e. creating etc/udev/rules.d/70-persistent-net.rules
so that from my corrent point of view it seems
#770
is hereby (hopefully) finally fixed.

To no longer get etc/udev/rules.d/70-persistent-net.rules
created by ReaR in the restored system by default we would
have to add /etc/udev/rules.d/70-persistent-net.rules to
BACKUP_RESTORE_MOVE_AWAY_FILES in default.conf

@gozora
Copy link
Member

gozora commented Feb 27, 2019

When I was running test restore of my Arch Linux (with EFISTUB, because EFISTUB is cool :-)) I've hit something which does not feel right to me.
Since I was in migration mode (because why not ...) I was stopped by 020_confirm_finalize.sh, which prompted me:

Confirm restored config files or edit them
1) Confirm it is OK to recreate initrd and reinstall bootloader and continue 'rear recover'
2) Edit restored etc/fstab (/mnt/local/etc/fstab)
3) View restored etc/fstab (/mnt/local/etc/fstab)
4) Use Relax-and-Recover shell and return back to here
5) Abort 'rear recover'
(default '1' timeout 300 seconds)

Now I was tempted to manually update my restored fstab because it did not had right UUIDs ...
Luckily I've let the file as it was because later usr/share/rear/finalize/GNU/Linux/280_migrate_uuid_tags.sh took care of this inconsistency.

What about moving 020_confirm_finalize.sh to later stage, ideally very close before mkinird ?

V.

Copy link
Member

@gozora gozora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've run code from this patch on my Arch Linux with EFISTUB boot twice:

  1. migrated /dev/sda => /dev/sdb
  2. regular restore /dev/sda => /dev/sda

and in both cases it worked flawlessly!

V.

@jsmeix
Copy link
Member Author

jsmeix commented Feb 28, 2019

@gozora
thank you so much for your careful observation.

You are absolutely right that 020_confirm_finalize.sh is too early.

When I added it via
0bdaf5f
I had only in mind to get that user dialog in any case
after the backup was restored (independent of the backup method)
so that I put that script at the beginning of the finalize stage
and not at the end of the restore stage (to be independent of the
particular restore scripts of the particular backup method), cf.
#1758
But at that time I did not have in mind that during finalize
stage ReaR changes restored files which contradicts
the whole idea behind 020_confirm_finalize.sh which is meant
to provide final power to the user to adapt restored files as he wants.

So 020_confirm_finalize.sh must be after ReaR had changed
any restored files and I need to adapt the comment in that script
to better explain what is actually meant...

@jsmeix
Copy link
Member Author

jsmeix commented Feb 28, 2019

With my latest
0401333
things look (and work) so much cleaner now
in migration mode during "rear recover" (excerpts):

Restoring finished (verify backup restore log messages in /var/lib/rear/restore/recover.backup.tar.gz.726.restore.log)
Recreating directories (with permissions) from /var/lib/rear/recovery/directories_permissions_owner_group
Migrating disk-by-id mappings in certain restored files in /mnt/local to current disk-by-id mappings ...
Migrating filesystem UUIDs in certain restored files in /mnt/local to current UUIDs ...
Patching filesystem UUIDs in boot/grub2/grub.cfg to current UUIDs
Patching filesystem UUIDs in etc/sysconfig/bootloader to current UUIDs
Skip patching symlink etc/mtab target /mnt/local/proc/5199/mounts on /proc/ /sys/ /dev/ or /run/
Patching filesystem UUIDs in etc/fstab to current UUIDs
Patching filesystem UUIDs in etc/mtools.conf to current UUIDs
Patching filesystem UUIDs in etc/smartd.conf to current UUIDs
Patching filesystem UUIDs in etc/sysconfig/smartmontools to current UUIDs
Patching filesystem UUIDs in etc/security/pam_mount.conf.xml to current UUIDs
Patching filesystem UUIDs in boot/efi/EFI/sles/grub.cfg to current UUIDs
Migrating network configuration files according to the mapping files ...
UserInput -I RESTORED_FILES_CONFIRMATION needed in /usr/share/rear/finalize/default/520_confirm_finalize.sh line 41
Confirm restored config files are OK or adapt them as needed
1) Confirm it is OK to recreate initrd and reinstall bootloader and continue 'rear recover'
2) Edit restored etc/fstab (/mnt/local/etc/fstab)
3) View restored etc/fstab (/mnt/local/etc/fstab)
4) Use Relax-and-Recover shell and return back to here
5) Abort 'rear recover'
(default '1' timeout 300 seconds)
UserInput: No real user input (empty or only spaces) - using default input
UserInput: Valid choice number result 'Confirm it is OK to recreate initrd and reinstall bootloader and continue 'rear recover''
User confirmed restored files
Running mkinitrd...
Recreated initrd (/sbin/mkinitrd).
Creating  EFI Boot Manager entry 'SUSE_LINUX 12.4' for 'EFI\sles\grubx64.efi' (UEFI_BOOTLOADER='/boot/efi/EFI/sles/grubx64.efi')
Finished recovering your system. You can explore it under '/mnt/local'.

FYI:
I used a somewhat sophisticated command to launch "rear recover"
with automated [Enter] responses (via plain 'echo') to all user dialogs, cf.
"It should be possible to run ReaR unattended" at
https://github.com/rear/rear/wiki/Coding-Style
plus having the output on the terminal also saved in a file 'rear_recover.out'
plus keeping my bash prompt by running all in the background in a subshell

# ( exec 0< <( while true ; do echo ; sleep 1 ; done ) ; rear -D recover | tee -a rear_recover.out ) &

because that 'while' loop (and the 'tee') run endlessly so that I won't be able
to just type reboot after 'rear recover' finished if it was not run in the background
(before the reboot I used scp to get 'rear_recover.out' out of the recovery system).

@jsmeix
Copy link
Member Author

jsmeix commented Feb 28, 2019

If there are no objections I would like to merge it today afternoon.

@jsmeix
Copy link
Member Author

jsmeix commented Feb 28, 2019

At least my browser shows here below

Some checks haven’t completed yet
1 pending check
continuous-integration/travis-ci/pr Pending — The Travis CI build is in progress [Details]

but the [Details] link
https://travis-ci.org/rear/rear/builds/499727007?utm_source=github_status&utm_medium=notification
shows (excerpts)

#2017 passed
    Ran for 40 sec
    about 3 hours ago

The command "make validate" exited with 0.
Done. Your build exited with 0.

so that I think actually all is o.k. and I can merge it now...

@jsmeix jsmeix merged commit ac2d7f4 into rear:master Feb 28, 2019
@jsmeix jsmeix deleted the skip_patching_absolute_symlinks_during_finalize_stage_issue1338 branch February 28, 2019 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do cleanup enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potentially patching wrong files during recovery
2 participants