On Thu, 21 Jul 2022 15:26:24 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
Hi Philipp,
On Wed, Jul 20, 2022 at 06:18:58PM +0200, Philipp Rudo wrote:
>Hi Coiby,
>
>On Tue, 12 Jul 2022 16:20:12 +0800
>Coiby Xu <coxu(a)redhat.com> wrote:
>
>> "grubby --zipl" only takes effect when setting default kernel.
It's
>> useless to add "--zipl" when updating kernel command line. Also
rename
>> _update_grub to _update_kernel_cmdline since s390x doesn't use GRUB.
>
>Not entirely sure but I think you are wrong here. In /etc/zipl.conf you
>can also define the kernel command line. I assume that grubby will
>update this entry only when you pass the --zipl option. That shouldn't
>make a difference in the default configuration nevertheless I prefer to
>keep the code as is.
--zipl indeed does nothing. grubby simply updates the default boot entry
in /et/zipl.conf with or without --zipl because grubby chooses to update
/etc/zipl.conf if bootloader=zipl [1]. And even without --zipl,
bootloader=zipl after grubby finds the arch is s390.
True, --zipl should only make a difference in a extremely weird
cross-arch setup. But in that case most likely lots of other things
would break as well ;-)
Please ignore my comment
Reviewed-by: Philipp Rudo <prudo(a)redhat.com>
[1]
https://src.fedoraproject.org/rpms/grubby/blob/rawhide/f/grubby-bls
>
>Thanks
>Philipp
>
>P.S. the new function name is much better. Maybe you can sneek it in
>with some other patch ;)
Good to know that:)
>
>> Signed-off-by: Coiby Xu <coxu(a)redhat.com>
>> ---
>> kdumpctl | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/kdumpctl b/kdumpctl
>> index 10e3429..46df7f4 100755
>> --- a/kdumpctl
>> +++ b/kdumpctl
>> @@ -1338,7 +1338,7 @@ _get_current_running_kernel_path()
>> fi
>> }
>>
>> -_update_grub()
>> +_update_kernel_cmdline()
>> {
>> local _kernel_path=$1 _crashkernel=$2 _dump_mode=$3 _fadump_val=$4
>>
>> @@ -1349,15 +1349,14 @@ _update_grub()
>> rpm-ostree kargs --append="crashkernel=$_crashkernel"
>> fi
>> else
>> - [[ -f /etc/zipl.conf ]] && zipl_arg="--zipl"
>> - grubby --args "crashkernel=$_crashkernel" --update-kernel
"$_kernel_path" $zipl_arg
>> + grubby --args "crashkernel=$_crashkernel" --update-kernel
"$_kernel_path"
>> if [[ $_dump_mode == kdump ]]; then
>> grubby --remove-args="fadump" --update-kernel
"$_kernel_path"
>> else
>> grubby --args="fadump=$_fadump_val" --update-kernel
"$_kernel_path"
>> fi
>> fi
>> - [[ $zipl_arg ]] && zipl > /dev/null
>> + [[ -f /etc/zipl.conf ]] && zipl > /dev/null
>> }
>>
>> _valid_grubby_kernel_path()
>> @@ -1487,7 +1486,7 @@ reset_crashkernel()
>> _new_dump_mode=kdump
>> _new_crashkernel=$(kdump_get_arch_recommend_crashkernel
"$_new_dump_mode")
>> if [[ $_old_crashkernel != "$_new_crashkernel" ]]; then
>> - _update_grub "" "$_new_crashkernel"
"$_new_dump_mode" ""
>> + _update_kernel_cmdline "" "$_new_crashkernel"
"$_new_dump_mode" ""
>> if [[ $_reboot == yes ]]; then
>> systemctl reboot
>> fi
>> @@ -1555,7 +1554,7 @@ reset_crashkernel()
>> _old_crashkernel=$(get_grub_kernel_boot_parameter "$_kernel"
crashkernel)
>> _old_fadump_val=$(get_grub_kernel_boot_parameter "$_kernel"
fadump)
>> if [[ $_old_crashkernel != "$_new_crashkernel" || $_old_fadump_val
!= "$_new_fadump_val" ]]; then
>> - _update_grub "$_kernel" "$_new_crashkernel"
"$_new_dump_mode" "$_new_fadump_val"
>> + _update_kernel_cmdline "$_kernel" "$_new_crashkernel"
"$_new_dump_mode" "$_new_fadump_val"
>> if [[ $_reboot != yes ]]; then
>> if [[ $_old_crashkernel != "$_new_crashkernel" ]]; then
>> _what_is_updated="Updated crashkernel=$_new_crashkernel"
>> @@ -1624,7 +1623,7 @@ reset_crashkernel_after_update()
>> 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
>> + if _update_kernel_cmdline "$_kernel"
"$_new_default_crashkernel" "$_dump_mode" "$_fadump_val";
then
>> echo "For kernel=$_kernel, crashkernel=$_new_default_crashkernel
now."
>> fi
>> fi
>> @@ -1680,7 +1679,7 @@ reset_crashkernel_for_installed_kernel()
>> _fadump_val_running=$(get_grub_kernel_boot_parameter "$_kernel"
fadump)
>>
>> if [[ $_crashkernel != "$_crashkernel_running" ]]; then
>> - if _update_grub "$_installed_kernel"
"$_crashkernel_running" "$_dump_mode_running"
"$_fadump_val_running"; then
>> + if _update_kernel_cmdline "$_installed_kernel"
"$_crashkernel_running" "$_dump_mode_running"
"$_fadump_val_running"; then
>> echo "kexec-tools has reset $_installed_kernel to use the new default
crashkernel value $_crashkernel_running"
>> fi
>> fi
>