Skip to contentSkip to conversation

Revert action_on_fail patches

PATCH In review [email protected] 2 participants 9 messages Aug 31, 2015 – Nov 13, 2015 2 messages not in the archive

lore
Share & tools
lore
#0 lore raw

cover Revert action_on_fail patches

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/[email protected]/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

#1 lore raw

PATCH 1/5 Revert "Change the implementation of action_on_fail"

This reverts commit d2765b5175663d094737d6819cc3f3df53e7a4cb.

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

---
 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(-)
dracut.cmdline.7.asc +4 -0
index 8c7e965..07d3a93 100644
@@ -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
 ~~~~~
modules.d/98dracut-systemd/dracut-emergency.service +1 -1
index 5a6d525..a4b81bc 100644
@@ -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=/
modules.d/98dracut-systemd/emergency.service +1 -1
index 5f1eaa2..c19fe37 100644
@@ -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=/
modules.d/99base/dracut-lib.sh +18 -10
index 1eaf4b3..1f50e2f 100755
@@ -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
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH 2/5 Revert "rename kernel command line param action_on_fail to rd.action_on_fail"

In reply to [email protected]

This reverts commit 391d6057908be2fe78f8084eb1b2adc27c781c0b.

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

---
 dracut.cmdline.7.asc           | 2 +-
 modules.d/99base/dracut-lib.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
dracut.cmdline.7.asc +1 -1
index 07d3a93..2e368ad 100644
@@ -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.
 
modules.d/99base/dracut-lib.sh +1 -1
index 1f50e2f..c119018 100755
@@ -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
Signature · 1 line
1.8.3.1
#3 lore raw

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

In reply to [email protected]

This reverts commit dcae873414ff643e1de790f256e414923e2aef8b.

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

---
 modules.d/98dracut-systemd/emergency.service | 1 -
 1 file changed, 1 deletion(-)
modules.d/98dracut-systemd/emergency.service +0 -1
index c19fe37..35d9c31 100644
@@ -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=/
Signature · 1 line
1.8.3.1
#4 lore raw

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

In reply to [email protected]

This reverts commit b78e6ac2c328c76057b17059a761ab44707f9aa0.

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

---
 .../98dracut-systemd/dracut-emergency.service      |  1 -
 9 files changed, 15 insertions(+), 40 deletions(-)
modules.d/98dracut-systemd/dracut-emergency.service +0 -1
index a4b81bc..a523671 100644
@@ -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=/
#5 lore raw

PATCH 4/5 Revert "Let user specify the action after fail"

In reply to [email protected]

This reverts commit 9d787df19674937e8c07bd295b5d282a63829209

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

---
 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(-)
dracut.cmdline.7.asc +0 -4
index 2e368ad..8c7e965 100644
@@ -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
 ~~~~~
modules.d/95rootfs-block/mount-root.sh +1 -1
index 7e9c5bb..924393b 100755
@@ -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
modules.d/98dracut-systemd/dracut-initqueue.sh +1 -1
index 881f639..3ddc236 100755
@@ -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."
modules.d/98dracut-systemd/dracut-mount.sh +1 -1
index 11ba7bc..c4febfe 100755
@@ -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
modules.d/98selinux/selinux-loadpolicy.sh +4 -2
index d0ac834..ca81a38 100755
@@ -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
 }
 
modules.d/98usrmount/mount-usr.sh +2 -2
index d1a629c..1699604 100755
@@ -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
 }
modules.d/99base/dracut-lib.sh +0 -22
index c119018..3cfc1a3 100755
@@ -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()
modules.d/99base/init.sh +6 -6
index a59e8ea..e2e4029 100755
@@ -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
Signature · 1 line
1.8.3.1
#6 lore raw

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

In reply to [email protected]

This reverts commit 679fa7ed3c0f4c827bfc6885d34f56713fe85c1e.

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

---
 modules.d/99fs-lib/fs-lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
modules.d/99fs-lib/fs-lib.sh +2 -2
index df3803d..672d027 100755
@@ -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
Signature · 1 line
1.8.3.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#8 lore raw
Quoted 12 lines · Dave Young

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.

Quoted 8 lines

Thus there was below post which create a new kdump error handler systemd service:
https://lists.stg.fedoraproject.org/archives/list/kexec-TuqUDEhatI4ANWPb/1PvSmm0pvjS0E/[email protected]/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

A message that is not in the archive (sent off-list or not yet received): 20150929081150.GD30623-sa4SJRhfYT7Js+WLOFIw6R/[email protected]

#10 lore raw
Quoted 24 lines · Dave Young
Quoted 12 lines · Dave Young

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.

Quoted 8 lines

Thus there was below post which create a new kdump error handler systemd service:
https://lists.stg.fedoraproject.org/archives/list/kexec-TuqUDEhatI4ANWPb/1PvSmm0pvjS0E/[email protected]/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

Quoted 7 lines

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!