From 93b0538210045fa217f9c1326632019f30438941 Mon Sep 17 00:00:00 2001 From: Philipp Rudo Date: Tue, 9 Jul 2024 15:31:17 +0200 Subject: [PATCH] kdumpctl: Simplify fadump handling in reset_crashkernel Resolves: RHEL-31422 Resolves: RHEL-42413 commit 574f8f588b5257d410c01de076644d398e1fc9e6 Author: Philipp Rudo Date: Thu Jun 1 17:04:13 2023 +0200 kdumpctl: Simplify fadump handling in reset_crashkernel When handling fadump there are three cases we need to consider 1) When running on non-ppc64le systems In this case _dump_mode=kdump and _new_fadump='' always. In other words we don't need to care updating the fadump parameter on the kernel command line. We could always remove it like the code did so far. But should we remove it when we never set it? In particular as that might overwrite a change explicitly made by the user (for whatever reason) 2) When running on ppc64le and the user provided --fadump option In this case _new_fadump and _dump_mode are set accordingly to what the user provided. Thus we need to update both the crashkernel (in case fadump was turned on/off) and the fadump (in case the fadump mode changed) parameters. 3) When running on ppc64le but the user did not provide --fadump option In this case both _new_fadump='' and _dump_mode=''. In this case we take over the previously set fadump parameter. Which means that we don't need to update the fadump parameter at all. We do need to check whether the _new_dump_mode is fadump or kdump though so we use the correct (new) default crashkernel value. In the three cases only the second one needs to update the fadump parameter. Reflect this discussion in code. This also fixes a bug that always prints kdump: Updated fadump= for kernel=$kernel. Please reboot the system for the change to take effect. when the crashkernel= parameter is unchanged as well as reboots the system, if --reboot is provided. Even for non-ppc architectures. Fixes: 140da74 ("rewrite reset_crashkernel to support fadump and to used by RPM scriptlet") Signed-off-by: Philipp Rudo Signed-off-by: Philipp Rudo --- ...y-fadump-handling-in-reset_crashkern.patch | 127 ++++++++++++++++++ ...escription-to-reset-crashkernel-rebo.patch | 32 +++++ kexec-tools.spec | 4 + 3 files changed, 163 insertions(+) create mode 100644 0003-kdumpctl-Simplify-fadump-handling-in-reset_crashkern.patch create mode 100644 0004-kdumpctl.8-Add-description-to-reset-crashkernel-rebo.patch diff --git a/0003-kdumpctl-Simplify-fadump-handling-in-reset_crashkern.patch b/0003-kdumpctl-Simplify-fadump-handling-in-reset_crashkern.patch new file mode 100644 index 0000000..d43d7e7 --- /dev/null +++ b/0003-kdumpctl-Simplify-fadump-handling-in-reset_crashkern.patch @@ -0,0 +1,127 @@ +From 8ebf2874a202a2d7116a27c69816b8621ace5224 Mon Sep 17 00:00:00 2001 +From: Philipp Rudo +Date: Thu, 1 Jun 2023 17:04:13 +0200 +Subject: [PATCH 3/7] kdumpctl: Simplify fadump handling in reset_crashkernel + +When handling fadump there are three cases we need to consider + +1) When running on non-ppc64le systems + In this case _dump_mode=kdump and _new_fadump='' always. In other + words we don't need to care updating the fadump parameter on the + kernel command line. We could always remove it like the code did so + far. But should we remove it when we never set it? In particular as + that might overwrite a change explicitly made by the user (for + whatever reason) + +2) When running on ppc64le and the user provided --fadump option + In this case _new_fadump and _dump_mode are set accordingly to what + the user provided. Thus we need to update both the crashkernel (in + case fadump was turned on/off) and the fadump (in case the fadump + mode changed) parameters. + +3) When running on ppc64le but the user did not provide --fadump option + In this case both _new_fadump='' and _dump_mode=''. In this case we + take over the previously set fadump parameter. Which means that we + don't need to update the fadump parameter at all. We do need to check + whether the _new_dump_mode is fadump or kdump though so we use the + correct (new) default crashkernel value. + +In the three cases only the second one needs to update the fadump +parameter. Reflect this discussion in code. + +This also fixes a bug that always prints + + kdump: Updated fadump= for kernel=$kernel. Please reboot the + system for the change to take effect. + +when the crashkernel= parameter is unchanged as well as reboots the +system, if --reboot is provided. Even for non-ppc architectures. + +Fixes: 140da74 ("rewrite reset_crashkernel to support fadump and to used by RPM scriptlet") +Signed-off-by: Philipp Rudo +--- + kdumpctl | 38 ++++++++++++++++++++++---------------- + 1 file changed, 22 insertions(+), 16 deletions(-) + +diff --git a/kdumpctl b/kdumpctl +index 8dc56e5..8ec638b 100755 +--- a/kdumpctl ++++ b/kdumpctl +@@ -1467,10 +1467,10 @@ _get_all_kernels_from_grubby() + reset_crashkernel() + { + local _opt _val _reboot _grubby_kernel_path _kernel _kernels +- local _dump_mode _new_dump_mode ++ local _dump_mode + local _has_changed _needs_reboot + local _old_ck _new_ck +- local _old_fadump _new_fadump ++ local _old_fadump _new_fadump _opt_fadump + + for _opt in "$@"; do + case "$_opt" in +@@ -1479,12 +1479,11 @@ reset_crashkernel() + derror "Option --fadump only valid on PPC" + exit 1 + fi +- _new_fadump=${_opt#*=} +- if ! _dump_mode=$(get_dump_mode_by_fadump_val $_new_fadump); then ++ _opt_fadump=${_opt#*=} ++ if ! _dump_mode=$(get_dump_mode_by_fadump_val $_opt_fadump); then + derror "failed to determine dump mode" + exit + fi +- [[ "$_new_fadump" == off ]] && _new_fadump="" + ;; + --kernel=*) + _val=${_opt#*=} +@@ -1519,8 +1518,6 @@ reset_crashkernel() + return + fi + +- [[ $(uname -m) != ppc64le ]] && _dump_mode=kdump +- + # If kernel-path not specified, either + # - use KDUMP_KERNELVER if it's defined + # - use current running kernel +@@ -1536,19 +1533,28 @@ reset_crashkernel() + + for _kernel in $_kernels; do + _has_changed="" +- if [[ -z $_dump_mode ]]; then +- _new_dump_mode=$(get_dump_mode_by_kernel "$_kernel") +- _new_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) ++ if [[ $(uname -m) == ppc64le ]]; then ++ if [[ -n "$_opt_fadump" ]]; then ++ _new_ck=$(kdump_get_arch_recommend_crashkernel "$_dump_mode") ++ _old_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) ++ _new_fadump="$_opt_fadump" ++ [[ "$_new_fadump" == off ]] && _new_fadump="" ++ if _update_kernel_cmdline "$_kernel" fadump "$_old_fadump" "$_new_fadump"; then ++ if [[ -n "$_new_fadump" ]]; then ++ _has_changed="Updated fadump=$_new_fadump" ++ else ++ _has_changed="Removed fadump" ++ fi ++ fi ++ else ++ _dump_mode="$(get_dump_mode_by_kernel "$_kernel")" ++ _new_ck=$(kdump_get_arch_recommend_crashkernel "$_dump_mode") ++ fi + else +- _new_dump_mode=$_dump_mode ++ _new_ck=$(kdump_get_arch_recommend_crashkernel kdump) + fi + + _old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) +- _new_ck=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") +- _old_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) +- if _update_kernel_cmdline "$_kernel" fadump "$_old_fadump" "$_new_fadump"; then +- _has_changed="Updated fadump=$_new_fadump" +- fi + if _update_kernel_cmdline "$_kernel" crashkernel "$_old_ck" "$_new_ck"; then + _has_changed="Updated crashkernel=$_new_ck" + fi +-- +2.45.2 + diff --git a/0004-kdumpctl.8-Add-description-to-reset-crashkernel-rebo.patch b/0004-kdumpctl.8-Add-description-to-reset-crashkernel-rebo.patch new file mode 100644 index 0000000..0a7fba6 --- /dev/null +++ b/0004-kdumpctl.8-Add-description-to-reset-crashkernel-rebo.patch @@ -0,0 +1,32 @@ +From de393c1bcb7d32e97d9b46bb6a73180072f03aa3 Mon Sep 17 00:00:00 2001 +From: Philipp Rudo +Date: Mon, 1 Jul 2024 12:52:39 +0200 +Subject: [PATCH 4/7] kdumpctl.8: Add description to reset-crashkernel --reboot + +There is no description for parameter --reboot for reset-crashkernel. +Thus add one. + +Suggested-by: Lichen Liu +Signed-off-by: Philipp Rudo +--- + kdumpctl.8 | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/kdumpctl.8 b/kdumpctl.8 +index 33c1115..29a6119 100644 +--- a/kdumpctl.8 ++++ b/kdumpctl.8 +@@ -62,6 +62,10 @@ grubby's kernel-path=ALL and kernel-path=DEFAULT. ppc64le supports FADump and + supports an additional [--fadump=[on|off|nocma]] parameter to toggle FADump + on/off. + ++If the optional parameter [--reboot] is provided the system will automatically ++reboot for changes to take effect. If no changes were made to the kernel ++command line the reboot is omitted. ++ + Note: The memory requirements for kdump varies heavily depending on the + used hardware and system configuration. Thus the recommended + crashkernel might not work for your specific setup. Please test if +-- +2.45.2 + diff --git a/kexec-tools.spec b/kexec-tools.spec index 85b421f..c773615 100644 --- a/kexec-tools.spec +++ b/kexec-tools.spec @@ -67,6 +67,8 @@ Patch611: kexec-tools-2.0.28-makedumpfile-0003-PATCH-Fix-wrong-exclusion-of-Slab Patch612: 0001-Use-grep-q-cmd-instead-of-cmd-grep-q.patch Patch613: 0001-kdumpctl-Drop-default-kexec-d-option.patch Patch614: 0002-kdump-lib-fix-sed-expression-in-prepare_cmdline-on-a.patch +Patch615: 0003-kdumpctl-Simplify-fadump-handling-in-reset_crashkern.patch +Patch616: 0004-kdumpctl.8-Add-description-to-reset-crashkernel-rebo.patch %description kexec-tools provides /sbin/kexec binary that facilitates a new @@ -164,6 +166,8 @@ tar -z -x -v -f %{SOURCE19} %patch 612 -p1 -d kdump-utils-%{kdump_utils_ver} %patch 613 -p1 -d kdump-utils-%{kdump_utils_ver} %patch 614 -p1 -d kdump-utils-%{kdump_utils_ver} +%patch 615 -p1 -d kdump-utils-%{kdump_utils_ver} +%patch 616 -p1 -d kdump-utils-%{kdump_utils_ver} %ifarch ppc %define archdef ARCH=ppc