Merge pull request #1171 from jsmeix/remove_needless_bash_from_chroot…

…_calls_issue862 Removed needless login shell from chroot calls where possible
rear · Jan 20, 2017 · 3baf9eb · 3baf9eb
2 parents 547c17b + caa3de0
commit 3baf9eb
Show file tree

Hide file tree

Showing 8 changed files with 187 additions and 194 deletions.
diff --git a/usr/share/rear/finalize/Debian/i386/170_rebuild_initramfs.sh b/usr/share/rear/finalize/Debian/i386/170_rebuild_initramfs.sh
@@ -1,7 +1,10 @@
 # rebuild the initramfs if the drivers changed
 #
-# probably not required, but I prefer to rely on this information when it
-# is backed by udev
+# probably not required, but I prefer to rely on this information when it is backed by udev
+# FIXME: who is 'I'?
+# Perhaps Schlomo Schapiro or someone who made the "P2V patch from Heinlein Support"?
+# (see commit 844d50b75ac4b7722f4fee7a5ee3350b93f3adb7)
+# And what happens if there is no 'have_udev'? Why is everything o.k. then to just 'return 0'?
 have_udev || return 0
 
 # check if we need to do something
@@ -35,14 +38,34 @@ update-initramfs afterwards to update the initramfs with the new mdadm.conf
 		fi
 	fi
 
-	if chroot $TARGET_FS_ROOT /bin/bash --login -c "update-initramfs -v -u -k all" >&2 ; then
-		LogPrint "Updated initramfs with new drivers for this system."
-	else
-		LogPrint "WARNING !!!
-initramfs creation failed, please check '$RUNTIME_LOGFILE' to see the error
-messages in detail and decide yourself, whether the system will boot or not.
+        # Run update-initramfs directly in chroot without a login shell in between (see https://github.com/rear/rear/issues/862).
+        # We need the update-initramfs binary in the chroot environment i.e. the update-initramfs binary in the recreated system.
+        # Normally we would use a login shell like: chroot $TARGET_FS_ROOT /bin/bash --login -c 'type -P update-initramfs'
+        # because otherwise there is no useful PATH (PATH is only /bin) so that 'type -P' won't find it
+        # but we cannot use a login shell because that contradicts https://github.com/rear/rear/issues/862
+        # so that we use a plain (non-login) shell and set a (hopefully) reasonable PATH:
+        local update_initramfs_binary=$( chroot $TARGET_FS_ROOT /bin/bash -c 'PATH=/sbin:/usr/sbin:/usr/bin:/bin type -P update-initramfs' )
+        # If there is no update-initramfs in the chroot environment plain 'chroot $TARGET_FS_ROOT' will hang up endlessly
+        # and then "rear recover" cannot be aborted with the usual [Ctrl]+[C] keys.
+        # Use plain $var because when var contains only blanks test "$var" results true because test " " results true:
+        if test $update_initramfs_binary ; then
+            if chroot $TARGET_FS_ROOT $update_initramfs_binary -v -u -k all >&2 ; then
+                LogPrint "Updated initramfs with new drivers for this system."
+            else
+                LogPrint "WARNING:
+Failed to create initramfs ($update_initramfs_binary).
+Check '$RUNTIME_LOGFILE' to see the error messages in detail
+and decide yourself, whether the system will boot or not.
 "
-	fi
-	umount $TARGET_FS_ROOT/proc $TARGET_FS_ROOT/sys
+            fi
+        else
+        LogPrint "WARNING:
+Cannot create initramfs (found no update-initramfs in the recreated system).
+Check the recreated system (mounted at $TARGET_FS_ROOT)
+and decide yourself, whether the system will boot or not.
+"
+        fi
+        umount $TARGET_FS_ROOT/proc $TARGET_FS_ROOT/sys
 
 fi
+
diff --git a/usr/share/rear/finalize/Fedora/i386/170_rebuild_initramfs.sh b/usr/share/rear/finalize/Fedora/i386/170_rebuild_initramfs.sh
@@ -1,7 +1,10 @@
 # rebuild the initramfs if the drivers changed
 #
-# probably not required, but I prefer to rely on this information when it
-# is backed by udev
+# probably not required, but I prefer to rely on this information when it is backed by udev
+# FIXME: who is 'I'?
+# Perhaps Schlomo Schapiro or someone who made the "P2V patch from Heinlein Support"?
+# (see commit 844d50b75ac4b7722f4fee7a5ee3350b93f3adb7)
+# And what happens if there is no 'have_udev'? Why is everything o.k. then to just 'return 0'?
 have_udev || return 0
 
 # check if we need to do something
@@ -49,23 +52,40 @@ if test -s $TMP_DIR/storage_drivers && ! diff $TMP_DIR/storage_drivers $VAR_DIR/
         unalias ls 2>/dev/null
 
         for INITRD_IMG in $( ls $TARGET_FS_ROOT/boot/initramfs-*.img $TARGET_FS_ROOT/boot/initrd-*.img | egrep -v '(kdump|rescue|plymouth)' ) ; do
-            # do not use KERNEL_VERSION here because that is readonly in the rear main script:
+            # Do not use KERNEL_VERSION here because that is readonly in the rear main script:
             kernel_version=$( basename $( echo $INITRD_IMG ) | cut -f2- -d"-" | sed s/"\.img"// )
-            INITRD=$( echo $INITRD_IMG|egrep -o "/boot/.*" )
-
-            echo "Running mkinitrd..."
-            if chroot $TARGET_FS_ROOT /bin/bash --login -c "mkinitrd -v -f ${WITH_INITRD_MODULES[@]} $INITRD $kernel_version" >&2 ; then
-                LogPrint "Updated initramfs with new drivers for Kernel $kernel_version."
+            INITRD=$( echo $INITRD_IMG | egrep -o "/boot/.*" )
+            LogPrint "Running mkinitrd..."
+            # Run mkinitrd directly in chroot without a login shell in between (see https://github.com/rear/rear/issues/862).
+            # We need the mkinitrd binary in the chroot environment i.e. the mkinitrd binary in the recreated system.
+            # Normally we would use a login shell like: chroot $TARGET_FS_ROOT /bin/bash --login -c 'type -P mkinitrd'
+            # because otherwise there is no useful PATH (PATH is only /bin) so that 'type -P' won't find it
+            # but we cannot use a login shell because that contradicts https://github.com/rear/rear/issues/862
+            # so that we use a plain (non-login) shell and set a (hopefully) reasonable PATH:
+            local mkinitrd_binary=$( chroot $TARGET_FS_ROOT /bin/bash -c 'PATH=/sbin:/usr/sbin:/usr/bin:/bin type -P mkinitrd' )
+            # If there is no mkinitrd in the chroot environment plain 'chroot $TARGET_FS_ROOT' will hang up endlessly
+            # and then "rear recover" cannot be aborted with the usual [Ctrl]+[C] keys.
+            # Use plain $var because when var contains only blanks test "$var" results true because test " " results true:
+            if test $mkinitrd_binary ; then
+                if chroot $TARGET_FS_ROOT $mkinitrd_binary -v -f ${WITH_INITRD_MODULES[@]} $INITRD $kernel_version >&2 ; then
+                    LogPrint "Updated initrd with new drivers for kernel $kernel_version."
+                else
+                    LogPrint "WARNING:
+Failed to create initrd for kernel version '$kernel_version'.
+Check '$RUNTIME_LOGFILE' to see the error messages in detail
+and decide yourself, whether the system will boot or not.
+"
+                fi
             else
-                LogPrint "WARNING !!!
-initramfs creation for Kernel version '$kernel_version' failed,
-please check '$RUNTIME_LOGFILE' to see the error messages in detail
+                LogPrint "WARNING:
+Cannot create initrd (found no mkinitrd in the recreated system).
+Check the recreated system (mounted at $TARGET_FS_ROOT)
 and decide yourself, whether the system will boot or not.
 "
             fi
-
         done
 
 	umount $TARGET_FS_ROOT/proc $TARGET_FS_ROOT/sys
 
 fi
+
diff --git a/usr/share/rear/finalize/Linux-ppc64/200_install_yaboot.sh b/usr/share/rear/finalize/Linux-ppc64/200_install_yaboot.sh
@@ -1,34 +1,32 @@
+
 # skip if yaboot conf is not found
-if test ! -f $TARGET_FS_ROOT/etc/yaboot.conf; then
-  return
-fi
+test -f $TARGET_FS_ROOT/etc/yaboot.conf || return
 
 # Reinstall yaboot boot loader
 LogPrint "Installing PPC PReP Boot partition."
 
 # Find PPC PReP Boot partitions
 part=$( awk -F '=' '/^boot=/ {print $2}' $TARGET_FS_ROOT/etc/yaboot.conf )
 
-if [ -n "$part" ]; then
-  LogPrint "Boot partion found: $part"
-  chroot $TARGET_FS_ROOT /bin/bash --login -c "/sbin/mkofboot -b $part --filesystem raw -f"
-  bootdev=$( echo $part | sed -e 's/[0-9]*$//' )
-  LogPrint "Boot device is $bootdev."
-  bootlist -m normal $bootdev
-  NOBOOTLOADER=
+if test "$part" ; then
+    LogPrint "Boot partion found: $part"
+    # Run mkofboot directly in chroot without a login shell in between, see https://github.com/rear/rear/issues/862
+    chroot $TARGET_FS_ROOT /sbin/mkofboot -b $part --filesystem raw -f
+    bootdev=$( echo $part | sed -e 's/[0-9]*$//' )
+    LogPrint "Boot device is $bootdev."
+    bootlist -m normal $bootdev
+    NOBOOTLOADER=
 else
-  bootparts=$( sfdisk -l 2>&1 | awk '/PPC PReP Boot/ {print $1}' )
-  LogPrint "Boot partitions found: $bootparts."
-  for part in $bootparts
-  do
-    LogPrint "Initializing boot partition $part."
-    chroot $TARGET_FS_ROOT /bin/bash --login -c "/sbin/mkofboot -b $part --filesystem raw -f"
-  done
-  bootdev=$( for part in $bootparts
-           do
-             echo $part | sed -e 's/[0-9]*$//'
-         done | sort | uniq )
-  LogPrint "Boot device list is $bootdev."
-  bootlist -m normal $bootdev
-  NOBOOTLOADER=
+    bootparts=$( sfdisk -l 2>&1 | awk '/PPC PReP Boot/ {print $1}' )
+    LogPrint "Boot partitions found: $bootparts."
+    for part in $bootparts ; do
+        LogPrint "Initializing boot partition $part."
+        # Run mkofboot directly in chroot without a login shell in between, see https://github.com/rear/rear/issues/862
+        chroot $TARGET_FS_ROOT /sbin/mkofboot -b $part --filesystem raw -f
+    done
+    bootdev=$( for part in $bootparts ; do echo $part | sed -e 's/[0-9]*$//' ; done | sort | uniq )
+    LogPrint "Boot device list is $bootdev."
+    bootlist -m normal $bootdev
+    NOBOOTLOADER=
 fi
+
diff --git a/usr/share/rear/finalize/Linux-ppc64/220_install_grub2.sh b/usr/share/rear/finalize/Linux-ppc64/220_install_grub2.sh
@@ -43,7 +43,8 @@ if [[ -r "$LAYOUT_FILE" ]]; then
     if [ -n "$part" ]; then
         LogPrint "Boot partition found: $part"
         dd if=/dev/zero of=$part
-        chroot $TARGET_FS_ROOT /bin/bash --login -c "$grub_name-install $part"
+        # Run grub-install/grub2-install directly in chroot without a login shell in between, see https://github.com/rear/rear/issues/862
+        chroot $TARGET_FS_ROOT $grub_name-install $part
         # Run bootlist only in PowerVM environment
         if ! grep -q "PowerNV" /proc/cpuinfo && ! grep -q "emulated by qemu" /proc/cpuinfo ; then
             #Using $LAYOUT_DEPS file to find the disk device containing the partition.

diff --git a/usr/share/rear/finalize/Linux-ppc64le/220_install_grub2.sh b/usr/share/rear/finalize/Linux-ppc64le/220_install_grub2.sh
diff --git a/usr/share/rear/finalize/Linux-ppc64le/220_install_grub2.sh b/usr/share/rear/finalize/Linux-ppc64le/220_install_grub2.sh
@@ -0,0 +1 @@
+../Linux-ppc64/220_install_grub2.sh
diff --git a/usr/share/rear/finalize/SUSE_LINUX/i386/170_rebuild_initramfs.sh b/usr/share/rear/finalize/SUSE_LINUX/i386/170_rebuild_initramfs.sh
@@ -1,18 +1,21 @@
 # rebuild the initramfs if the drivers changed
 #
-# probably not required, but I prefer to rely on this information when it
-# is backed by udev
+# probably not required, but I prefer to rely on this information when it is backed by udev
+# FIXME: who is 'I'?
+# Perhaps Schlomo Schapiro or someone who made the "P2V patch from Heinlein Support"?
+# (see commit 844d50b75ac4b7722f4fee7a5ee3350b93f3adb7)
+# And what happens if there is no 'have_udev'? Why is everything o.k. then to just 'return 0'?
 have_udev || return 0
 
 # check if we need to do something
 if test -s $TMP_DIR/storage_drivers && ! diff $TMP_DIR/storage_drivers $VAR_DIR/recovery/storage_drivers >&8 ; then
-	# remember, diff returns 0 if the files are the same
+    # remember, diff returns 0 if the files are the same
 
-	# merge new drivers with previous initrd modules
-	# BUG: we only add modules to the initrd, we don't take old ones out
-	#      could be done better, but might not be worth the risk
+    # merge new drivers with previous initrd modules
+    # BUG: we only add modules to the initrd, we don't take old ones out
+    #      could be done better, but might not be worth the risk
 
-	# set INITRD_MODULES from recovered system
+   # set INITRD_MODULES from recovered system
    if test -r $TARGET_FS_ROOT/etc/sysconfig/kernel
    then # In SLE12 RC2 /etc/sysconfig/kernel is an useless stub that contains only one line
         #   INITRD_MODULES=""
@@ -28,39 +31,59 @@ if test -s $TMP_DIR/storage_drivers && ! diff $TMP_DIR/storage_drivers $VAR_DIR/
         #   # and the dracut (--force-drivers paramter) manpage.
         # Because the comment above reads "probably not required" at least for now
         # there is no support for force_drivers in /etc/dracut.conf.d/01-dist.conf.
-	source $TARGET_FS_ROOT/etc/sysconfig/kernel
-	StopIfError "Could not source '$TARGET_FS_ROOT/etc/sysconfig/kernel'"
+        source $TARGET_FS_ROOT/etc/sysconfig/kerneli || Error "Could not source '$TARGET_FS_ROOT/etc/sysconfig/kernel'"
 
-	Log "Original INITRD_MODULES='$INITRD_MODULES'"
-	OLD_INITRD_MODULES=( $INITRD_MODULES ) # use array to split into words
-	# To see what has been added by the migration process, the new modules are added to the
-	# end of the list. To achieve this, we list the old modules twice in the variable
-	# NEW_INITRD_MODULES and then add the new modules. Then we use "uniq -u" to filter out
-	# the modules which only appear once in the list. The result array the only
-	# contains the new modules.
-	NEW_INITRD_MODULES=( $INITRD_MODULES $INITRD_MODULES $( cat $TMP_DIR/storage_drivers ) )
+        Log "Original INITRD_MODULES='$INITRD_MODULES'"
+        OLD_INITRD_MODULES=( $INITRD_MODULES ) # use array to split into words
+        # To see what has been added by the migration process, the new modules are added to the
+        # end of the list. To achieve this, we list the old modules twice in the variable
+        # NEW_INITRD_MODULES and then add the new modules. Then we use "uniq -u" to filter out
+        # the modules which only appear once in the list. The result array the only
+        # contains the new modules.
+        NEW_INITRD_MODULES=( $INITRD_MODULES $INITRD_MODULES $( cat $TMP_DIR/storage_drivers ) )
 
-	# uniq INITRD_MODULES
+        # uniq INITRD_MODULES
 
-	NEW_INITRD_MODULES=( $(tr " " "\n" <<< "${NEW_INITRD_MODULES[*]}" | sort | uniq -u) )
+        NEW_INITRD_MODULES=( $( tr " " "\n" <<< "${NEW_INITRD_MODULES[*]}" | sort | uniq -u ) )
 
-	Log "New INITRD_MODULES='${OLD_INITRD_MODULES[@]} ${NEW_INITRD_MODULES[@]}'"
+        Log "New INITRD_MODULES='${OLD_INITRD_MODULES[@]} ${NEW_INITRD_MODULES[@]}'"
 
-	sed -i -e '/^INITRD_MODULES/s/^.*$/#&\nINITRD_MODULES="'"${OLD_INITRD_MODULES[*]} ${NEW_INITRD_MODULES[*]}"'"/' $TARGET_FS_ROOT/etc/sysconfig/kernel
-   fi
-	my_udevtrigger
-	sleep 5
-	mount -t proc none $TARGET_FS_ROOT/proc
-	mount -t sysfs none $TARGET_FS_ROOT/sys
-        echo "Running mkinitrd..."
-	if chroot $TARGET_FS_ROOT /bin/bash --login -c "mkinitrd" >&2 ; then
-		LogPrint "Recreated initramfs (mkinitrd)."
-	else
-		LogPrint "WARNING !!!
-initramfs creation (mkinitrd) failed, please check '$RUNTIME_LOGFILE' to see the error
-messages in detail and decide yourself, whether the system will boot or not.
+        sed -i -e '/^INITRD_MODULES/s/^.*$/#&\nINITRD_MODULES="'"${OLD_INITRD_MODULES[*]} ${NEW_INITRD_MODULES[*]}"'"/' $TARGET_FS_ROOT/etc/sysconfig/kernel
+    fi
+
+    my_udevtrigger
+    sleep 5
+    mount -t proc none $TARGET_FS_ROOT/proc
+    mount -t sysfs none $TARGET_FS_ROOT/sys
+    LogPrint "Running mkinitrd..."
+    # Run mkinitrd directly in chroot without a login shell in between (see https://github.com/rear/rear/issues/862).
+    # We need the mkinitrd binary in the chroot environment i.e. the mkinitrd binary in the recreated system.
+    # Normally we would use a login shell like: chroot $TARGET_FS_ROOT /bin/bash --login -c 'type -P mkinitrd'
+    # because otherwise there is no useful PATH (PATH is only /bin) so that 'type -P' won't find it
+    # but we cannot use a login shell because that contradicts https://github.com/rear/rear/issues/862
+    # so that we use a plain (non-login) shell and set a (hopefully) reasonable PATH:
+    local mkinitrd_binary=$( chroot $TARGET_FS_ROOT /bin/bash -c 'PATH=/sbin:/usr/sbin:/usr/bin:/bin type -P mkinitrd' )
+    # If there is no mkinitrd in the chroot environment plain 'chroot $TARGET_FS_ROOT' will hang up endlessly
+    # and then "rear recover" cannot be aborted with the usual [Ctrl]+[C] keys.
+    # Use plain $var because when var contains only blanks test "$var" results true because test " " results true:
+    if test $mkinitrd_binary ; then
+        if chroot $TARGET_FS_ROOT $mkinitrd_binary >&2 ; then
+            LogPrint "Recreated initrd ($mkinitrd_binary)."
+        else
+            LogPrint "WARNING:
+Failed to create initrd ($mkinitrd_binary).
+Check '$RUNTIME_LOGFILE' to see the error messages in detail
+and decide yourself, whether the system will boot or not.
+"
+        fi
+    else
+        LogPrint "WARNING:
+Cannot create initrd (found no mkinitrd in the recreated system).
+Check the recreated system (mounted at $TARGET_FS_ROOT)
+and decide yourself, whether the system will boot or not.
 "
-	fi
-	umount $TARGET_FS_ROOT/proc $TARGET_FS_ROOT/sys
+    fi
+    umount $TARGET_FS_ROOT/proc $TARGET_FS_ROOT/sys
 
 fi
+