Hi Philipp,
On Tue, Feb 22, 2022 at 10:15:30AM +0100, Philipp Rudo wrote:
Hi Coiby,
the code looks good to me. A few typos though.
Reviewed-by: Philipp Rudo <prudo(a)redhat.com>
On Mon, 21 Feb 2022 15:21:15 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
> If GRUB_ETC_DEFAULT use crashkernel=auto or
> crashkernel=OLD_DEFAULT_CRASHKERNEL, it should be updated as well.
>
> Note a helper function to read kernel cmdline parameter from
s/Note/Add/ ?
> GRUB_ETC_DEFAULT. This function would be used to read kernel cmdline
s/would be/is/
> parameter like fadump or crashkernel.
>
> Signed-off-by: Coiby Xu <coxu(a)redhat.com>
> ---
> kdumpctl | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/kdumpctl b/kdumpctl
> index d9edc4b..92451aa 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -1472,6 +1472,16 @@ _remove_kernel_arg_in_grub_etc_default()
> }" "$GRUB_ETC_DEFAULT"
> }
>
> +# Read the kernel arg in default grub conf.
> +
> +# Note removing a kernel parameter that doesn't have a value isn't
supported.
s/removing/reading/
Thanks for reviewing the patch and catching these typos.
Thanks
Philipp
> +#
> +# $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 _dump_mode _fadump_val _reboot _grubby_kernel_path _kernel
_kernels
> @@ -1606,6 +1616,34 @@ reset_crashkernel()
> fi
> }
>
> +# update the crashkernel value in GRUB_ETC_DEFAULT if necessary
> +#
> +# called by reset_crashkernel_after_update and inherit its array variable
> +# _crashkernel_vals
> +update_crashkernel_in_grub_etc_default_after_update()
> +{
> + local _crashkernel _fadump_val
> + local _dump_mode _old_default_crashkernel _new_default_crashkernel
> +
> + _crashkernel=$(_read_kernel_cmdline_in_grub_etc_default crashkernel)
> +
> + if [[ -z $_crashkernel ]]; then
> + return
> + fi
> +
> + _fadump_val=$(read_kernel_cmdline_in_grub_etc_default fadump)
> + _dump_mode=$(get_dump_mode_by_fadump_val "$_fadump_val")
> +
> + _old_default_crashkernel=${_crashkernel_vals[old_${_dump_mode}]}
> + _new_default_crashkernel=${_crashkernel_vals[new_${_dump_mode}]}
> +
> + if [[ $_crashkernel == auto ]] ||
> + [[ $_crashkernel == "$_old_default_crashkernel" &&
> + $_new_default_crashkernel != "$_old_default_crashkernel" ]]; then
> + _add_kernel_arg_in_grub_etc_default crashkernel
"$_new_default_crashkernel"
> + fi
> +}
> +
> # shellcheck disable=SC2154 # false positive when dereferencing an array
> reset_crashkernel_after_update()
> {
> @@ -1634,6 +1672,8 @@ reset_crashkernel_after_update()
> fi
> fi
> done
> +
> + update_crashkernel_in_grub_etc_default_after_update
> }
>
> # read the value of an environ variable from given environ file path
--
Best regards,
Coiby