On Thu, Dec 23, 2021 at 03:07:16PM +0800, Coiby Xu wrote:
Hi Philipp,
On Wed, Dec 22, 2021 at 05:29:22PM +0100, Philipp Rudo wrote:
>Hi Coiby,
>
>On Thu, 16 Dec 2021 14:36:52 +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
>>
>>There are several problems when running kdumpctl in the RPM scripts
>>for CoreOS/Atomic/Silverblue, for example, the lock can't be acquired by
>>kdumpctl, "rpm-ostree kargs" can't be run and etc.. So don't
enable this
>>feature for CoreOS/Atomic/Silverblue.
>>
>>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 412420d..a6582f8 100755
>>--- a/kdumpctl
>>+++ b/kdumpctl
>>@@ -1534,6 +1534,36 @@ reset_crashkernel()
>> [[ $_reboot == yes && $_crashkernel_changed == yes ]] && reboot
>> }
>>
>>+# 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)
>>+
>>+ 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
>>@@ -1599,6 +1629,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..d72044a 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 ! grep -q "ostree" /proc/cmdline && [ $1 == 2 ] &&
grep "get-default-crashkernel" /usr/bin/kdumpctl &>/dev/null; then
>
>do you really need to check if get-default-crashkernel exists here?
>What would happen if kdumpctl returns with an error?
>
>I'm just asking myself as there is no corresponding check in the %post
>scriplet. Furthermore I think we can assume that the spec file and
>kdumpctl are updated together. Especially that using a new spec file
>with an old kdumpctl that doesn't support get-default-crashkernel is
>not a supported use case. Or am I missing something?
Using using a new spec file with an old kdumpctl does happen for the
%pre RPM scriplet since this scriptlet is called before the
installation thus
the old kdumpctl is called by the %pre scriptlet. But there is indeed
no need to check if get-default-crashkernel exists since the case of
an empty
/tmp/default_crashkernel file could be correctly taken care of. I'll
drop this check in v4.
Oh, I couldn't drop the check because dnf would check the return code of
scriptlet. So in v4. I'll simply use "grep -q" instead.
>
>Thanks
>Philipp
>
>>+ kdumpctl get-default-crashkernel kdump > /tmp/old_default_crashkernel
2>/dev/null
>>+%ifarch ppc64 ppc64le
>>+ kdumpctl get-default-crashkernel fadump >
/tmp/old_default_crashkernel_fadump 2>/dev/null
>>+%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 ! grep -q "ostree" /proc/cmdline && [ $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
--
Best regards,
Coiby