[patch 0/6] Revert action_on_fail patches

[patch 0/6] Revert action_on_fail patches

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

Hi,

action_on_fail was introduced originally for kdump error handling.
Before the patches, dracut will drop into emergency shell for many
failure cases, but kdump does not care about some of failures as
long as vmcore can be saved successfully. Thus action_on_fail was
introduced to fall to kdump service in case early failure in dracut.

But later with new systemd services, there are cases that kdump
script can never be called in case early dracut failure because of
systemd service dependency issues.

Thus there was below post which create a new kdump error handler
systemd service:
https://lists.stg.fedoraproject.org/archives/list/kexec-TuqUDEhatI4ANWPb/1PvSmm0pvjS0E/A@public.gmane.org/thread/3EN63K2NDC6C545M5Z434VCHWDR3BCHD/

Now this action_on_fail interface was useless, nobody use it and
it is broken with systemd sometimes. So I send this series to revert
them one by one.

Thanks
Dave

[patch 1/6] [PATCH 1/5] Revert "Change the implementation of action_on_fail"

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

[-- Attachment #1: 0001-Revert-Change-the-implementation-of-action_on_fail.patch --]
[-- Type: text/plain, Size: 3351 bytes --]

This reverts commit d2765b5175663d094737d6819cc3f3df53e7a4cb.

action_on_fail is useless thus remove the commits for it one by one.

Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 dracut.cmdline.7.asc                               |  4 ++++
 .../98dracut-systemd/dracut-emergency.service      |  2 +-
 modules.d/98dracut-systemd/emergency.service       |  2 +-
 modules.d/99base/dracut-lib.sh                     | 28 ++++++++++++++--------
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/dracut.cmdline.7.asc b/dracut.cmdline.7.asc
index 8c7e965..07d3a93 100644
--- a/dracut.cmdline.7.asc
+++ b/dracut.cmdline.7.asc
@@ -159,6 +159,10 @@ Misc
    specify the controlling terminal for the console.
    This is useful, if you have multiple "console=" arguments.
 
+**rd.action_on_fail=**_{shell|continue}_::
+   Specify the action after failure. By default it's emergency_shell.
+   'continue' means: ignore the current failure and go ahead.
+
 [[dracutkerneldebug]]
 Debug
 ~~~~~
diff --git a/modules.d/98dracut-systemd/dracut-emergency.service b/modules.d/98dracut-systemd/dracut-emergency.service
index 5a6d525..a4b81bc 100644
--- a/modules.d/98dracut-systemd/dracut-emergency.service
+++ b/modules.d/98dracut-systemd/dracut-emergency.service
@@ -13,7 +13,7 @@ DefaultDependencies=no
 After=systemd-vconsole-setup.service
 Wants=systemd-vconsole-setup.service
 Conflicts=emergency.service emergency.target
-ConditionPathExists=!/lib/dracut/no-emergency-shell
+ConditionKernelCommandLine=!action_on_fail=continue
 
 [Service]
 Environment=HOME=/
diff --git a/modules.d/98dracut-systemd/emergency.service b/modules.d/98dracut-systemd/emergency.service
index 5f1eaa2..c19fe37 100644
--- a/modules.d/98dracut-systemd/emergency.service
+++ b/modules.d/98dracut-systemd/emergency.service
@@ -12,7 +12,7 @@ Description=Emergency Shell
 DefaultDependencies=no
 After=systemd-vconsole-setup.service
 Wants=systemd-vconsole-setup.service
-ConditionPathExists=!/lib/dracut/no-emergency-shell
+ConditionKernelCommandLine=!action_on_fail=continue
 
 [Service]
 Environment=HOME=/
diff --git a/modules.d/99base/dracut-lib.sh b/modules.d/99base/dracut-lib.sh
index 1eaf4b3..1f50e2f 100755
--- a/modules.d/99base/dracut-lib.sh
+++ b/modules.d/99base/dracut-lib.sh
@@ -1145,16 +1145,24 @@ emergency_shell()
 
 action_on_fail()
 {
-    if [ -f "$initdir/lib/dracut/no-emergency-shell" ]; then
-        [ "$1" = "-n" ] && shift 2
-        [ "$1" = "--shutdown" ] && shift 2
-        warn "$*"
-        warn "Not dropping to emergency shell, because $initdir/lib/dracut/no-emergency-shell exists."
-        return 0
-    fi
-
-    emergency_shell $@
-    return 1
+    local _action=$(getarg rd.action_on_fail= -d action_on_fail=)
+    case "$_action" in
+        continue)
+            [ "$1" = "-n" ] && shift 2
+            [ "$1" = "--shutdown" ] && shift 2
+            warn "$*"
+            warn "Not dropping to emergency shell, because 'action_on_fail=continue' was set on the kernel command line."
+            return 0
+            ;;
+        shell)
+            emergency_shell $@
+            return 1
+            ;;
+        *)
+            emergency_shell $@
+            return 1
+            ;;
+    esac
 }
 
 # Retain the values of these variables but ensure that they are unexported
-- 
1.8.3.1

[patch 2/6] [PATCH 2/5] Revert "rename kernel command line param action_on_fail to rd.action_on_fail"

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

[-- Attachment #1: 0002-Revert-rename-kernel-command-line-param-action_on_fa.patch --]
[-- Type: text/plain, Size: 1300 bytes --]

This reverts commit 391d6057908be2fe78f8084eb1b2adc27c781c0b.

action_on_fail is useless thus remove the commits for it one by one.

Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 dracut.cmdline.7.asc           | 2 +-
 modules.d/99base/dracut-lib.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dracut.cmdline.7.asc b/dracut.cmdline.7.asc
index 07d3a93..2e368ad 100644
--- a/dracut.cmdline.7.asc
+++ b/dracut.cmdline.7.asc
@@ -159,7 +159,7 @@ Misc
    specify the controlling terminal for the console.
    This is useful, if you have multiple "console=" arguments.
 
-**rd.action_on_fail=**_{shell|continue}_::
+**action_on_fail=**_{shell|continue}_::
    Specify the action after failure. By default it's emergency_shell.
    'continue' means: ignore the current failure and go ahead.
 
diff --git a/modules.d/99base/dracut-lib.sh b/modules.d/99base/dracut-lib.sh
index 1f50e2f..c119018 100755
--- a/modules.d/99base/dracut-lib.sh
+++ b/modules.d/99base/dracut-lib.sh
@@ -1145,7 +1145,7 @@ emergency_shell()
 
 action_on_fail()
 {
-    local _action=$(getarg rd.action_on_fail= -d action_on_fail=)
+    local _action=$(getarg action_on_fail=)
     case "$_action" in
         continue)
             [ "$1" = "-n" ] && shift 2
-- 
1.8.3.1

[patch 3/6] [PATCH 3/5] Revert "systemd/emergency.service: do not run for action_on_fail=continue"

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

[-- Attachment #1: 0003-Revert-systemd-emergency.service-do-not-run-for-acti.patch --]
[-- Type: text/plain, Size: 779 bytes --]

This reverts commit dcae873414ff643e1de790f256e414923e2aef8b.

action_on_fail is useless thus remove the commits for it one by one.

Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 modules.d/98dracut-systemd/emergency.service | 1 -
 1 file changed, 1 deletion(-)

diff --git a/modules.d/98dracut-systemd/emergency.service b/modules.d/98dracut-systemd/emergency.service
index c19fe37..35d9c31 100644
--- a/modules.d/98dracut-systemd/emergency.service
+++ b/modules.d/98dracut-systemd/emergency.service
@@ -12,7 +12,6 @@ Description=Emergency Shell
 DefaultDependencies=no
 After=systemd-vconsole-setup.service
 Wants=systemd-vconsole-setup.service
-ConditionKernelCommandLine=!action_on_fail=continue
 
 [Service]
 Environment=HOME=/
-- 
1.8.3.1

[patch 4/6] [PATCH 4/5] Revert "dracut-emergency.service: do not start for "action_on_fail=continue""

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

[-- Attachment #1: 0004-Revert-dracut-emergency.service-do-not-start-for-act.patch --]
[-- Type: text/plain, Size: 839 bytes --]

This reverts commit b78e6ac2c328c76057b17059a761ab44707f9aa0.

action_on_fail is useless thus remove the commits for it one by one.

Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 .../98dracut-systemd/dracut-emergency.service      |  1 -
 9 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/modules.d/98dracut-systemd/dracut-emergency.service b/modules.d/98dracut-systemd/dracut-emergency.service
index a4b81bc..a523671 100644
--- a/modules.d/98dracut-systemd/dracut-emergency.service
+++ b/modules.d/98dracut-systemd/dracut-emergency.service
@@ -13,7 +13,6 @@ DefaultDependencies=no
 After=systemd-vconsole-setup.service
 Wants=systemd-vconsole-setup.service
 Conflicts=emergency.service emergency.target
-ConditionKernelCommandLine=!action_on_fail=continue
 
 [Service]
 Environment=HOME=/

[patch 5/6] [PATCH 5/5] Revert "99fs-lib/fs-lib.sh: Let user specify the action after fail for fsck"

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

[-- Attachment #1: 0005-Revert-99fs-lib-fs-lib.sh-Let-user-specify-the-actio.patch --]
[-- Type: text/plain, Size: 1098 bytes --]

This reverts commit 679fa7ed3c0f4c827bfc6885d34f56713fe85c1e.

action_on_fail is useless thus remove the commits for it one by one.

Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 modules.d/99fs-lib/fs-lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules.d/99fs-lib/fs-lib.sh b/modules.d/99fs-lib/fs-lib.sh
index df3803d..672d027 100755
--- a/modules.d/99fs-lib/fs-lib.sh
+++ b/modules.d/99fs-lib/fs-lib.sh
@@ -5,14 +5,14 @@ type getarg >/dev/null 2>&1 || . /lib/dracut-lib.sh
 fsck_ask_reboot() {
     info "note - fsck suggests reboot, if you"
     info "leave shell, booting will continue normally"
-    action_on_fail -n "(reboot ?)"
+    emergency_shell -n "(reboot ?)"
 }
 
 fsck_ask_err() {
     warn "*** An error occurred during the file system check."
     warn "*** Dropping you to a shell; the system will try"
     warn "*** to mount the filesystem(s), when you leave the shell."
-    action_on_fail -n "(Repair filesystem)"
+    emergency_shell -n "(Repair filesystem)"
 }
 
 # inherits: _ret _drv _out
-- 
1.8.3.1

[patch 6/6] [PATCH 4/5] Revert "Let user specify the action after fail"

[dyoung-H+wXaHxf7aLQT0dZR+AlfA]

[-- Attachment #1: 0006-Let-user-specify-the-action-after-fail.patch --]
[-- Type: text/plain, Size: 7395 bytes --]

This reverts commit 9d787df19674937e8c07bd295b5d282a63829209

action_on_fail is useless thus remove the commits for it one by one.

Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 dracut.cmdline.7.asc                               |  4 ----
 modules.d/95rootfs-block/mount-root.sh             |  2 +-
 modules.d/98dracut-systemd/dracut-initqueue.sh     |  2 +-
 modules.d/98dracut-systemd/dracut-mount.sh         |  2 +-
 modules.d/98selinux/selinux-loadpolicy.sh          |  6 ++++--
 modules.d/98usrmount/mount-usr.sh                  |  4 ++--
 modules.d/99base/dracut-lib.sh                     | 22 ----------------------
 modules.d/99base/init.sh                           | 12 ++++++------
 9 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/dracut.cmdline.7.asc b/dracut.cmdline.7.asc
index 2e368ad..8c7e965 100644
--- a/dracut.cmdline.7.asc
+++ b/dracut.cmdline.7.asc
@@ -159,10 +159,6 @@ Misc
    specify the controlling terminal for the console.
    This is useful, if you have multiple "console=" arguments.
 
-**action_on_fail=**_{shell|continue}_::
-   Specify the action after failure. By default it's emergency_shell.
-   'continue' means: ignore the current failure and go ahead.
-
 [[dracutkerneldebug]]
 Debug
 ~~~~~
diff --git a/modules.d/95rootfs-block/mount-root.sh b/modules.d/95rootfs-block/mount-root.sh
index 7e9c5bb..924393b 100755
--- a/modules.d/95rootfs-block/mount-root.sh
+++ b/modules.d/95rootfs-block/mount-root.sh
@@ -66,7 +66,7 @@ mount_root() {
                 warn "*** Warning -- the system did not shut down cleanly. "
                 warn "*** Dropping you to a shell; the system will continue"
                 warn "*** when you leave the shell."
-                action_on_fail
+                emergency_shell
             fi
             fsckoptions="$AUTOFSCK_OPT $fsckoptions"
         fi
diff --git a/modules.d/98dracut-systemd/dracut-initqueue.sh b/modules.d/98dracut-systemd/dracut-initqueue.sh
index 881f639..3ddc236 100755
--- a/modules.d/98dracut-systemd/dracut-initqueue.sh
+++ b/modules.d/98dracut-systemd/dracut-initqueue.sh
@@ -68,7 +68,7 @@ while :; do
     main_loop=$(($main_loop+1))
     if [ $main_loop -gt $RDRETRY ]; then
         if ! [ -f /sysroot/etc/fstab ] || ! [ -e /sysroot/sbin/init ] ; then
-            action_on_fail "Could not boot." && break
+            emergency_shell "Could not boot."
         fi
         warn "Not all disks have been found."
         warn "You might want to regenerate your initramfs."
diff --git a/modules.d/98dracut-systemd/dracut-mount.sh b/modules.d/98dracut-systemd/dracut-mount.sh
index 11ba7bc..c4febfe 100755
--- a/modules.d/98dracut-systemd/dracut-mount.sh
+++ b/modules.d/98dracut-systemd/dracut-mount.sh
@@ -29,7 +29,7 @@ while :; do
     done
 
     i=$(($i+1))
-    [ $i -gt 20 ] && action_on_fail "Can't mount root filesystem" && break
+    [ $i -gt 20 ] && emergency_shell "Can't mount root filesystem"
 done
 
 export -p > /dracut-state.sh
diff --git a/modules.d/98selinux/selinux-loadpolicy.sh b/modules.d/98selinux/selinux-loadpolicy.sh
index d0ac834..ca81a38 100755
--- a/modules.d/98selinux/selinux-loadpolicy.sh
+++ b/modules.d/98selinux/selinux-loadpolicy.sh
@@ -54,14 +54,16 @@ rd_load_policy()
         if [ $ret -eq 3 -o $permissive -eq 0 ]; then
             warn "Machine in enforcing mode."
             warn "Not continuing"
-            action_on_fail -n selinux || exit 1
+            emergency_shell -n selinux
+            exit 1
         fi
         return 0
     elif [ $permissive -eq 0 -a "$SELINUX" != "disabled" ]; then
         warn "Machine in enforcing mode and cannot execute load_policy."
         warn "To disable selinux, add selinux=0 to the kernel command line."
         warn "Not continuing"
-        action_on_fail -n selinux || exit 1
+        emergency_shell -n selinux
+        exit 1
     fi
 }
 
diff --git a/modules.d/98usrmount/mount-usr.sh b/modules.d/98usrmount/mount-usr.sh
index d1a629c..1699604 100755
--- a/modules.d/98usrmount/mount-usr.sh
+++ b/modules.d/98usrmount/mount-usr.sh
@@ -40,7 +40,7 @@ fsck_usr()
             warn "*** Warning -- the system did not shut down cleanly. "
             warn "*** Dropping you to a shell; the system will continue"
             warn "*** when you leave the shell."
-            action_on_fail
+            emergency_shell
         fi
         _fsckoptions="$AUTOFSCK_OPT $_fsckoptions"
     fi
@@ -103,7 +103,7 @@ mount_usr()
             warn "Mounting /usr to $NEWROOT/usr failed"
             warn "*** Dropping you to a shell; the system will continue"
             warn "*** when you leave the shell."
-            action_on_fail
+            emergency_shell
         fi
     fi
 }
diff --git a/modules.d/99base/dracut-lib.sh b/modules.d/99base/dracut-lib.sh
index c119018..3cfc1a3 100755
--- a/modules.d/99base/dracut-lib.sh
+++ b/modules.d/99base/dracut-lib.sh
@@ -1143,28 +1143,6 @@ emergency_shell()
     fi
 }
 
-action_on_fail()
-{
-    local _action=$(getarg action_on_fail=)
-    case "$_action" in
-        continue)
-            [ "$1" = "-n" ] && shift 2
-            [ "$1" = "--shutdown" ] && shift 2
-            warn "$*"
-            warn "Not dropping to emergency shell, because 'action_on_fail=continue' was set on the kernel command line."
-            return 0
-            ;;
-        shell)
-            emergency_shell $@
-            return 1
-            ;;
-        *)
-            emergency_shell $@
-            return 1
-            ;;
-    esac
-}
-
 # Retain the values of these variables but ensure that they are unexported
 # This is a POSIX-compliant equivalent of bash's "export -n"
 export_n()
diff --git a/modules.d/99base/init.sh b/modules.d/99base/init.sh
index a59e8ea..e2e4029 100755
--- a/modules.d/99base/init.sh
+++ b/modules.d/99base/init.sh
@@ -85,7 +85,7 @@ if command -v kmod >/dev/null 2>/dev/null; then
         done
 fi
 
-trap "action_on_fail Signal caught!" 0
+trap "emergency_shell Signal caught!" 0
 
 export UDEVVERSION=$(udevadm --version)
 if [ $UDEVVERSION -gt 166 ]; then
@@ -214,7 +214,7 @@ while :; do
 
     main_loop=$(($main_loop+1))
     [ $main_loop -gt $RDRETRY ] \
-        && { flock -s 9 ; action_on_fail "Could not boot." && break; } 9>/.console_lock
+        && { flock -s 9 ; emergency_shell "Could not boot."; } 9>/.console_lock
 done
 unset job
 unset queuetriggered
@@ -249,7 +249,7 @@ while :; do
 
     _i_mount=$(($_i_mount+1))
     [ $_i_mount -gt 20 ] \
-        && { flock -s 9 ; action_on_fail "Can't mount root filesystem" && break; } 9>/.console_lock
+        && { flock -s 9 ; emergency_shell "Can't mount root filesystem"; } 9>/.console_lock
 done
 
 {
@@ -283,7 +283,7 @@ done
 [ "$INIT" ] || {
     echo "Cannot find init!"
     echo "Please check to make sure you passed a valid root filesystem!"
-    action_on_fail
+    emergency_shell
 }
 
 if [ $UDEVVERSION -lt 168 ]; then
@@ -387,13 +387,13 @@ if [ -f /etc/capsdrop ]; then
 	warn "Command:"
 	warn capsh --drop=$CAPS_INIT_DROP -- -c exec switch_root "$NEWROOT" "$INIT" $initargs
 	warn "failed."
-	action_on_fail
+	emergency_shell
     }
 else
     unset RD_DEBUG
     exec $SWITCH_ROOT "$NEWROOT" "$INIT" $initargs || {
 	warn "Something went very badly wrong in the initramfs.  Please "
 	warn "file a bug against dracut."
-	action_on_fail
+	emergency_shell
     }
 fi
-- 
1.8.3.1

Re: [patch 0/6] Revert action_on_fail patches

[Dave Young]

On 08/31/15 at 10:43am, Dave Young wrote:
> Hi,
> 
> action_on_fail was introduced originally for kdump error handling.
> Before the patches, dracut will drop into emergency shell for many
> failure cases, but kdump does not care about some of failures as
> long as vmcore can be saved successfully. Thus action_on_fail was
> introduced to fall to kdump service in case early failure in dracut.
> 
> But later with new systemd services, there are cases that kdump
> script can never be called in case early dracut failure because of
> systemd service dependency issues.
> 
> Thus there was below post which create a new kdump error handler
> systemd service:
> https://lists.stg.fedoraproject.org/archives/list/kexec-TuqUDEhatI4ANWPb/1PvSmm0pvjS0E/A@public.gmane.org/thread/3EN63K2NDC6C545M5Z434VCHWDR3BCHD/
> 
> Now this action_on_fail interface was useless, nobody use it and
> it is broken with systemd sometimes. So I send this series to revert
> them one by one.
> 

Harald, ping

Since the code is useless, IMHO it is odd to keep them and it will
cause trouble in the future for further fixes and improvements.

What do you think?

Thanks
Dave

Re: [patch 0/6] Revert action_on_fail patches

[Harald Hoyer]

On 29.09.2015 10:11, Dave Young wrote:
> On 08/31/15 at 10:43am, Dave Young wrote:
>> Hi,
>>
>> action_on_fail was introduced originally for kdump error handling.
>> Before the patches, dracut will drop into emergency shell for many
>> failure cases, but kdump does not care about some of failures as
>> long as vmcore can be saved successfully. Thus action_on_fail was
>> introduced to fall to kdump service in case early failure in dracut.
>>
>> But later with new systemd services, there are cases that kdump
>> script can never be called in case early dracut failure because of
>> systemd service dependency issues.
>>
>> Thus there was below post which create a new kdump error handler
>> systemd service:
>> https://lists.stg.fedoraproject.org/archives/list/kexec-TuqUDEhatI4ANWPb/1PvSmm0pvjS0E/A@public.gmane.org/thread/3EN63K2NDC6C545M5Z434VCHWDR3BCHD/
>>
>> Now this action_on_fail interface was useless, nobody use it and
>> it is broken with systemd sometimes. So I send this series to revert
>> them one by one.
>>
> 
> Harald, ping
> 
> Since the code is useless, IMHO it is odd to keep them and it will
> cause trouble in the future for further fixes and improvements.
> 
> What do you think?
> 
> Thanks
> Dave


Thanks, pushed!