From 8175924e89aefbc91e5f0b40213e0ed1ea394283 Mon Sep 17 00:00:00 2001 From: Philipp Rudo Date: Wed, 6 Sep 2023 10:49:43 +0200 Subject: [PATCH] kdumpctl: Stop updating grub config in reset_crashkernel With multiple kernel variants on the same architecture, e.g. the 4k and 64k kernel on aarch64, we can no longer assume that the crashkernel value for the currently running kernel will work for all installed kernels. This also means that we can no longer update the grub config as we don't know which value to set it to. Thus get the crashkernel value for each kernel and stop updating the grub config. While at it merge the _new_fadump and _fadump_val variables and remove _read_kernel_arg_in_grub_etc_default which has no user. Signed-off-by: Philipp Rudo Reviewed-by: Pingfan Liu --- kdumpctl | 96 ++----------------------- spec/kdumpctl_general_spec.sh | 67 ----------------- spec/kdumpctl_reset_crashkernel_spec.sh | 10 --- 3 files changed, 5 insertions(+), 168 deletions(-) diff --git a/kdumpctl b/kdumpctl index 113d8bc..6e4685f 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1480,69 +1480,9 @@ _get_all_kernels_from_grubby() echo -n "$_kernels" } -GRUB_ETC_DEFAULT="/etc/default/grub" -# Update a kernel parameter in default grub conf -# -# If a value is specified, it will be inserted in the end. Otherwise it -# would remove given kernel parameter. -# -# Note this function doesn't address the following cases, -# 1. The kernel ignores everything on the command line after a '--'. So -# simply adding the new entry to the end will fail if the cmdline -# contains a --. -# 2. If the value for a parameter contains spaces it can be quoted using -# double quotes, for example param="value with spaces". This will -# break the [^[:space:]\"] regex for the value. -# 3. Dashes and underscores in the parameter name are equivalent. So -# some_parameter and some-parameter are identical. -# 4. Some parameters, e.g. efivar_ssdt, can be given multiple times. -# 5. Some kernel parameters, e.g. quiet, doesn't have value -# -# $1: the name of the kernel command line parameter -# $2: new value. If empty, given parameter would be removed -_update_kernel_arg_in_grub_etc_default() -{ - local _para=$1 _val=$2 _para_val - - if [[ $(uname -m) == s390x ]]; then - return - fi - - if [[ -n $_val ]]; then - _para_val="$_para=$_val" - fi - - # Update the command line /etc/default/grub, i.e. - # on the line that starts with 'GRUB_CMDLINE_LINUX=', - # 1) remove $para=$val if the it's the first arg - # 2) remove all occurences of $para=$val - # 3) insert $_para_val to end - # 4) remove duplicate spaces left over by 1) or 2) or 3) - # 5) remove space at the beginning of the string left over by 1) or 2) or 3) - # 6) remove space at the end of the string left over by 1) or 2) or 3) - sed -i -E "/^GRUB_CMDLINE_LINUX=/ { - s/\"${_para}=[^[:space:]\"]*/\"/g; - s/[[:space:]]+${_para}=[^[:space:]\"]*/ /g; - s/\"$/ ${_para_val}\"/ - s/[[:space:]]+/ /g; - s/(\")[[:space:]]+/\1/g; - s/[[:space:]]+(\")/\1/g; - }" "$GRUB_ETC_DEFAULT" -} - -# Read the kernel arg in default grub conf. - -# Note reading a kernel parameter that doesn't have a value isn't supported. -# -# $1: the name of the kernel command line parameter -_read_kernel_arg_in_grub_etc_default() -{ - sed -n -E "s/^GRUB_CMDLINE_LINUX=.*[[:space:]\"]${1}=([^[:space:]\"]*).*$/\1/p" "$GRUB_ETC_DEFAULT" -} - reset_crashkernel() { - local _opt _val _fadump_val _reboot _grubby_kernel_path _kernel _kernels + local _opt _val _reboot _grubby_kernel_path _kernel _kernels local _dump_mode _new_dump_mode local _has_changed _needs_reboot local _old_ck _new_ck @@ -1555,13 +1495,12 @@ reset_crashkernel() derror "Option --fadump only valid on PPC" exit 1 fi - _val=${_opt#*=} - if _dump_mode=$(get_dump_mode_by_fadump_val $_val); then - _fadump_val=$_val - else + _new_fadump=${_opt#*=} + if ! _dump_mode=$(get_dump_mode_by_fadump_val $_new_fadump); then derror "failed to determine dump mode" exit fi + [[ "$_new_fadump" == off ]] && _new_fadump="" ;; --kernel=*) _val=${_opt#*=} @@ -1598,29 +1537,6 @@ reset_crashkernel() [[ $(uname -m) != ppc64le ]] && _dump_mode=kdump - # If the dump mode is determined, we can also know the default crashkernel value - if [[ -n $_dump_mode ]]; then - _crashkernel=$(kdump_get_arch_recommend_crashkernel "$_dump_mode") - fi - - # If --kernel-path=ALL, update GRUB_CMDLINE_LINUX in /etc/default/grub. - # - # An exception case is when the ppc64le user doesn't specify the fadump value. - # In this case, the dump mode would be determined by parsing the kernel - # command line of the kernel(s) to be updated thus don't update GRUB_CMDLINE_LINUX. - # - # The following code has been simplified because of what has been done early, - # - set the dump mode as kdump for non-ppc64le cases - # - retrieved the default crashkernel value for given dump mode - if [[ $_grubby_kernel_path == ALL && -n $_dump_mode ]]; then - _update_kernel_arg_in_grub_etc_default crashkernel "$_crashkernel" - # remove the fadump if fadump is disabled - if [[ $_fadump_val == off ]]; then - _fadump_val="" - fi - _update_kernel_arg_in_grub_etc_default fadump "$_fadump_val" - fi - # If kernel-path not specified, either # - use KDUMP_KERNELVER if it's defined # - use current running kernel @@ -1638,15 +1554,13 @@ reset_crashkernel() _has_changed="" if [[ -z $_dump_mode ]]; then _new_dump_mode=$(get_dump_mode_by_kernel "$_kernel") - _new_ck=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") _new_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump) else _new_dump_mode=$_dump_mode - _new_ck=$_crashkernel - _new_fadump=$_fadump_val 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" diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index 7304cd4..dcac2f8 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -155,73 +155,6 @@ Describe 'kdumpctl' End End - Describe '_update_kernel_arg_in_grub_etc_default()' - GRUB_ETC_DEFAULT=/tmp/default_grub - - cleanup() { - rm -rf "$GRUB_ETC_DEFAULT" - } - AfterAll 'cleanup' - - Context 'when the given parameter is in different positions' - Parameters - "crashkernel=222M fadump=on rhgb quiet" crashkernel 333M - " fadump=on crashkernel=222M rhgb quiet" crashkernel 333M - "fadump=on rhgb quiet crashkernel=222M" crashkernel 333M - "fadump=on rhgb quiet" crashkernel 333M - "fadump=on foo=bar1 rhgb quiet" foo bar2 - End - - It 'should update the kernel parameter correctly' - echo 'GRUB_CMDLINE_LINUX="'"$1"'"' >$GRUB_ETC_DEFAULT - When call _update_kernel_arg_in_grub_etc_default "$2" "$3" - # the updated kernel parameter should appear in the end - The contents of file $GRUB_ETC_DEFAULT should include "$2=$3\"" - End - End - - It 'should only update the given parameter and not update the parameter that has the given parameter as suffix' - echo 'GRUB_CMDLINE_LINUX="fadump=on rhgb quiet ckcrashkernel=222M"' >$GRUB_ETC_DEFAULT - _ck_val=1G-4G:192M,4G-64G:256M,64G-102400T:512M - When call _update_kernel_arg_in_grub_etc_default crashkernel "$_ck_val" - The contents of file $GRUB_ETC_DEFAULT should include "crashkernel=$_ck_val\"" - The contents of file $GRUB_ETC_DEFAULT should include "ckcrashkernel=222M" - End - - It 'should be able to handle the cases of there are multiple crashkernel entries' - echo 'GRUB_CMDLINE_LINUX="fadump=on rhgb quiet crashkernel=101M crashkernel=222M"' >$GRUB_ETC_DEFAULT - _ck_val=1G-4G:192M,4G-64G:256M,64G-102400T:512M - When call _update_kernel_arg_in_grub_etc_default crashkernel "$_ck_val" - The contents of file $GRUB_ETC_DEFAULT should include "crashkernel=$_ck_val\"" - The contents of file $GRUB_ETC_DEFAULT should not include "crashkernel=222M" - End - - Context 'when it removes a kernel parameter' - - It 'should remove all values for given arg' - echo 'GRUB_CMDLINE_LINUX="crashkernel=33M crashkernel=11M fadump=on crashkernel=222M"' >$GRUB_ETC_DEFAULT - When call _update_kernel_arg_in_grub_etc_default crashkernel - The contents of file $GRUB_ETC_DEFAULT should equal 'GRUB_CMDLINE_LINUX="fadump=on"' - End - - It 'should not remove args that have the given arg as suffix' - echo 'GRUB_CMDLINE_LINUX="ckcrashkernel=33M crashkernel=11M ckcrashkernel=222M"' >$GRUB_ETC_DEFAULT - When call _update_kernel_arg_in_grub_etc_default crashkernel - The contents of file $GRUB_ETC_DEFAULT should equal 'GRUB_CMDLINE_LINUX="ckcrashkernel=33M ckcrashkernel=222M"' - End - End - - End - - Describe '_read_kernel_arg_in_grub_etc_default()' - GRUB_ETC_DEFAULT=/tmp/default_grub - It 'should read the value for given arg' - echo 'GRUB_CMDLINE_LINUX="crashkernel=33M crashkernel=11M ckcrashkernel=222M"' >$GRUB_ETC_DEFAULT - When call _read_kernel_arg_in_grub_etc_default crashkernel - The output should equal '11M' - End - End - Describe '_find_kernel_path_by_release()' grubby() { echo -e 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"' diff --git a/spec/kdumpctl_reset_crashkernel_spec.sh b/spec/kdumpctl_reset_crashkernel_spec.sh index 8fb4385..2e3b2ba 100644 --- a/spec/kdumpctl_reset_crashkernel_spec.sh +++ b/spec/kdumpctl_reset_crashkernel_spec.sh @@ -111,11 +111,6 @@ Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]' fi } - _update_kernel_arg_in_grub_etc_default() { - # don't modify /etc/default/grub during the test - echo _update_kernel_arg_in_grub_etc_default "$@" - } - kdump_crashkernel=$(get_default_crashkernel kdump) fadump_crashkernel=$(get_default_crashkernel fadump) Context "when no --kernel specified" @@ -141,7 +136,6 @@ Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]' grubby --args crashkernel=$ck --update-kernel ALL Specify 'kdumpctl should warn the user that crashkernel has been udpated' When call reset_crashkernel --kernel=ALL --fadump=on - The line 1 of output should include "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel" The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1" The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel2" End @@ -200,8 +194,6 @@ Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]' grubby --args fadump=on --update-kernel ALL Specify 'fadump=on to fadump=nocma' When call reset_crashkernel --kernel=ALL --fadump=nocma - The line 1 of output should equal "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel" - The line 2 of output should equal "_update_kernel_arg_in_grub_etc_default fadump nocma" The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1" The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel2" End @@ -213,8 +205,6 @@ Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]' Specify 'fadump=nocma to fadump=on' When call reset_crashkernel --kernel=ALL --fadump=on - The line 1 of output should equal "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel" - The line 2 of output should equal "_update_kernel_arg_in_grub_etc_default fadump on" The error should include "Updated fadump=on for kernel=$kernel1" End