7:10 a.m.
On Mon, Dec 13, 2021 at 03:26:13PM +0100, Philipp Rudo wrote:
On Wed, 8 Dec 2021 10:25:36 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
> kexec-tools could update the default crashkernel value.
> When auto_reset_crashkernel=yes, reset kernel to new crashkernel
> value in the following two cases,
> - crashkernel=auto is found in the kernel cmdline
> - the kernel crashkernel was previously set by kexec-tools i.e.
> the kernel is using old default crashkernel value
>
> To tell if the user is using a custom value for the kernel crashkernel
> or not, we assume the user would never use the default crashkernel value
> as custom value. When kexec-tools gets updated,
> 1. save the default crashkernel value of the older package to
> /tmp/crashkernel (for POWER system, /tmp/crashkernel_fadump is saved
> as well).
> 2. If auto_reset_crashkernel=yes, iterate all installed kernels.
> For each kernel, compare its crashkernel value with the old
> default crashkernel and reset it if yes
>
> The implementation makes use of two RPM scriptlets [2],
> - %pre is run before a package is installed so we can use it to save
> old default crashkernel value
> - %post is run after a package installed so we can use it to try to reset
> kernel crashkernel
>
> Note latest shellcheck (0.8.0) gives false positives about the
> associative array as of this commit. And Fedora's shellcheck is 0.7.2
> and can't even correctly parse the shell code because of the associative
> array.
>
> [1] https://github.com/koalaman/shellcheck/issues/2399
> [2] https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/
>
> Signed-off-by: Coiby Xu <coxu(a)redhat.com>
> ---
> kdumpctl | 35 +++++++++++++++++++++++++++++++++++
> kexec-tools.spec | 15 +++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/kdumpctl b/kdumpctl
> index 26358d2..070f777 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -1480,6 +1480,36 @@ reset_crashkernel()
> dwarn "For kernel=$_grubby_kernel_path, crashkernel=$_kernel_crashkernel now.
Please reboot the system to take effect."
> }
>
> +# shellcheck disable=SC2154 # false positive when dereferencing an array
> +reset_crashkernel_after_update()
> +{
> + local _kernel _crashkernel _dump_mode _fadump_val _old_default_crashkernel
_new_default_crashkernel
> + declare -A _crashkernel_vals
> +
> + _crashkernel_vals[old_kdump]=$(cat /tmp/old_default_crashkernel 2>/dev/null)
> + _crashkernel_vals[old_fadump]=$(cat /tmp/old_default_crashkernel_fadump
2>/dev/null)
> + _crashkernel_vals[new_kdump]=$(get_default_crashkernel kdump)
> + _crashkernel_vals[new_fadump]=$(get_default_crashkernel fadump)
Are you planning to use the array more often for the CoreOS support?
CoreOS may need it. But in this patch, I haven't taken CoreOS into
consideration.
Because at the moment I find it a little bit overkill for what it
does.
Especially as you still need variables for
_{old,new}_default_crashkernel. So having this array is somewhat
redundant at the moment.
Initially, the next patch "[PATCH v2 10/10] reset kernel crashkernel for
the special case where the kernel is updated right after kexec-tools"
also needed to get the old and new default crashkernel values so using
array to pass multiple values tends to be a good idea. But later I
realized there is no need to do so. The reason I still use the array
here is it simplify the code, i.e. I can use an variable like
"old_${_dump_mode}]"
as the key to access an array element thus no need to use if here. If I
don't use array here, the code would as follows,
if [[ $_crashkernel == auto ]]; then
reset_crashkernel "--kernel=$_kernel"
elif [[ -n $_crashkernel ]]; then
_dump_mode=$(get_dump_mode_by_kernel "$_kernel")
if [[ $_dump_mode == kdump ]]; then
_old_default_crashkernel=$_old_kdump_crashkernel_val
_new_default_crashkernel=$_new_kdump_crashkernel_val
else
_old_default_crashkernel=$_old_fadump_crashkernel_val
_new_default_crashkernel=$_new_fadump_crashkernel_val
fi
if [[ $_crashkernel == "$_old_default_crashkernel" ]] &&
[[ $_new_default_crashkernel != "$_old_default_crashkernel" ]]; then
_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
if _update_grub "$_kernel" "$_new_default_crashkernel"
"$_dump_mode" "$_fadump_val"; then
echo "For kernel=$_kernel, crashkernel=$_new_default_crashkernel now."
fi
fi
fi
But I don't know if using an array could bring some drawbacks. Please let
me know if there could me.
> + for _kernel in $(_get_all_kernels_from_grubby); do
> + _crashkernel=$(get_grub_kernel_boot_parameter "$_kernel" crashkernel)
> + if [[ $_crashkernel == auto ]]; then
> + reset_crashkernel "--kernel=$_kernel"
> + elif [[ -n $_crashkernel ]]; then
> + _dump_mode=$(get_dump_mode_by_kernel "$_kernel")
> + _old_default_crashkernel=${_crashkernel_vals[old_${_dump_mode}]}
> + _new_default_crashkernel=${_crashkernel_vals[new_${_dump_mode}]}
> + if [[ $_crashkernel == "$_old_default_crashkernel" ]] &&
> + [[ $_new_default_crashkernel != "$_old_default_crashkernel" ]]; then
> + _fadump_val=$(get_grub_kernel_boot_parameter "$_kernel" fadump)
> + if _update_grub "$_kernel" "$_new_default_crashkernel"
"$_dump_mode" "$_fadump_val"; then
> + echo "For kernel=$_kernel, crashkernel=$_new_default_crashkernel
now."
> + fi
> + fi
> + fi
> + done
> +}
> +
> if [[ ! -f $KDUMP_CONFIG_FILE ]]; then
> derror "Error: No kdump config file found!"
> exit 1
> @@ -1545,6 +1575,11 @@ main()
> shift
> reset_crashkernel "$@"
> ;;
> + reset-crashkernel-after-update)
> + if [[ $(kdump_get_conf_val auto_reset_crashkernel) != no ]]; then
> + reset_crashkernel_after_update
> + fi
> + ;;
> *)
> dinfo $"Usage: $0
{estimate|start|stop|status|restart|reload|rebuild|reset-crashkernel|propagate|showmem}"
> exit 1
> diff --git a/kexec-tools.spec b/kexec-tools.spec
> index ab7f41f..e346062 100644
> --- a/kexec-tools.spec
> +++ b/kexec-tools.spec
> @@ -258,6 +258,14 @@ chmod 755
$RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/99zz-fadumpini
> mkdir -p $RPM_BUILD_ROOT/%{dracutlibdir}/modules.d/
> mv $RPM_BUILD_ROOT/etc/kdump-adv-conf/kdump_dracut_modules/*
$RPM_BUILD_ROOT/%{dracutlibdir}/modules.d/
>
> +%pre
> +if [ $1 == 2 ] && grep "get-default-crashkernel" /usr/bin/kdumpctl
&>/dev/null; then
> + kdumpctl get-default-crashkernel > /tmp/old_default_crashkernel 2>/dev/null
> +%ifarch ppc64 ppc64le
> + kdumpctl get_default_crashkernel fadump > /tmp/old_default_crashkernel_fadump
2>/dev/null
^^^^^^^^^^^^^^^^^^^^^^^
s/_/-/g
Thanks for catching this mistake. Fixed in v3.
Thanks
Philipp
> +%endif
> +fi
> +
> %post
> # Initial installation
> %systemd_post kdump.service
> @@ -290,6 +298,13 @@ then
> /etc/sysconfig/kdump > /etc/sysconfig/kdump.new
> mv /etc/sysconfig/kdump.new /etc/sysconfig/kdump
> fi
> +if [ $1 == 2 ]; then
> + kdumpctl reset-crashkernel-after-update
> + rm /tmp/old_default_crashkernel 2>/dev/null
> +%ifarch ppc64 ppc64le
> + rm /tmp/old_default_crashkernel_fadump 2>/dev/null
> +%endif
> +fi
>
>
> %postun
--
Best regards,
Coiby