On Fri, Nov 19, 2021 at 01:31:19PM +0800, Pingfan Liu wrote:

On Fri, Nov 19, 2021 at 11:22:59AM +0800, Coiby Xu wrote:

...

Factor out kdump_get_arch_recommend_crashkernel to prepare for kdump-anaonda-plugin to retrieve the default crashkernel value.

I am ignorant with the add-on process, and try to assure something. -1. Before the installation of kexec-tools.rpm, add-on allows users to pick kexec-tools and choose kdump/fadump

kdumpctl exist in the running installing system. So we have kdumpctl even before installing kexec-tools to the target system. Btw, kdump-anaconda-addon marks kexec-tools as a requirement and don't provide a choice for user to choose kexec-tools.

-2. After the installation, there is post-script, which will call kdump_get_arch_recommend_crashkernel().

Currently, the value of crashkernel has already been saved in the kdump window before the installation. After the installing, kdump-anacond-addon would configure bootloader with the saved crashkernel.

Right?

Thanks for raising up these questions. I'll update the commit message to share how kdump-anaconda-addon would set crashkernel.

Thanks,

Pingfan

...

Signed-off-by: Coiby Xu coxu@redhat.com

kdump-lib.sh | 60 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh index b8f6c96..94cc8ff 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -822,40 +822,52 @@ get_recommend_size() IFS="$OLDIFS" }

+# get default crashkernel +# $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable +kdump_get_arch_recommend_crashkernel() +{

• local _arch _ck_cmdline _dump_mode
• if [[ -z "$1" ]]; then

• if is_fadump_capable; then
  

• 	_dump_mode=fadump
  

• else
  

• 	_dump_mode=kdump
  

• fi
  

• else

• _dump_mode=$1
  

• fi
• _arch=$(lscpu | sed -n "0,/Architecture:/s/Architecture:\s*(.*)/\U\1/p")
• if [[ $_arch == "X86_64" ]] || [[ $_arch == "S390X" ]]; then

• _ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
  

• elif [[ $_arch == "AARCH64" ]]; then

• _ck_cmdline="2G-:448M"
  

• elif [[ $_arch == "PPC64LE" ]]; then

• if [[ $_dump_mode == "fadump" ]]; then
  

• 	_ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
  

• else
  

• 	_ck_cmdline="2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"
  

• fi
  

• fi
• _ck_cmdline=${_ck_cmdline//-:/-102400T:}
• echo -n "$_ck_cmdline"

+}

# return recommended size based on current system RAM size # $1: kernel version, if not set, will defaults to $(uname -r) kdump_get_arch_recommend_size() {

• local kernel=$1 arch

• local _ck_cmdline

  if ! [[ -r "/proc/iomem" ]]; then echo "Error, can not access /proc/iomem." return 1 fi

• [[ -z $kernel ]] && kernel=$(uname -r)
• ck_cmdline=$(cat "/usr/lib/modules/$kernel/crashkernel.default" 2> /dev/null)
• if [[ -n $ck_cmdline ]]; then

• ck_cmdline=${ck_cmdline#crashkernel=}
  

• else

• arch=$(lscpu | grep Architecture | awk -F ":" '{ print $2 }' | tr '[:lower:]' '[:upper:]')
  

• if [[ $arch == "X86_64" ]] || [[ $arch == "S390X" ]]; then
  

• 	ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
  

• elif [[ $arch == "AARCH64" ]]; then
  

• 	ck_cmdline="2G-:448M"
  

• elif [[ $arch == "PPC64LE" ]]; then
  

• 	if is_fadump_capable; then
  

• 		ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
  

• 	else
  

• 		ck_cmdline="2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"
  

• 	fi
  

• fi
  

• fi
• ck_cmdline=${ck_cmdline//-:/-102400T:} sys_mem=$(get_system_size)

• _ck_cmdline=$(kdump_get_arch_recommend_crashkernel is_fadump_capable) get_recommend_size "$sys_mem" "$ck_cmdline"

}

-- 2.31.1

Hi Philipp,

On Tue, Nov 30, 2021 at 06:37:41PM +0100, Philipp Rudo wrote:

Hi Coiby,

On Fri, 19 Nov 2021 11:22:59 +0800 Coiby Xu coxu@redhat.com wrote:

...

Factor out kdump_get_arch_recommend_crashkernel to prepare for kdump-anaonda-plugin to retrieve the default crashkernel value.

Signed-off-by: Coiby Xu coxu@redhat.com

kdump-lib.sh | 60 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh index b8f6c96..94cc8ff 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -822,40 +822,52 @@ get_recommend_size() IFS="$OLDIFS" }

+# get default crashkernel +# $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable +kdump_get_arch_recommend_crashkernel() +{

• local _arch _ck_cmdline _dump_mode
• if [[ -z "$1" ]]; then

• if is_fadump_capable; then
  

• 	_dump_mode=fadump
  

• else
  

• 	_dump_mode=kdump
  

• fi
  

• else

• _dump_mode=$1
  

• fi
• _arch=$(lscpu | sed -n "0,/Architecture:/s/Architecture:\s*(.*)/\U\1/p")

this looks much better than before, but using

The credit should go to Kairui:)

uname -m | tr '[:lower:]' '[:upper:]'

would be even simpler. Especially it omits the need to parse the lscpu output.

Thanks for providing a simpler solution!

...

• if [[ $_arch == "X86_64" ]] || [[ $_arch == "S390X" ]]; then

• _ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
  

• elif [[ $_arch == "AARCH64" ]]; then

• _ck_cmdline="2G-:448M"
  

• elif [[ $_arch == "PPC64LE" ]]; then

• if [[ $_dump_mode == "fadump" ]]; then
  

• 	_ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
  

• else
  

• 	_ck_cmdline="2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"
  

• fi
  

• fi
• _ck_cmdline=${_ck_cmdline//-:/-102400T:}
• echo -n "$_ck_cmdline"

+}

# return recommended size based on current system RAM size # $1: kernel version, if not set, will defaults to $(uname -r) kdump_get_arch_recommend_size() {

• local kernel=$1 arch

• local _ck_cmdline

  if ! [[ -r "/proc/iomem" ]]; then echo "Error, can not access /proc/iomem." return 1 fi

• [[ -z $kernel ]] && kernel=$(uname -r)
• ck_cmdline=$(cat "/usr/lib/modules/$kernel/crashkernel.default" 2> /dev/null)

you are silently dropping the support of crashkenrel.default in this patch. I think it's ok to remove the support, as that's the overall goal of the series but it should be mentioned in the commit message.

I'll add a note for this. Thanks!

Thanks Philipp

...

• if [[ -n $ck_cmdline ]]; then

• ck_cmdline=${ck_cmdline#crashkernel=}
  

• else

• arch=$(lscpu | grep Architecture | awk -F ":" '{ print $2 }' | tr '[:lower:]' '[:upper:]')
  

• if [[ $arch == "X86_64" ]] || [[ $arch == "S390X" ]]; then
  

• 	ck_cmdline="1G-4G:192M,4G-64G:256M,64G-:512M"
  

• elif [[ $arch == "AARCH64" ]]; then
  

• 	ck_cmdline="2G-:448M"
  

• elif [[ $arch == "PPC64LE" ]]; then
  

• 	if is_fadump_capable; then
  

• 		ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"
  

• 	else
  

• 		ck_cmdline="2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"
  

• 	fi
  

• fi
  

• fi
• ck_cmdline=${ck_cmdline//-:/-102400T:} sys_mem=$(get_system_size)

• _ck_cmdline=$(kdump_get_arch_recommend_crashkernel is_fadump_capable) get_recommend_size "$sys_mem" "$ck_cmdline"

}

On Wed, Dec 01, 2021 at 02:15:26PM +0100, Philipp Rudo wrote:

Hi Coiby,

On Fri, 19 Nov 2021 11:23:03 +0800 Coiby Xu coxu@redhat.com wrote:

...

This helper function will be used to retrieve the value of kernel cmdline parameters including crashkernel, fadump, swiotlb and etc.

Signed-off-by: Coiby Xu coxu@redhat.com

kdumpctl | 9 +++++++++ 1 file changed, 9 insertions(+)

diff --git a/kdumpctl b/kdumpctl index 1938968..6f1afad 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1304,6 +1304,15 @@ get_default_crashkernel() local _dump_mode=$1

echo -n "$(kdump_get_arch_recommend_crashkernel "$_dump_mode")"

• }

wrong indentation

Corrected in v2, thanks!

...

+# Read kernel cmdline parameter for a specific kernel +# $1: kernel path, DEFAULT or kernel path +# $2: kernel cmldine parameter +get_grub_kernel_boot_parameter() {

• local _kernel_path=$1 _para=$2
• grubby --info="$_kernel_path" | sed -n 's/^args="(.*)"$/\1/p' | sed -n "s/^.*${_para}=(\S*).*/\1/p"

1. what happens with $_kernel_path=ALL ? in that case this function

will return multiple lines. 2. you can merge the two calls to sed and drop the second pipe 3. the second regex is frail to partial matches, e.g. crashkernel=256M and foo-crashkernel=256M will return the same value which might not be what you want.

The way I see it the sed call should look like this

sed -En -e "/^args=.*$/{s/^.*(\s|")${_para}=(\S*).*"$/\2/p;q}"

this will give you the first appearance of of $_para from the grubby output, which AFAIK is from the last kernel installed.

Applied to v2, thanks!

Thanks Philipp

...

}

reset_crashkernel()

Hi Coiby,

On Fri, 19 Nov 2021 11:23:04 +0800 Coiby Xu coxu@redhat.com wrote:

kumpctl reset-crashkernel [[CRASHKERNEL], [KERNELPATH[, CRASHKERNEL]]

With no CRASHKERNEL specified, kexec-tools would use the value from kdump.conf. With valid CRASHKERNEL specified, kexec-tools would also write the value to kdump.conf.

With no KERNELPATH specified, crashkernel would be set for current running kernel, i.e. `uname -r`. If KERNELRELEASE=ALL, the same

s/KERNELRELEASE/KERNELPATH/ ?

crashkernel values would be set for all kernels.

This interface will also be called by kernel installation hook 92-crashkernel.install to set crashkernel for a newly installed kernel.

Signed-off-by: Coiby Xu coxu@redhat.com

kdumpctl | 122 ++++++++++++++++++++++++++++++++++++++++++++--------- kdumpctl.8 | 12 +++--- 2 files changed, 107 insertions(+), 27 deletions(-)

diff --git a/kdumpctl b/kdumpctl index 6f1afad..345e112 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1315,41 +1315,123 @@ get_grub_kernel_boot_parameter() { grubby --info="$_kernel_path" | sed -n 's/^args="(.*)"$/\1/p' | sed -n "s/^.*${_para}=(\S*).*/\1/p" }

-reset_crashkernel() +_update_crashkernel() {

• local kernel=$1 entry crashkernel_default

• local _entry="$1" _crashkernel="$2" local grub_etc_default="/etc/default/grub"

• [[ -z $kernel ]] && kernel=$(uname -r)
• crashkernel_default=$(cat "/usr/lib/modules/$kernel/crashkernel.default" 2> /dev/null)

This silently removes the support for crashkernel.default again. Please mention it in the commit message.

• if [[ -z $crashkernel_default ]]; then

• derror "$kernel doesn't have a crashkernel.default"
  

• exit 1
  

• fi
• if is_atomic; then if rpm-ostree kargs | grep -q "crashkernel="; then

• 	rpm-ostree --replace="crashkernel=$crashkernel_default"
  

• 	rpm-ostree --replace="crashkernel=$_crashkernel"
  

  else

• 	rpm-ostree --append="crashkernel=$crashkernel_default"
  

• 	rpm-ostree --append="crashkernel=$_crashkernel"
  

  fi else

• entry=$(grubby --info ALL | grep "^kernel=.*$kernel")
  

• entry=${entry#kernel=}
  

• entry=${entry#\"}
  

• entry=${entry%\"}
  

• if [[ -f $grub_etc_default ]]; then

• 	sed -i -e "s/^\(GRUB_CMDLINE_LINUX=.*\)crashkernel=[^\ \"]*\([\ \"].*\)$/\1$crashkernel_default\2/" "$grub_etc_default"
  

• 	sed -i -e "s/^\(GRUB_CMDLINE_LINUX=.*\)crashkernel=[^\ \"]*\([\ \"].*\)$/\1$_crashkernel\2/" "$grub_etc_default"
  

Like in patch 5 this regex is frail to partial matches, i.e. it also matches foo-crashkernel=.

fi

[[ -f /etc/zipl.conf ]] && zipl_arg="--zipl"

• grubby --args "$crashkernel_default" --update-kernel "$entry" $zipl_arg
  

• grubby --args "crashkernel=$_crashkernel" --update-kernel "$_entry" $zipl_arg
  

  [[ $zipl_arg ]] && zipl > /dev/null fi

}

+_find_grubby_kernel_path() +{

• local _kernel="$1" _kernel_path
• if [[ $_kernel == ALL || $_kernel == DEFAULT ]]; then

• _kernel_path=$_kernel
  

• else

• _kernel_path=$(grubby --info ALL | grep "^kernel=.*$_kernel")
  

• _kernel_path=${_kernel_path#kernel=}
  

• _kernel_path=${_kernel_path#\"}
  

• _kernel_path=${_kernel_path%\"}
  

• [[ -z "$_kernel_path" ]] && derror "kernel $_kernel doesn't eixt" && return 1
  

typo s/eixt/exist/

in general can you explain, which values $_kernel (which IIUC is identical to the CRASHKERNEL parameter passed by the user) can have? The way I understand grubby allows either ALL, DEFAULT, vmlinuz-<version> or the path to the vmlinuz file. The last case (path to vmlinuz file) isn't handled correctly here. In particular the path could also be relative, e.g. ../../boot/vmlinuz-<version>, which would break the grep. I guess the easiest solution is to add

[[ -e $_kernel ]] && _kernel=$(realpath $_kernel)

at the beginning of the else block. This would also allow symbolic links to the vmlinuz file, something grubby doesn't support. But I don't think that is a problem.

• fi
• echo -n "$_kernel_path"

+}

+# Besides valid kernel path, grubby also accept DEFAULT and ALL +_valid_grubby_kernel_path() +{

• [[ -e $1 ]] || [[ $1 == ALL ]] || [[ $1 == DEFAULT ]]

-e only checks if the file exists but not if it is a valid kernel grubby can work with. Why not just call grubby and see if it can work with the provided kernel, i.e.

grubby --info="$1" > /dev/null 2>&1

+}

+get_dump_mode_of_kernel() +{

• local _kernel_path=$1 _fadump_val
• _fadump_val=$(get_grub_kernel_boot_parameter "$_kernel_path" fadump)
• if [[ -z $_fadump_val ]] || [[ $_fadump_val == off ]]; then

• echo -n fadump
  

• elif [[ $_fadump_val == on ]] || [[ $_fadump_val == nocma ]]; then

• echo -n kdump
  

When $_fadump_val is "on" the dump mode is kdump and when it's "off" it's "fadump"? Is that correct? From the naming I would expect the opposite. If it is correct could you please add a comment with an explanation. I find that very confusing.

• else

• derror "invalid fadump=$_fadump_val"
  

• exit
  

• fi

+}

+# determine which parameter is kernel_path and crashkernel +# return kernel_path;crashkernel +_determine_kernel_path_and_crashkernel() +{

• local _grub_kernel_path _crashkernel
• if [[ -n "$1" ]] && [[ -n "$2" ]]; then

• _grub_kernel_path=$(_find_grubby_kernel_path "$1") || return 1
  

• if _valid_grubby_kernel_path "$1"; then
  

• 	_grub_kernel_path=$1
  

• else
  

• 	derror "kernel $1 doesn't exist"
  

• 	exit 1
  

• fi
  

• _crashkernel="$2"
  

• elif [[ -n "$1" ]]; then

• if _valid_grubby_kernel_path "$1"; then
  

• 	_grub_kernel_path=$1
  

• else
  

• 	_crashkernel="$1"
  

• fi
  

• fi
• [[ -z "$_grub_kernel_path" ]] && _grub_kernel_path=$(_find_grubby_kernel_path "$(uname -r)")
• echo -n "${_grub_kernel_path};${_crashkernel}"

Introducing this special notation isn't really nice. You could simplify it by simply

echo $_grub_kernel_path $_crashkernel

and then ...

+}

+reset_crashkernel() +{

• local _crashkernel _kernel_crashkernel _grub_kernel_path _tmp_str _dump_mode
• if _tmp_str=$(_determine_kernel_path_and_crashkernel "$1" "$2"); then

• _grub_kernel_path=$(cut -d ";" -f 1 <<< "$_tmp_str")
  

• _crashkernel=$(cut -d ";" -f 2 <<< "$_tmp_str")
  

• else

• exit 1
  

• fi

... change this to

read _grub_kernel_path _crashkernel < <(_determine_kernel_path_and_crashkernel "$1" "$2")

This also removes the superfluous error check above. The way I see it _determine_kernel_path_and_crashkernel will either exit directly or at least return ";". So $tmp_str will always evaluate as true and the the else block will never be reached.

• if [[ -z "$_crashkernel" ]]; then

• _crashkernel=$(kdump_get_conf_val crashkernel)
  

The -z _grub_kernel_path is handled in _determine_kernel_path_and_crashkernel. Wouldn't it make sense to handle both -z cases in the same function?

• else

• kdump_write_conf_val crashkernel "$_crashkernel"
  

• fi

As said in an earlier mail I wouldn't silently update kdump.conf with the new value. Imagine the case

1) a user runs the standard kernel, has kdump setup and everything works fine 2) the user installs kernel-debug and wants to increase the crashkernel memory for this kernel 3) with this kdumpctl updates kdump.conf so that every other standard kernel will use the larger crashkernel memory for the debug kernel unless the user manually resets the value back to the original value.

That's not the behavior I would expect.

Thanks Philipp

• if [[ $_grub_kernel_path == ALL ]]; then

• _dump_mode="$DEFAULT_DUMP_MODE"
  

• else

• _dump_mode=$(get_dump_mode_of_kernel "$_grub_kernel_path")
  

• fi
• if [[ $_crashkernel == "auto" ]] ; then

• _kernel_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_dump_mode")
  

• else

• _kernel_crashkernel="$_crashkernel"
  

• fi
• _update_crashkernel "$_grub_kernel_path" "$_kernel_crashkernel"

+}

if [[ ! -f $KDUMP_CONFIG_FILE ]]; then derror "Error: No kdump config file found!" exit 1 @@ -1412,7 +1494,7 @@ main() get_default_crashkernel "$2" ;; reset-crashkernel)

• reset_crashkernel "$2"
  

• reset_crashkernel "$2" "$3"
  

  ;; *) dinfo $"Usage: $0 {estimate|start|stop|status|restart|reload|rebuild|reset-crashkernel|propagate|showmem}"

diff --git a/kdumpctl.8 b/kdumpctl.8 index 74be062..19734aa 100644 --- a/kdumpctl.8 +++ b/kdumpctl.8 @@ -50,13 +50,11 @@ Estimate a suitable crashkernel value for current machine. This is a best-effort estimate. It will print a recommanded crashkernel value based on current kdump setup, and list some details of memory usage. .TP -.I reset-crashkernel [KERNEL] -Reset crashkernel value to default value. kdumpctl will try to read -from /usr/lib/modules/<KERNEL>/crashkernel.default and reset specified -kernel's crashkernel cmdline value. If no kernel is -specified, will reset current running kernel's crashkernel value. -If /usr/lib/modules/<KERNEL>/crashkernel.default doesn't exist, will -simply exit return 1. +.I reset-crashkernel [[CRASHKERNEL], [KERNELPATH[, CRASHKERNEL]] +Reset crashkernel value to CRASHKERNEL or the value specified in +/etc/kdump.conf. If no kernel is specified, will reset current running +kernel's crashkernel value. If CRASHKERNEL is given, the value would be also +writeen to /etc/kdump.conf.

.SH "SEE ALSO"

Hi Coiby,

On Fri, 3 Dec 2021 18:18:55 +0800 Coiby Xu coxu@redhat.com wrote:

Hi Philipp,

On Wed, Dec 01, 2021 at 04:04:11PM +0100, Philipp Rudo wrote:

...

Hi Coiby,

On Fri, 19 Nov 2021 11:23:04 +0800 Coiby Xu coxu@redhat.com wrote:

[...]

...

...

• if [[ -z $crashkernel_default ]]; then

• derror "$kernel doesn't have a crashkernel.default"
  

• exit 1
  

• fi
• if is_atomic; then if rpm-ostree kargs | grep -q "crashkernel="; then

• 	rpm-ostree --replace="crashkernel=$crashkernel_default"
  

• 	rpm-ostree --replace="crashkernel=$_crashkernel"
  

  else

• 	rpm-ostree --append="crashkernel=$crashkernel_default"
  

• 	rpm-ostree --append="crashkernel=$_crashkernel"
  

  fi else

• entry=$(grubby --info ALL | grep "^kernel=.*$kernel")
  

• entry=${entry#kernel=}
  

• entry=${entry#\"}
  

• entry=${entry%\"}
  

• if [[ -f $grub_etc_default ]]; then

• 	sed -i -e "s/^\(GRUB_CMDLINE_LINUX=.*\)crashkernel=[^\ \"]*\([\ \"].*\)$/\1$crashkernel_default\2/" "$grub_etc_default"
  

• 	sed -i -e "s/^\(GRUB_CMDLINE_LINUX=.*\)crashkernel=[^\ \"]*\([\ \"].*\)$/\1$_crashkernel\2/" "$grub_etc_default"  
  

Like in patch 5 this regex is frail to partial matches, i.e. it also matches foo-crashkernel=.

Taking the case of setting debug kernel's crashkernel value into consideration, I think it's better to drop it. Another reason is grubby would "update the GRUB_CMDLINE_LINUX variable in /etc/default/grub when the --update-kernel option is used with the ALL argument". So there is no need to fix the regex issue in v2.

That's even better :)

...

...

fi

[[ -f /etc/zipl.conf ]] && zipl_arg="--zipl"

• grubby --args "$crashkernel_default" --update-kernel "$entry" $zipl_arg
  

• grubby --args "crashkernel=$_crashkernel" --update-kernel "$_entry" $zipl_arg
  

  [[ $zipl_arg ]] && zipl > /dev/null fi

}

+_find_grubby_kernel_path() +{

• local _kernel="$1" _kernel_path
• if [[ $_kernel == ALL || $_kernel == DEFAULT ]]; then

• _kernel_path=$_kernel
  

• else

• _kernel_path=$(grubby --info ALL | grep "^kernel=.*$_kernel")
  

• _kernel_path=${_kernel_path#kernel=}
  

• _kernel_path=${_kernel_path#\"}
  

• _kernel_path=${_kernel_path%\"}
  

• [[ -z "$_kernel_path" ]] && derror "kernel $_kernel doesn't eixt" && return 1  
  

typo s/eixt/exist/

Thanks for pointing out the typo!

np, thanks for taking care of the problem!

...

in general can you explain, which values $_kernel (which IIUC is identical to the CRASHKERNEL parameter passed by the user) can have?

Sure, I'll add this part in the commit msg next time.

...

The way I understand grubby allows either ALL, DEFAULT, vmlinuz-<version> or the path to the vmlinuz file. The last case (path to vmlinuz file) isn't handled correctly here. In particular the path could also be relative, e.g. ../../boot/vmlinuz-<version>, which would break the grep. I guess the easiest solution is to add

[[ -e $_kernel ]] && _kernel=$(realpath $_kernel)

at the beginning of the else block. This would also allow symbolic links to the vmlinuz file, something grubby doesn't support. But I don't think that is a problem.

It seems grubby don't accept vmlinuz-<version> or relative path ot the vmlinux file,

$ sudo grubby --info=vmlinuz-`uname -r` The param vmlinuz-5.14.14-200.fc34.x86_64 is incorrect

$ sudo grubby --info=./vmlinuz-5.14.14-200.fc34.x86_64 The param ./vmlinuz-5.14.14-200.fc34.x86_64 is incorrect

So we don't need to take care these two cases.

In my test on F35 it works...

# grubby --info=vmlinuz-$(uname -r) index=0 kernel="/boot/vmlinuz-5.15.5-200.fc35.x86_64" args="ro rd.lvm.lv=fedora_fedora/root console=ttyS0 crashkernel=256M" root="/dev/mapper/fedora_fedora-root" initrd="/boot/initramfs-5.15.5-200.fc35.x86_64.img" title="Fedora Linux (5.15.5-200.fc35.x86_64) 35 (Server Edition)" id="ed32f6182bc44595983ddbc6ed06bc92-5.15.5-200.fc35.x86_64"

# pwd && grubby --info=../boot/vmlinuz-$(uname -r) /root index=0 kernel="/boot/vmlinuz-5.15.5-200.fc35.x86_64" args="ro rd.lvm.lv=fedora_fedora/root console=ttyS0 crashkernel=256M" root="/dev/mapper/fedora_fedora-root" initrd="/boot/initramfs-5.15.5-200.fc35.x86_64.img" title="Fedora Linux (5.15.5-200.fc35.x86_64) 35 (Server Edition)" id="ed32f6182bc44595983ddbc6ed06bc92-5.15.5-200.fc35.x86_64"

# ls -l kernel && grubby --info=kernel lrwxrwxrwx. 1 root root 36 Dec 6 15:47 kernel -> /boot/vmlinuz-5.15.5-200.fc35.x86_64 The param kernel is incorrect

...

...

• fi
• echo -n "$_kernel_path"

+}

+# Besides valid kernel path, grubby also accept DEFAULT and ALL +_valid_grubby_kernel_path() +{

• [[ -e $1 ]] || [[ $1 == ALL ]] || [[ $1 == DEFAULT ]]

-e only checks if the file exists but not if it is a valid kernel grubby can work with. Why not just call grubby and see if it can work with the provided kernel, i.e.

grubby --info="$1" > /dev/null 2>&1

Thanks for a better solution!

...

...

+}

+get_dump_mode_of_kernel() +{

• local _kernel_path=$1 _fadump_val
• _fadump_val=$(get_grub_kernel_boot_parameter "$_kernel_path" fadump)
• if [[ -z $_fadump_val ]] || [[ $_fadump_val == off ]]; then

• echo -n fadump
  

• elif [[ $_fadump_val == on ]] || [[ $_fadump_val == nocma ]]; then

• echo -n kdump  
  

When $_fadump_val is "on" the dump mode is kdump and when it's "off" it's "fadump"? Is that correct? From the naming I would expect the opposite. If it is correct could you please add a comment with an explanation. I find that very confusing.

...

• else

• derror "invalid fadump=$_fadump_val"
  

• exit
  

• fi

+}

+# determine which parameter is kernel_path and crashkernel +# return kernel_path;crashkernel +_determine_kernel_path_and_crashkernel() +{

• local _grub_kernel_path _crashkernel
• if [[ -n "$1" ]] && [[ -n "$2" ]]; then

• _grub_kernel_path=$(_find_grubby_kernel_path "$1") || return 1
  

• if _valid_grubby_kernel_path "$1"; then
  

• 	_grub_kernel_path=$1
  

• else
  

• 	derror "kernel $1 doesn't exist"
  

• 	exit 1
  

• fi
  

• _crashkernel="$2"
  

• elif [[ -n "$1" ]]; then

• if _valid_grubby_kernel_path "$1"; then
  

• 	_grub_kernel_path=$1
  

• else
  

• 	_crashkernel="$1"
  

• fi
  

• fi
• [[ -z "$_grub_kernel_path" ]] && _grub_kernel_path=$(_find_grubby_kernel_path "$(uname -r)")
• echo -n "${_grub_kernel_path};${_crashkernel}"

Introducing this special notation isn't really nice. You could simplify it by simply

echo $_grub_kernel_path $_crashkernel

and then ...

...

+}

+reset_crashkernel() +{

• local _crashkernel _kernel_crashkernel _grub_kernel_path _tmp_str _dump_mode
• if _tmp_str=$(_determine_kernel_path_and_crashkernel "$1" "$2"); then

• _grub_kernel_path=$(cut -d ";" -f 1 <<< "$_tmp_str")
  

• _crashkernel=$(cut -d ";" -f 2 <<< "$_tmp_str")
  

• else

• exit 1
  

• fi

... change this to

read _grub_kernel_path _crashkernel < <(_determine_kernel_path_and_crashkernel "$1" "$2")

This also removes the superfluous error check above. The way I see it _determine_kernel_path_and_crashkernel will either exit directly or at least return ";". So $tmp_str will always evaluate as true and the the else block will never be reached.

I designed this by purpose since bash couldn't return two values (in retrospect, an array seems to be a better idea). Because both _grub_kernel_path and _crashkernel could be empty. So if _grub_kernel_path is empty and _crashkernel is not, the following way won't work,

yes, not being able to return two values reliably is a real pain in bash. But you are right, my proposal wouldn't handle situations properly, when _grub_kernel_path and/or _crashkernel are empty.

read _grub_kernel_path _crashkernel < <(_determine_kernel_path_and_crashkernel "$1" "$2")

But in next version, the interface would be implemented as,

kdumpctl reset-crashkernel [--fadump=[on|off|nocma]] [--kernel=path_to_kernel] [--reboot]

and "kumpctl fadump" is not needed. So there is no need to abstract _determine_kernel_path_and_crashkernel.

I'm looking forward to see your v2.

...

...

• if [[ -z "$_crashkernel" ]]; then

• _crashkernel=$(kdump_get_conf_val crashkernel)  
  

The -z _grub_kernel_path is handled in _determine_kernel_path_and_crashkernel. Wouldn't it make sense to handle both -z cases in the same function?

Thanks for the suggestion. Unfortunately, this suggestion couldn't be applied to v2 since the implementation has changed as mentioned before.

np, it's just a minor nit anyway.

...

...

• else

• kdump_write_conf_val crashkernel "$_crashkernel"
  

• fi

As said in an earlier mail I wouldn't silently update kdump.conf with the new value. Imagine the case

1. a user runs the standard kernel, has kdump setup and everything

works fine 2) the user installs kernel-debug and wants to increase the crashkernel memory for this kernel 3) with this kdumpctl updates kdump.conf so that every other standard kernel will use the larger crashkernel memory for the debug kernel unless the user manually resets the value back to the original value.

That's not the behavior I would expect.

Thanks for the suggestion! There is no updating kdump.conf in v2.

Thanks!

Thanks Philipp

...

...

• if [[ $_grub_kernel_path == ALL ]]; then

• _dump_mode="$DEFAULT_DUMP_MODE"
  

• else

• _dump_mode=$(get_dump_mode_of_kernel "$_grub_kernel_path")
  

• fi
• if [[ $_crashkernel == "auto" ]] ; then

• _kernel_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_dump_mode")
  

• else

• _kernel_crashkernel="$_crashkernel"
  

• fi
• _update_crashkernel "$_grub_kernel_path" "$_kernel_crashkernel"

+}

if [[ ! -f $KDUMP_CONFIG_FILE ]]; then derror "Error: No kdump config file found!" exit 1 @@ -1412,7 +1494,7 @@ main() get_default_crashkernel "$2" ;; reset-crashkernel)

• reset_crashkernel "$2"
  

• reset_crashkernel "$2" "$3"
  

  ;; *) dinfo $"Usage: $0 {estimate|start|stop|status|restart|reload|rebuild|reset-crashkernel|propagate|showmem}"

diff --git a/kdumpctl.8 b/kdumpctl.8 index 74be062..19734aa 100644 --- a/kdumpctl.8 +++ b/kdumpctl.8 @@ -50,13 +50,11 @@ Estimate a suitable crashkernel value for current machine. This is a best-effort estimate. It will print a recommanded crashkernel value based on current kdump setup, and list some details of memory usage. .TP -.I reset-crashkernel [KERNEL] -Reset crashkernel value to default value. kdumpctl will try to read -from /usr/lib/modules/<KERNEL>/crashkernel.default and reset specified -kernel's crashkernel cmdline value. If no kernel is -specified, will reset current running kernel's crashkernel value. -If /usr/lib/modules/<KERNEL>/crashkernel.default doesn't exist, will -simply exit return 1. +.I reset-crashkernel [[CRASHKERNEL], [KERNELPATH[, CRASHKERNEL]] +Reset crashkernel value to CRASHKERNEL or the value specified in +/etc/kdump.conf. If no kernel is specified, will reset current running +kernel's crashkernel value. If CRASHKERNEL is given, the value would be also +writeen to /etc/kdump.conf.

.SH "SEE ALSO"

On Fri, Nov 19, 2021 at 11:23:05AM +0800, Coiby Xu wrote:

Say let's add extra 20M to "2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G", add_extra_mem_to_crashkernel will cover the above string to "2G-4G:406M,4G-16G:534M,16G-64G:1046M,64G-128G:2070M,128G-:4118M"

The "crashkernel=" is a practice value. There is no connection between the scale of the different system memory size.

I do not strongly argue, but maybe let user explictly to set a value is a better choice.

Thanks,

Pingfan

Signed-off-by: Coiby Xu coxu@redhat.com

kdump-lib.sh | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)

diff --git a/kdump-lib.sh b/kdump-lib.sh index 94cc8ff..5c79436 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -822,6 +822,41 @@ get_recommend_size() IFS="$OLDIFS" }

+# add memory to crashkernel string +# $1 extra memory size, unit:M +# return crashkernel string (unit=M) +add_extra_mem_to_crashkernel() +{

• local _extra_mem="$1" _ck_cmdline="$2" _new_ck_cmdline
• local _range _mem _size _unit _ele _new_ele
• local OLDIFS="$IFS"
• IFS=','
• for _ele in $_ck_cmdline; do

• _range=$(awk -F ":" '{print $1}' <<< "$_ele")
  

• _mem=$(awk -F ":" '{print $2}' <<< "$_ele")
  

• _size=${_mem::-1}
  

• _unit=${_mem: -1}
  

• if [[ $_unit != "M" ]] && [[ $_unit != "G" ]]; then
  

• 	perror_exit "can't process $_unit"
  

• fi
  

• if [[ $_unit == 'G' ]]; then
  

• 	_size=$((_size * 1024))
  

• fi
  

• _size=$((_size + _extra_mem))
  

• _new_ele="${_range}:${_size}M"
  

• if [[ -z $_new_ck_cmdline ]]; then
  

• 	_new_ck_cmdline=$_new_ele
  

• else
  

• 	_new_ck_cmdline="${_new_ck_cmdline},${_new_ele}"
  

• fi
  

• done
• IFS="$OLDIFS"
• echo -n "$_new_ck_cmdline"

+}

# get default crashkernel # $1 dump mode, if not specified, dump_mode will be judged by is_fadump_capable kdump_get_arch_recommend_crashkernel() -- 2.31.1

On Fri, Nov 19, 2021 at 11:23:07AM +0800, Coiby Xu wrote:

"kdumpctl fadump on/off" would be used to toggle on/off fadump and then call reset-crashkernel.

Signed-off-by: Coiby Xu coxu@redhat.com

kdumpctl | 23 +++++++++++++++++++++++ kdumpctl.8 | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl index 5149cfe..22ee48b 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1458,6 +1458,26 @@ reset_crashkernel() _update_crashkernel "$_grub_kernel_path" "$_kernel_crashkernel" }

+toggle_fadump() +{

• local _grub_kernel_path _crashkernel _tmp_str
• if [[ $1 != on && $1 != nocma && $1 != off ]]; then

• derror "$1 invalid, valid values are on/off/nocma"
  

• exit 1
  

• fi
• if _tmp_str=$(_determine_kernel_path_and_crashkernel "$2" "$3"); then

• _grub_kernel_path=$(cut -d ";" -f 1 <<< "$_tmp_str")
  

• _crashkernel=$(cut -d ";" -f 2 <<< "$_tmp_str")
  

• else

• exit 1
  

• fi
• grubby --args "fadump=$1" --update-kernel "$_grub_kernel_path"
• reset_crashkernel "$_grub_kernel_path" "$_crashkernel"

The kdump_get_arch_recommend_size()->is_fadump_capable() only works if booting with fadump=on.

So here let us say fadump=off -> on, unless I misunderstand, it can get the correct "crashkernel="

Thanks,

Pingfan

+}

if [[ ! -f $KDUMP_CONFIG_FILE ]]; then derror "Error: No kdump config file found!" exit 1 @@ -1522,6 +1542,9 @@ main() reset-crashkernel) reset_crashkernel "$2" "$3" ;;

• fadump)

• toggle_fadump "$2" "$3" "$4"
  

• ;;
  

  *) dinfo $"Usage: $0 {estimate|start|stop|status|restart|reload|rebuild|reset-crashkernel|propagate|showmem}" exit 1

diff --git a/kdumpctl.8 b/kdumpctl.8 index 19734aa..17b10cf 100644 --- a/kdumpctl.8 +++ b/kdumpctl.8 @@ -55,7 +55,9 @@ Reset crashkernel value to CRASHKERNEL or the value specified in /etc/kdump.conf. If no kernel is specified, will reset current running kernel's crashkernel value. If CRASHKERNEL is given, the value would be also writeen to /etc/kdump.conf.

+.TP +.I fadump on/off [[KERNELPATH[, CRASHKERNEL]], [CRASHKERNEL], [KERNELPATH]] +Toggle fadump on or off and reset crashkernel.

.SH "SEE ALSO" .BR kdump.conf (5), -- 2.31.1

On Fri, Nov 19, 2021 at 1:52 PM Coiby Xu coxu@redhat.com wrote:

On Fri, Nov 19, 2021 at 01:25:25PM +0800, Pingfan Liu wrote:

...

On Fri, Nov 19, 2021 at 11:23:07AM +0800, Coiby Xu wrote:

...

"kdumpctl fadump on/off" would be used to toggle on/off fadump and then call reset-crashkernel.

Signed-off-by: Coiby Xu coxu@redhat.com

kdumpctl | 23 +++++++++++++++++++++++ kdumpctl.8 | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl index 5149cfe..22ee48b 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1458,6 +1458,26 @@ reset_crashkernel() _update_crashkernel "$_grub_kernel_path" "$_kernel_crashkernel" }

+toggle_fadump() +{

• local _grub_kernel_path _crashkernel _tmp_str
• if [[ $1 != on && $1 != nocma && $1 != off ]]; then

•        derror "$1 invalid, valid values are on/off/nocma"
  

•        exit 1
  

• fi
• if _tmp_str=$(_determine_kernel_path_and_crashkernel "$2" "$3"); then

•        _grub_kernel_path=$(cut -d ";" -f 1 <<< "$_tmp_str")
  

•        _crashkernel=$(cut -d ";" -f 2 <<< "$_tmp_str")
  

• else

•        exit 1
  

• fi
• grubby --args "fadump=$1" --update-kernel "$_grub_kernel_path"
• reset_crashkernel "$_grub_kernel_path" "$_crashkernel"

The kdump_get_arch_recommend_size()->is_fadump_capable() only works if booting with fadump=on.

This interface doesn't use kdump_get_arch_recommend_size. To determine if fadump is enabled for a kernel, the only reliably way is to get it from grubby. For example, a user enables fadump for a kernel by running "kumpctl fadump on" but is_fadump_capable is false in this case before rebooting the system.

...

So here let us say fadump=off -> on, unless I misunderstand, it can get the correct "crashkernel="

As the above question, could you enlighten me about how to get the correct "crashkernel=" in the series ?

Thanks

...

Thanks,

  Pingfan

...

+}

if [[ ! -f $KDUMP_CONFIG_FILE ]]; then derror "Error: No kdump config file found!" exit 1 @@ -1522,6 +1542,9 @@ main() reset-crashkernel) reset_crashkernel "$2" "$3" ;;

• fadump)

•        toggle_fadump "$2" "$3" "$4"
  

•        ;;
  

  *) dinfo $"Usage: $0 {estimate|start|stop|status|restart|reload|rebuild|reset-crashkernel|propagate|showmem}" exit 1

diff --git a/kdumpctl.8 b/kdumpctl.8 index 19734aa..17b10cf 100644 --- a/kdumpctl.8 +++ b/kdumpctl.8 @@ -55,7 +55,9 @@ Reset crashkernel value to CRASHKERNEL or the value specified in /etc/kdump.conf. If no kernel is specified, will reset current running kernel's crashkernel value. If CRASHKERNEL is given, the value would be also writeen to /etc/kdump.conf.

+.TP +.I fadump on/off [[KERNELPATH[, CRASHKERNEL]], [CRASHKERNEL], [KERNELPATH]] +Toggle fadump on or off and reset crashkernel.

.SH "SEE ALSO" .BR kdump.conf (5), -- 2.31.1

-- Best regards, Coiby

On Fri, Nov 19, 2021 at 2:42 PM Coiby Xu coxu@redhat.com wrote:

On Fri, Nov 19, 2021 at 02:20:44PM +0800, Pingfan Liu wrote:

...

On Fri, Nov 19, 2021 at 1:52 PM Coiby Xu coxu@redhat.com wrote:

...

On Fri, Nov 19, 2021 at 01:25:25PM +0800, Pingfan Liu wrote:

...

On Fri, Nov 19, 2021 at 11:23:07AM +0800, Coiby Xu wrote:

...

"kdumpctl fadump on/off" would be used to toggle on/off fadump and then call reset-crashkernel.

Signed-off-by: Coiby Xu coxu@redhat.com

kdumpctl | 23 +++++++++++++++++++++++ kdumpctl.8 | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl index 5149cfe..22ee48b 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1458,6 +1458,26 @@ reset_crashkernel() _update_crashkernel "$_grub_kernel_path" "$_kernel_crashkernel" }

+toggle_fadump() +{

• local _grub_kernel_path _crashkernel _tmp_str
• if [[ $1 != on && $1 != nocma && $1 != off ]]; then

•        derror "$1 invalid, valid values are on/off/nocma"
  

•        exit 1
  

• fi
• if _tmp_str=$(_determine_kernel_path_and_crashkernel "$2" "$3"); then

•        _grub_kernel_path=$(cut -d ";" -f 1 <<< "$_tmp_str")
  

•        _crashkernel=$(cut -d ";" -f 2 <<< "$_tmp_str")
  

• else

•        exit 1
  

• fi
• grubby --args "fadump=$1" --update-kernel "$_grub_kernel_path"
• reset_crashkernel "$_grub_kernel_path" "$_crashkernel"

The kdump_get_arch_recommend_size()->is_fadump_capable() only works if booting with fadump=on.

This interface doesn't use kdump_get_arch_recommend_size. To determine if fadump is enabled for a kernel, the only reliably way is to get it from grubby. For example, a user enables fadump for a kernel by running "kumpctl fadump on" but is_fadump_capable is false in this case before rebooting the system.

...

So here let us say fadump=off -> on, unless I misunderstand, it can get the correct "crashkernel="

As the above question, could you enlighten me about how to get the correct "crashkernel=" in the series ?

For crashkernel=auto, there are three cases where we need to get crashkernel value,

1. when installing OS

kdump-anaconda-addon will call "kdumpctl get-default-crashkernel" to get the value. If the user chooses to enable fadump, the addon would call "kdumpctl get-default-crashkernel fadump" instead.

2. when the user toggles fadump on

a) kdumpctl would add "fadump=on" to the specified kernel cmdline. b) reset_crashkernel would call get_dump_mode_of_kernel which would return fadump since fadump=on is found in the kernel cmdline. c) Then kdump_get_arch_recommend_crashkernel "$_dump_mode" would return the fadump default crashkernel.

This is what I was concerned about. And I think you can avoid to call grubby twice by taking the "1" way, something like "kdumpctl get-default-crashkernel fadump=on"

Thanks,

Pingfan

On Fri, Nov 19, 2021 at 01:13:48PM +0800, Pingfan Liu wrote:

On Fri, Nov 19, 2021 at 11:23:08AM +0800, Coiby Xu wrote:

...

The installation hook can call "kdumpctl reset-crashkernel KERNELIMAGE" to set crashkernel for new kernel. Also remove crashkenel.default related code.

Signed-off-by: Coiby Xu coxu@redhat.com

92-crashkernel.install | 135 +---------------------------------------- 1 file changed, 1 insertion(+), 134 deletions(-)

diff --git a/92-crashkernel.install b/92-crashkernel.install index 78365ff..b8025e4 100755 --- a/92-crashkernel.install +++ b/92-crashkernel.install @@ -5,142 +5,9 @@ KERNEL_VERSION="$2" KDUMP_INITRD_DIR_ABS="$3" KERNEL_IMAGE="$4"

-grub_etc_default="/etc/default/grub"

-ver_lt() {

• [[ "$(echo -e "$1\n$2" | sort -V)" == $1$'\n'* ]] && [[ $1 != "$2" ]]

-}

-# Read crashkernel= value in /etc/default/grub -get_grub_etc_ck() {

• [[ -e $grub_etc_default ]] && \
• sed -n -e "s/^GRUB_CMDLINE_LINUX=.*(crashkernel=[^\ "]*)[\ "].*$/\1/p" $grub_etc_default

-}

-# Read crashkernel.default value of specified kernel -get_ck_default() {

• ck_file="/usr/lib/modules/$1/crashkernel.default"
• [[ -f "$ck_file" ]] && cat "$ck_file"

-}

-# Iterate installed kernels, find the kernel with the highest version that has a -# valid crashkernel.default file, exclude current installing/removing kernel -# -# $1: a string representing a crashkernel= cmdline. If given, will also check the -# content of crashkernel.default, only crashkernel.default with the same value will match -get_highest_ck_default_kver() {

• for kernel in $(find /usr/lib/modules -maxdepth 1 -mindepth 1 -printf "%f\n" | sort --version-sort -r); do

• [[ $kernel == "$KERNEL_VERSION" ]] && continue
  

• [[ -s "/usr/lib/modules/$kernel/crashkernel.default" ]] || continue
  

• echo "$kernel"
  

• return 0
  

• done
• return 1

-}

-set_grub_ck() {

• sed -i -e "s/^(GRUB_CMDLINE_LINUX=.*)crashkernel=[^\ "]*([\ "].*)$/\1$1\2/" "$grub_etc_default"

-}

-# Set specified kernel's crashkernel cmdline value -set_kernel_ck() {

• kernel=$1
• ck_cmdline=$2
• entry=$(grubby --info ALL | grep "^kernel=.*$kernel")
• entry=${entry#kernel=}
• entry=${entry#"}
• entry=${entry%"}
• if [[ -z "$entry" ]]; then

• echo "$0: failed to find boot entry for kernel $kernel"
  

• return 1
  

• fi
• [[ -f /etc/zipl.conf ]] && zipl_arg="--zipl"
• grubby --args "$ck_cmdline" --update-kernel "$entry" $zipl_arg
• [[ $zipl_arg ]] && zipl > /dev/null ||:

-}

case "$COMMAND" in add)

• # - If current boot kernel is using default crashkernel value, update
• # installing kernel's crashkernel value to its default value,
• # - If intalling a higher version kernel, and /etc/default/grub's
• # crashkernel value is using default value, update it to installing
• # kernel's default value.
• inst_ck_default=$(get_ck_default "$KERNEL_VERSION")
• # If installing kernel doesn't have crashkernel.default, just exit.
• [[ -z "$inst_ck_default" ]] && exit 0
• boot_kernel=$(uname -r)
• boot_ck_cmdline=$(sed -n -e "s/^.*(crashkernel=\S*).*$/\1/p" /proc/cmdline)
• highest_ck_default_kver=$(get_highest_ck_default_kver)
• highest_ck_default=$(get_ck_default "$highest_ck_default_kver")
• # Try update /etc/default/grub if present, else grub2-mkconfig could
• # override crashkernel value.
• grub_etc_ck=$(get_grub_etc_ck)
• if [[ -n "$grub_etc_ck" ]]; then

• if [[ -z "$highest_ck_default_kver" ]]; then
  

• 	# None of installed kernel have a crashkernel.default,
  

• 	# check for 'crashkernel=auto' in case of legacy kernel
  

• 	[[ "$grub_etc_ck" == "crashkernel=auto" ]] && \
  

• 		set_grub_ck "$inst_ck_default"
  

• else
  

• 	# There is a valid crashkernel.default, check if installing kernel
  

• 	# have a higher version and grub config is using default value
  

• 	ver_lt "$highest_ck_default_kver" "$KERNEL_VERSION" && \
  

• 		[[ "$grub_etc_ck" == "$highest_ck_default" ]] && \
  

• 		[[ "$grub_etc_ck" != "$inst_ck_default" ]] && \
  

• 		set_grub_ck "$inst_ck_default"
  

• fi
  

• fi
• # Exit if crashkernel is not used in current cmdline
• [[ -z $boot_ck_cmdline ]] && exit 0
• # Get current boot kernel's default value
• boot_ck_default=$(get_ck_default "$boot_kernel")
• if [[ $boot_ck_cmdline == "crashkernel=auto" ]]; then

• # Legacy RHEL kernel defaults to "auto"
  

• boot_ck_default="$boot_ck_cmdline"
  

• fi
• # If boot kernel doesn't have a crashkernel.default, check
• # if it's using any installed kernel's crashkernel.default
• if [[ -z $boot_ck_default ]]; then

• [[ $(get_highest_ck_default_kver "$boot_ck_cmdline") ]] && boot_ck_default="$boot_ck_cmdline"
  

• fi
• # If boot kernel is using a default crashkernel, update
• # installing kernel's crashkernel to new default value
• if [[ "$boot_ck_cmdline" != "$inst_ck_default" ]] && [[ "$boot_ck_cmdline" == "$boot_ck_default" ]]; then

• set_kernel_ck "$KERNEL_VERSION" "$inst_ck_default"
  

• fi
• exit 0
• ;;

-remove)

• # If grub default value is upgraded when this kernel was installed, try downgrade it
• grub_etc_ck=$(get_grub_etc_ck)
• [[ $grub_etc_ck ]] || exit 0
• removing_ck_conf=$(get_ck_default "$KERNEL_VERSION")
• [[ $removing_ck_conf ]] || exit 0
• highest_ck_default_kver=$(get_highest_ck_default_kver) || exit 0
• highest_ck_default=$(get_ck_default "$highest_ck_default_kver")
• [[ $highest_ck_default ]] || exit 0
• if ver_lt "$highest_ck_default_kver" "$KERNEL_VERSION"; then

• if [[ $grub_etc_ck == "$removing_ck_conf" ]] && [[ $grub_etc_ck != "$highest_ck_default" ]]; then
  

• 	set_grub_ck "$highest_ck_default"
  

• fi
  

• fi

• kdumpctl set-crashkernel "$KERNEL_IMAGE"

        ^^^ reset?

Yes, it should be reset-crashkernel. Thanks for spotting this mistake!

...

exit 0 ;; esac -- 2.31.1