#2047 PR `merged`: Bind mount proc sys dev run at one place issue2045¶

Labels: enhancement, bug, cleanup, fixed / solved / done

jsmeix opened issue at 2019-02-18 15:48:¶

Type: Bug Fix / Enhancement / Cleanup
Impact: High

There might be regressions when installing old bootloaders
like GRUB1, LILO, and ELILO (none was tested, see below).

Reference to related issue (URL):

https://github.com/rear/rear/issues/2045
https://github.com/rear/rear/issues/2035
perhaps also
https://github.com/rear/rear/issues/2044

How was this pull request tested?

Works for me on SLES15 and SLES12 - both use GRUB2.
Other bootloaders (like GRUB, LILO, and ELILO) not tested.

Brief description of the changes in this pull request:

Now /proc /sys /dev and /run are bind-mounted
at the beginning of the finalize stage via one single new
finalize/default/110_bind_mount_proc_sys_dev_run.sh

All existing mount and umount commands in finalize scripts
for /proc /sys /dev and things like that were removed and
as needed such scripts were further adapted, in particular
finalize/GNU/Linux/430_create_multipath_config.sh
and finalize/Linux-i386/610_install_lilo.sh
and in finalize/Linux-i386/620_install_elilo.sh
a not yet fixed bug is marked as FIXME and left to be fixed
by someone who knows better than I about ELILO...

The old finalize/default/100_populate_dev.sh was removed
because what it intended to do is now done by the new
finalize/default/110_bind_mount_proc_sys_dev_run.sh

Because finalize/default/110_bind_mount_proc_sys_dev_run.sh
calls mountpoint that program is added to
REQUIRED_PROGS in default.conf, cf.
https://github.com/rear/rear/issues/2035#issuecomment-464773208

jsmeix commented at 2019-02-18 15:52:¶

@gozora
I moved your usr/share/rear/finalize/Linux-i386/100_EFISTUB_run_efibootmgr.sh
to usr/share/rear/finalize/Linux-i386/150_EFISTUB_run_efibootmgr.sh
i.e. after usr/share/rear/finalize/default/110_bind_mount_proc_sys_dev_run.sh

I think your EFISTUB_run_efibootmgr.sh should not use a 1nn number
in the finalize scripts because usr/share/rear/finalize/readme reads

000-099: initialization
100-199: creating devices on TARGET (/dev)
200-299: disk device migration
300-399: network device migration
400-499: Update some configuration on TARGET (like multipath)
500-599: rebuild initrd
600-699: bootloader installation 
700-799: 
800-899: Last Check
900-999: Remount all TARGET FS in /mnt/local + post-recovery (NBU)

i.e. the numbers 100-199 are meant for creating devices on TARGET
while the numbers 600-699 are meant for bootloader installation.

Is there a special reason why EFISTUB_run_efibootmgr.sh must
run so early during finalize stage?

gozora commented at 2019-02-18 16:20:¶

@jsmeix I've totally forgotten about usr/share/rear/finalize/readme :-(

Is there a special reason why EFISTUB_run_efibootmgr.sh must
run so early during finalize stage?

Excerpt from 100_EFISTUB_run_efibootmgr.sh

# This script should be triggered BEFORE any other boot loader installation scripts
# in this directory. If EFI_STUB is enabled, we will automatically set
# NOBOOTLOADER variable empty to avoid other boot loader installation attempts,
# which will most probably fail (due missing binaries on original system).
# If creation process of boot entry later fails, we will just print information
# message to user to create boot entry manually, because ending restore process
# with error in such late stage is not desirable.

So you can update its name to 609_EFISTUB_run_efibootmgr.sh without breaking anything.

V.

gozora commented at 2019-02-18 19:10:¶

Hello @jsmeix

This patch have uncovered problem.
Namely with finalize/GNU/Linux/280_migrate_uuid_tags.sh. It looks like it benefited from missing /proc.

More precisely following condition:

...
[[ ! -f "$file" ]] && continue # skip directory and file not found
...

will evaluate true when $file=<symlink_pointing_to_proc> or more specifically in my case:

arch-efi:(/root)(root)# ll /etc/mtab 
lrwxrwxrwx 1 root root 19 Dec  6 15:19 /etc/mtab -> ../proc/self/mounts

So in current ReaR master code (without this patch merged) condition will evaluate true (because /proc is not mounted) and happily stops current loop iteration and continues with next one.

However since your patch will correctly mount /proc, condition will evaluate as false, and remaining code in loop iteration will execute which fails with:

++ for file in [b]oot/{grub.conf,menu.lst,device.map} [e]tc/grub.* [b]oot/grub/{grub.conf,grub.cfg,menu.lst,device.map} [b]oot/grub2/{grub.conf,grub.cfg,menu.lst,device.map} [e]tc/sysconfig/grub [e]tc/sysconfig/bootloader [e]tc/lilo.conf [e]tc/elilo.conf [e]tc/mtab [e]tc/fstab [e]tc/mtools.conf [e]tc/smartd.conf [e]tc/sysconfig/smartmontools [e]tc/sysconfig/rawdevices [e]tc/security/pam_mount.conf.xml [b]oot/efi/*/*/grub.cfg
++ [[ ! -f etc/mtab ]]
++ test -L etc/mtab
+++ readlink -f etc/mtab
++ linkdest=/mnt/local/proc/2256/mounts
++ echo /mnt/local/proc/2256/mounts
++ grep -q '^/proc'
++ LogPrint 'Patching '\''/mnt/local/proc/2256/mounts'\'' instead of '\''etc/mtab'\'''
++ Log 'Patching '\''/mnt/local/proc/2256/mounts'\'' instead of '\''etc/mtab'\'''
++ echo '2019-02-18 19:24:36.242714650 Patching '\''/mnt/local/proc/2256/mounts'\'' instead of '\''etc/mtab'\'''
2019-02-18 19:24:36.242714650 Patching '/mnt/local/proc/2256/mounts' instead of 'etc/mtab'
++ Print 'Patching '\''/mnt/local/proc/2256/mounts'\'' instead of '\''etc/mtab'\'''
++ file=/mnt/local/proc/2256/mounts
++ LogPrint 'Patching file '\''/mnt/local/proc/2256/mounts'\'''
++ Log 'Patching file '\''/mnt/local/proc/2256/mounts'\'''
++ echo '2019-02-18 19:24:36.244160117 Patching file '\''/mnt/local/proc/2256/mounts'\'''
2019-02-18 19:24:36.244160117 Patching file '/mnt/local/proc/2256/mounts'
++ Print 'Patching file '\''/mnt/local/proc/2256/mounts'\'''
++ sed -i ';/2255-403B/s/2255-403B/84F7-09EC/g' /mnt/local/proc/2256/mounts
sed: can't read /mnt/local/proc/2256/mounts: No such file or directory
++ StopIfError 'Patching '\''/mnt/local/proc/2256/mounts'\'' with sed failed.'
++ ((  2 != 0  ))
++ Error 'Patching '\''/mnt/local/proc/2256/mounts'\'' with sed failed.'
++ PrintError 'ERROR: Patching '\''/mnt/local/proc/2256/mounts'\'' with sed failed.'
++ PrintError 'Some latest log messages since the last called script 280_migrate_uuid_tags.sh:'
++ PrintError '  2019-02-18 19:24:36.230447973 Including finalize/GNU/Linux/280_migrate_uuid_tags.sh

For completeness, current ReaR master code behaves like this:

++ for file in [b]oot/{grub.conf,menu.lst,device.map} [e]tc/grub.* [b]oot/grub/{grub.conf,grub.cfg,menu.lst,device.map} [b]oot/grub2/{grub.conf,grub.cfg,menu.lst,device.map} [e]tc/sysconfig/grub [e]tc/sysconfig/bootloader [e]tc/lilo.conf [e]tc/elilo.conf [e]tc/mtab [e]tc/fstab [e]tc/mtools.conf [e]tc/smartd.conf [e]tc/sysconfig/smartmontools [e]tc/sysconfig/rawdevices [e]tc/security/pam_mount.conf.xml [b]oot/efi/*/*/grub.cfg
++ [[ ! -f etc/mtab ]]
++ continue

Some time ago /etc/mtab changed from regular file to symlink pointing /proc/mtab resp /proc/self/mtab and since this change we should not anyhow touch this file and poking aroud /proc with sed (I think that using sed -i in /proc is not even possible).
Migrating /etc/mtab with 280_migrate_uuid_tags.sh should be done only on OLD systems where /etc/mtab is actual file.

Your patch is fully OK, but if it will be merged other things might stop working because your patch actually fixed something :-) (what an irony)!

V.

jsmeix commented at 2019-02-19 09:11:¶

@gozora
thank you so much for finding the issue
with finalize/GNU/Linux/280_migrate_uuid_tags.sh
I will fix that too.

Regarding NNN_EFISTUB_run_efibootmgr.sh
I cannot have it as 609_EFISTUB_run_efibootmgr.sh
because we have already 600_install_elilo.sh and
600_install_yaboot.sh and 600_restore_arm_bootloader.sh
so that I need to do some more re-numbering in this area...

gozora commented at 2019-02-19 09:56:¶

@jsmeix

thank you so much for finding the issue
with finalize/GNU/Linux/280_migrate_uuid_tags.sh
I will fix that too.

Anytime!
If you decide for essential rework of finalize/GNU/Linux/280_migrate_uuid_tags.sh just void my (rather simplistic workaround) https://github.com/rear/rear/pull/2048 ;-)

Regarding NNN_EFISTUB_run_efibootmgr.sh
I cannot have it as 609_EFISTUB_run_efibootmgr.sh
because we have already 600_install_elilo.sh and
600_install_yaboot.sh and 600_restore_arm_bootloader.sh
so that I need to do some more re-numbering in this area...

I guess we can use 609_EFISTUB_run_efibootmgr.sh after all. Because files from Linux-i386, Linux-ppc64, Linux-arm, Linux-ia64 or Linux-ppc64le directories will never execute simultaneously.

V.

jsmeix commented at 2019-02-19 13:11:¶

@gozora
could you please re-test if now finalize/GNU/Linux/280_migrate_uuid_tags.sh
behaves better in your case?

I don't know how to trigger that 280_migrate_uuid_tags.sh gets actually run.
On my SLES15 and SLES12 systems I do not get a $FS_UUID_MAP file
so 280_migrate_uuid_tags.sh returns immediately in my case.

jsmeix commented at 2019-02-19 13:13:¶

Only FYI
how I tested the changes in this pull request here:

# git clone https://github.com/jsmeix/rear.git

# mv rear rear.jsmeix

# cd rear.jsmeix

# git checkout remotes/origin/bind_mount_proc_sys_dev_run_at_one_place_issue2045

jsmeix commented at 2019-02-19 13:54:¶

@schabrolles
I would much appreciate it if you could verify on POWER architecture
that this changes here do not cause obvious regressions
(I had to renumber all the bootloader install scripts).

jsmeix commented at 2019-02-19 13:55:¶

@rmetrich
I would much appreciate it if you could verify on Red Hat systems
that this changes here do not cause obvious regressions
in particular on older Red Hat systems that do not use GRUB2
or that do not have /run mounted or things like that...

gozora commented at 2019-02-19 15:16:¶

Hello @jsmeix

Works fine for me!

Log from running rear recover says:

++ test -f etc/mtab
++ test -L etc/mtab
+++ readlink -f etc/mtab
++ symlink_target=/mnt/local/proc/2303/mounts
++ echo /mnt/local/proc/2303/mounts
++ egrep -q '/proc/|/sys/|/dev/|/run/'
++ LogPrint 'Skip patching symlink etc/mtab target /mnt/local/proc/2303/mounts on /proc/ /sys/ /dev/ or /run/'
++ Log 'Skip patching symlink etc/mtab target /mnt/local/proc/2303/mounts on /proc/ /sys/ /dev/ or /run/'

V.

jsmeix commented at 2019-02-20 09:10:¶

@gozora
thank you for testing it again.
Your https://github.com/rear/rear/pull/2047#issuecomment-465171193
shows that now 280_migrate_uuid_tags.sh behaves as intended.

jsmeix commented at 2019-02-21 09:30:¶

@schabrolles @rmetrich
if you do not object I will merge it today afternoon.

[Export of Github issue for rear/rear.]

#2047 PR merged: Bind mount proc sys dev run at one place issue2045¶