On Thu, Dec 30, 2021 at 09:45:57PM +0800, Pingfan Liu wrote:
>On Tue, Dec 28, 2021 at 08:49:27AM +0800, Coiby Xu 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 8b7e829f..cbafaa19 100755
>> --- a/kdumpctl
>> +++ b/kdumpctl
>> @@ -1559,6 +1559,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
>> @@ -1624,6 +1654,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 ab7f41fd..f439cb5a 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 -q get-default-crashkernel /usr/bin/kdumpctl; then
> ^^^
>Sorry I can not understand this condition "[ $1 == 2 ]"
"[ $1 == 2 ]" in %pre scriplet means upgrading a package as explained in
https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_sy...
>> + 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
>
>Does fadump=off fall in this category?
Sorry, I don't understand this question. But it's not related to
fadump=off.
I made a mistake. I had thought that kdump on ppc64 can not be handled
by this scriptlet. But it turns out wrong.
Thanks,
Pingfan
>> +%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 || :
> ^^^ what is the
> purpose?
>> +%ifarch ppc64 ppc64le
>> + rm /tmp/old_default_crashkernel_fadump 2>/dev/null || :
> ^^^ as above
/tmp/old_default_crashkernel may not exist and rm could return 1 in
case. In this case, dnf would complain about exit code not being 0.
>
>For the other part,
>
>LGTM.
>
>Thanks,
>
> Pingfan
>> +%endif
>> +fi
>>
>>
>> %postun
>> --
>> 2.31.1
>>
>
--
Best regards,
Coiby