kdumpctl: simplify _update_kernel_cmdline

_update_kernel_cmdline handles two cmdline parameters at once. This does not
only make the function itself but also its callers more complicated than
necessary. For example in _update_crashkernel the fadump gets "updated" to
the value that has been read from grubby. Thus simplify
_update_kernel_cmdline to only update one parameter at once.

While at it shorten some variable named in the callers.

Signed-off-by: Philipp Rudo <prudo@redhat.com>
Reviewed-by: Pingfan Liu <piliu@redhat.com>
This commit is contained in:
Philipp Rudo 2023-09-06 10:49:41 +02:00 committed by Coiby Xu
parent 1049e1c79c
commit f5785c60aa
3 changed files with 67 additions and 51 deletions

108
kdumpctl
View File

@ -1424,21 +1424,33 @@ _find_kernel_path_by_release()
_update_kernel_cmdline() _update_kernel_cmdline()
{ {
local _kernel_path=$1 _crashkernel=$2 _dump_mode=$3 _fadump_val=$4 local _kernel _param _old _new
_kernel="$1"
_param="$2"
_old="$3"
_new="$4"
# _new = "" removes a _param, so _new = _old = "" is fine
[[ -n "$_new" ]] && [[ "$_old" == "$_new" ]] && return 1
if is_ostree; then if is_ostree; then
if rpm-ostree kargs | grep -q "crashkernel="; then if [[ -z "$_new" ]]; then
rpm-ostree kargs --replace="crashkernel=$_crashkernel" rpm-ostree kargs --delete="$_param=$_old"
elif [[ -n "$_old" ]]; then
rpm-ostree kargs --replace="$_param=$_new"
else else
rpm-ostree kargs --append="crashkernel=$_crashkernel" rpm-ostree kargs --append="$_param=$_new"
fi
elif has_command grubby; then
if [[ -n "$_new" ]]; then
grubby --update-kernel "$_kernel" --args "$_param=$_new"
else
grubby --update-kernel "$_kernel" --remove-args "$_param"
fi fi
else else
grubby --args "crashkernel=$_crashkernel" --update-kernel "$_kernel_path" derror "Unable to update kernel command line"
if [[ $_dump_mode == kdump ]]; then exit 1
grubby --remove-args="fadump" --update-kernel "$_kernel_path"
else
grubby --args="fadump=$_fadump_val" --update-kernel "$_kernel_path"
fi
fi fi
# Don't use "[[ CONDITION ]] && COMMAND" which returns 1 under false CONDITION # Don't use "[[ CONDITION ]] && COMMAND" which returns 1 under false CONDITION
@ -1530,9 +1542,11 @@ _read_kernel_arg_in_grub_etc_default()
reset_crashkernel() reset_crashkernel()
{ {
local _opt _val _dump_mode _fadump_val _reboot _grubby_kernel_path _kernel _kernels local _opt _val _fadump_val _reboot _grubby_kernel_path _kernel _kernels
local _old_crashkernel _new_crashkernel _new_dump_mode _crashkernel_changed local _dump_mode _new_dump_mode
local _new_fadump_val _old_fadump_val _what_is_updated local _has_changed _needs_reboot
local _old_ck _new_ck
local _old_fadump _new_fadump
for _opt in "$@"; do for _opt in "$@"; do
case "$_opt" in case "$_opt" in
@ -1570,14 +1584,10 @@ reset_crashkernel()
# modifying the kernel command line so there is no need for kexec-tools # modifying the kernel command line so there is no need for kexec-tools
# to repeat it. # to repeat it.
if is_ostree; then if is_ostree; then
_old_crashkernel=$(rpm-ostree kargs | sed -n -E 's/.*(^|\s)crashkernel=(\S*).*/\2/p') _old_ck=$(rpm-ostree kargs | sed -n -E 's/.*(^|\s)crashkernel=(\S*).*/\2/p')
_new_dump_mode=kdump _new_ck=$(kdump_get_arch_recommend_crashkernel kdump)
_new_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") if _update_kernel_cmdline "" crashkernel "$_old_ck" "$_new_ck"; then
if [[ $_old_crashkernel != "$_new_crashkernel" ]]; then [[ $_reboot == yes ]] && systemctl reboot
_update_kernel_cmdline "" "$_new_crashkernel" "$_new_dump_mode" ""
if [[ $_reboot == yes ]]; then
systemctl reboot
fi
fi fi
return return
fi fi
@ -1626,35 +1636,33 @@ reset_crashkernel()
fi fi
for _kernel in $_kernels; do for _kernel in $_kernels; do
_has_changed=""
if [[ -z $_dump_mode ]]; then if [[ -z $_dump_mode ]]; then
_new_dump_mode=$(get_dump_mode_by_kernel "$_kernel") _new_dump_mode=$(get_dump_mode_by_kernel "$_kernel")
_new_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode") _new_ck=$(kdump_get_arch_recommend_crashkernel "$_new_dump_mode")
_new_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump) _new_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
else else
_new_dump_mode=$_dump_mode _new_dump_mode=$_dump_mode
_new_crashkernel=$_crashkernel _new_ck=$_crashkernel
_new_fadump_val=$_fadump_val _new_fadump=$_fadump_val
fi fi
_old_crashkernel=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) _old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel)
_old_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump) _old_fadump=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
if [[ $_old_crashkernel != "$_new_crashkernel" || $_old_fadump_val != "$_new_fadump_val" ]]; then if _update_kernel_cmdline "$_kernel" fadump "$_old_fadump" "$_new_fadump"; then
_update_kernel_cmdline "$_kernel" "$_new_crashkernel" "$_new_dump_mode" "$_new_fadump_val" _has_changed="Updated fadump=$_new_fadump"
if [[ $_reboot != yes ]]; then fi
if [[ $_old_crashkernel != "$_new_crashkernel" ]]; then if _update_kernel_cmdline "$_kernel" crashkernel "$_old_ck" "$_new_ck"; then
_what_is_updated="Updated crashkernel=$_new_crashkernel" _has_changed="Updated crashkernel=$_new_ck"
else fi
# This case happens only when switching between fadump=on and fadump=nocma if [[ -n "$_has_changed" ]]; then
_what_is_updated="Updated fadump=$_new_fadump_val" _needs_reboot=1
fi dwarn "$_has_changed for kernel=$_kernel. Please reboot the system for the change to take effect."
dwarn "$_what_is_updated for kernel=$_kernel. Please reboot the system for the change to take effect."
fi
_crashkernel_changed=yes
fi fi
done done
if [[ $_reboot == yes && $_crashkernel_changed == yes ]]; then if [[ $_reboot == yes && $_needs_reboot == 1 ]]; then
reboot systemctl reboot
fi fi
} }
@ -1670,21 +1678,19 @@ _is_bootloader_installed()
_update_crashkernel() _update_crashkernel()
{ {
local _kernel _kver _dump_mode _old_default_crashkernel _new_default_crashkernel _fadump_val _msg local _kernel _kver _dump_mode _msg
local _old_ck _new_ck
_kernel=$1 _kernel=$1
_dump_mode=$(get_dump_mode_by_kernel "$_kernel") _dump_mode=$(get_dump_mode_by_kernel "$_kernel")
_old_default_crashkernel=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel) _old_ck=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel)
_kver=$(parse_kver_from_path "$_kernel") _kver=$(parse_kver_from_path "$_kernel")
# The second argument is for the case of aarch64, where installing a 64k variant on a 4k kernel, or vice versa # The second argument is for the case of aarch64, where installing a 64k variant on a 4k kernel, or vice versa
_new_default_crashkernel=$(kdump_get_arch_recommend_crashkernel "$_dump_mode" "$_kver") _new_ck=$(kdump_get_arch_recommend_crashkernel "$_dump_mode" "$_kver")
if [[ $_old_default_crashkernel != "$_new_default_crashkernel" ]]; then if _update_kernel_cmdline "$_kernel" crashkernel "$_old_ck" "$_new_ck"; then
_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump) _msg="For kernel=$_kernel, crashkernel=$_new_ck now. Please reboot the system for the change to take effect."
if _update_kernel_cmdline "$_kernel" "$_new_default_crashkernel" "$_dump_mode" "$_fadump_val"; then _msg+=" Note if you don't want kexec-tools to manage the crashkernel kernel parameter, please set auto_reset_crashkernel=no in /etc/kdump.conf."
_msg="For kernel=$_kernel, crashkernel=$_new_default_crashkernel now. Please reboot the system for the change to take effet." dinfo "$_msg"
_msg+=" Note if you don't want kexec-tools to manage the crashkernel kernel parameter, please set auto_reset_crashkernel=no in /etc/kdump.conf."
dinfo "$_msg"
fi
fi fi
} }

View File

@ -52,6 +52,11 @@ Describe 'Management of the kernel crashkernel parameter.'
@grubby --no-etc-grub-update --grub2 --config-file="$GRUB_CFG" --bad-image-okay --env="$KDUMP_SPEC_TEST_RUN_DIR"/env_temp -b "$KDUMP_SPEC_TEST_RUN_DIR"/boot_load_entries "$@" @grubby --no-etc-grub-update --grub2 --config-file="$GRUB_CFG" --bad-image-okay --env="$KDUMP_SPEC_TEST_RUN_DIR"/env_temp -b "$KDUMP_SPEC_TEST_RUN_DIR"/boot_load_entries "$@"
} }
# The mocking breaks has_command. Mock it as well to fix the tests.
has_command() {
[[ "$1" == grubby ]]
}
Describe "When kexec-tools have its default crashkernel updated, " Describe "When kexec-tools have its default crashkernel updated, "
Context "if kexec-tools is updated alone, " Context "if kexec-tools is updated alone, "

View File

@ -29,6 +29,11 @@ Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]'
@grubby --no-etc-grub-update --grub2 --bad-image-okay --env="$KDUMP_SPEC_TEST_RUN_DIR"/env_temp -b "$KDUMP_SPEC_TEST_RUN_DIR"/boot_load_entries "$@" @grubby --no-etc-grub-update --grub2 --bad-image-okay --env="$KDUMP_SPEC_TEST_RUN_DIR"/env_temp -b "$KDUMP_SPEC_TEST_RUN_DIR"/boot_load_entries "$@"
} }
# The mocking breaks has_command. Mock it as well to fix the tests.
has_command() {
[[ "$1" == grubby ]]
}
Describe "Test the kdump dump mode " Describe "Test the kdump dump mode "
uname() { uname() {
if [[ $1 == '-m' ]]; then if [[ $1 == '-m' ]]; then