Bind mount proc sys dev run at one place issue2045 #2047

jsmeix · 2019-02-18T15:48:36Z

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):

#2045
#2035
perhaps also
#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.
#2035 (comment)

…(i.e. after 110_bind_mount_proc_sys_dev_run.sh)

jsmeix · 2019-02-18T15:52:25Z

@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 · 2019-02-18T16:20:06Z

@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 · 2019-02-18T19:10:52Z

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 · 2019-02-19T09:11:44Z

@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 · 2019-02-19T09:56:57Z

@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) #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.

…e_uuid_tags.sh

…ze/Linux-ppc64/660_install_grub2.sh that oints to ../Linux-ppc64le/680_install_PPC_bootlist.sh to keep the scripts ordering in finalize/Linux-ppc64 as it was before

jsmeix · 2019-02-19T13:11:07Z

@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 · 2019-02-19T13:13:49Z

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

…comments

jsmeix · 2019-02-19T13:54:13Z

@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 · 2019-02-19T13:55:37Z

@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 · 2019-02-19T15:16:11Z

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 · 2019-02-20T09:10:06Z

@gozora
thank you for testing it again.
Your #2047 (comment)
shows that now 280_migrate_uuid_tags.sh behaves as intended.

… comments)

jsmeix · 2019-02-21T09:30:41Z

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

finalize/Linux-i386/620_install_grub2.sh had been moved to finalize/Linux-i386/660_install_grub2.sh via 6b1889c as a part of #2047

In finalize/Linux-ppc64le/660_install_grub2.sh remove the "mount --bind <proc|sys|dev> at TARGET_FS_ROOT" section because that is done in finalize/default/110_bind_mount_proc_sys_dev_run.sh since #2045 where finalize/Linux-ppc64le/..._install_grub2.sh was accidentally forgotten in #2047 therein see in particular dd9977f and therein see the change for finalize/Linux-i386/..._install_grub2.sh which we supplement now also for finalize/Linux-ppc64le/..._install_grub2.sh

In finalize/Linux-ppc64le/660_install_grub2.sh remove the "mount --bind <proc|sys|dev> at TARGET_FS_ROOT" section because that is done in finalize/default/110_bind_mount_proc_sys_dev_run.sh since rear#2045 where finalize/Linux-ppc64le/..._install_grub2.sh was accidentally forgotten in rear#2047 therein see in particular rear@dd9977f and therein see the change for finalize/Linux-i386/..._install_grub2.sh which we supplement now also for finalize/Linux-ppc64le/..._install_grub2.sh

jsmeix added 3 commits February 18, 2019 15:45

bind-mount /proc /sys /dev /run at one place (issue2045)

dd9977f

moved 100_EFISTUB_run_efibootmgr.sh to 150_EFISTUB_run_efibootmgr.sh …

dd07743

…(i.e. after 110_bind_mount_proc_sys_dev_run.sh)

mountpoint is now a required prog in the recovery system

d557b8e

jsmeix added enhancement Adaptions and new features bug The code does not do what it is meant to do cleanup labels Feb 18, 2019

jsmeix added this to the ReaR v2.5 milestone Feb 18, 2019

jsmeix self-assigned this Feb 18, 2019

jsmeix requested review from gdha, rmetrich, gozora and schabrolles February 18, 2019 15:48

gozora mentioned this pull request Feb 18, 2019

'rear recover' hangs while running mkinitrd (bind mount of /run missing in TARGET_FS_ROOT) #2035

Closed

gozora mentioned this pull request Feb 18, 2019

Do not patch $TARGET_FS_ROOT/etc/mtab if symbolic link. #2048

Closed

gdha approved these changes Feb 19, 2019

View reviewed changes

gozora approved these changes Feb 19, 2019

View reviewed changes

jsmeix added 2 commits February 19, 2019 11:28

renumbered bootloader install scripts and tried to improve 280_migrat…

6b1889c

…e_uuid_tags.sh

recreated symlink finalize/Linux-ppc64/620_install_grub2.sh as finali…

9cd9785

…ze/Linux-ppc64/660_install_grub2.sh that oints to ../Linux-ppc64le/680_install_PPC_bootlist.sh to keep the scripts ordering in finalize/Linux-ppc64 as it was before

fixed renumbered bootloader install script names where referenced in …

86191a5

…comments

minor cleanup in 280_migrate_uuid_tags.sh (local variables and better…

49ebd5e

… comments)

jsmeix mentioned this pull request Feb 20, 2019

630_run_efibootmgr.sh wrongly checks boot loader outside chroot #2051

Closed

fixed 670_run_efibootmgr.sh as in rear#2051

1bba7ff

jsmeix merged commit bf91ae9 into rear:master Feb 21, 2019

jsmeix deleted the bind_mount_proc_sys_dev_run_at_one_place_issue2045 branch February 21, 2019 12:39

jsmeix added the fixed / solved / done label Feb 21, 2019

This was referenced Feb 22, 2019

bind-mount proc sys dev and run at one central place in finalize stage #2045

Closed

WARNING: Failed to connect to lvmetad. Falling back to device scanning #2044

Closed

gozora mentioned this pull request Feb 26, 2019

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

Merged

jsmeix mentioned this pull request Apr 15, 2019

Fix for issue 2116: added new SUSE-specific script to call 'shim-inst… #2117

Merged

jsmeix added a commit that referenced this pull request Jun 7, 2022

Typo fix in default.conf comment

0f6c923

finalize/Linux-i386/620_install_grub2.sh had been moved to finalize/Linux-i386/660_install_grub2.sh via 6b1889c as a part of #2047

jsmeix mentioned this pull request Jan 10, 2023

Addendum for issue 2045 for Linux-ppc64le/660_install_grub2.sh #2910

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bind mount proc sys dev run at one place issue2045 #2047

Bind mount proc sys dev run at one place issue2045 #2047

jsmeix commented Feb 18, 2019 •

edited

Loading

jsmeix commented Feb 18, 2019

gozora commented Feb 18, 2019

gozora commented Feb 18, 2019

jsmeix commented Feb 19, 2019 •

edited

Loading

gozora commented Feb 19, 2019

jsmeix commented Feb 19, 2019 •

edited

Loading

jsmeix commented Feb 19, 2019

jsmeix commented Feb 19, 2019

jsmeix commented Feb 19, 2019

gozora commented Feb 19, 2019

jsmeix commented Feb 20, 2019

jsmeix commented Feb 21, 2019

Bind mount proc sys dev run at one place issue2045 #2047

Bind mount proc sys dev run at one place issue2045 #2047

Conversation

jsmeix commented Feb 18, 2019 • edited Loading

jsmeix commented Feb 18, 2019

gozora commented Feb 18, 2019

gozora commented Feb 18, 2019

jsmeix commented Feb 19, 2019 • edited Loading

gozora commented Feb 19, 2019

jsmeix commented Feb 19, 2019 • edited Loading

jsmeix commented Feb 19, 2019

jsmeix commented Feb 19, 2019

jsmeix commented Feb 19, 2019

gozora commented Feb 19, 2019

jsmeix commented Feb 20, 2019

jsmeix commented Feb 21, 2019

jsmeix commented Feb 18, 2019 •

edited

Loading

jsmeix commented Feb 19, 2019 •

edited

Loading

jsmeix commented Feb 19, 2019 •

edited

Loading