Hi Lianbo, Bhupesh
Thank you for the comments.
On 1/10/2019 2:40 PM, Bhupesh Sharma wrote:
On Thu, Jan 10, 2019 at 6:28 PM lijiang <lijiang(a)redhat.com>
wrote:
>
> 在 2019年01月10日 14:59, Bhupesh Sharma 写道:
>> Hi Kazu, Lianbo,
>>
>> On Thu, Jan 10, 2019 at 10:00 AM lijiang <lijiang(a)redhat.com> wrote:
>>>
>>> 在 2019年01月10日 04:38, Kazuhito Hagio 写道:
>>>> If a crash occurs repeatedly after enabling kdump, the system goes
>>>> into a crash loop and the dump target may get filled up by vmcores.
>>>> This is likely to happen especially with early kdump.
>>>>
>>>> This patch introduces 'final_action' option to kdump.conf, in
order
>>>> for users to be able to power off the system even after capturing
>>>> a vmcore successfully.
>>>>
>>>> Signed-off-by: Kazuhito Hagio <k-hagio(a)ab.jp.nec.com>
>>>> Cc: Dave Young <dyoung(a)redhat.com>
>>>> Cc: Lianbo Jiang <lijiang(a)redhat.com>
>>>> ---
>>>> kdump-lib-initramfs.sh | 13 +++++++++++++
>>>> kdump.conf | 5 +++++
>>>> kdump.conf.5 | 6 ++++++
>>>> kdumpctl | 22 +++++++++++++++++++++-
>>>> 4 files changed, 45 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
>>>> index f5155a4..9f482cd 100755
>>>> --- a/kdump-lib-initramfs.sh
>>>> +++ b/kdump-lib-initramfs.sh
>>>> @@ -70,6 +70,19 @@ get_kdump_confs()
>>>> ;;
>>>> esac
>>>> ;;
>>>> + final_action)
>>>> + case $config_val in
>>>> + reboot)
>>>> + FINAL_ACTION="systemctl reboot -f"
>>>> + ;;
>>>> + halt)
>>>> + FINAL_ACTION="halt"
>>>> + ;;
>>>> + poweroff)
>>>> + FINAL_ACTION="systemctl poweroff -f"
>>>> + ;;
>>>> + esac
>>>> + ;;
>>>> esac
>>>> done < $KDUMP_CONF
>>>>
>>> Thanks for your patch.
>>>
>>> Once dumping fails, it will switch to the kdump-error-handler.sh, but this
script always calls
>>> the do_default_action() first, and then executes the final action. Because
the default is always
>>> 'reboot' in case the 'default' is not set in
'/etc/kdump.conf', so the final action could have no
>>> chance to execute. Unless we set 'default poweroff' and
'final_action poweroff' together.
>>
>> Indeed.
Yes, this is why I wrote the following in the 2/2 patch.
+ final_action poweroff
+ default poweroff
>>
>>> Not sure whether it would be more better like this:
>>>
>>> diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
>>> index 7ba99b6..953b8bc 100755
>>> --- a/kdump-lib-initramfs.sh
>>> +++ b/kdump-lib-initramfs.sh
>>> @@ -6,7 +6,7 @@ KDUMP_PATH="/var/crash"
>>> CORE_COLLECTOR=""
>>> DEFAULT_CORE_COLLECTOR="makedumpfile -l --message-level 1 -d 31"
>>> DMESG_COLLECTOR="/sbin/vmcore-dmesg"
>>> -DEFAULT_ACTION="systemctl reboot -f"
>>> +DEFAULT_ACTION=""
>>>
>>> do_default_action()
>>> {
>>> - echo "Kdump: Executing default action $DEFAULT_ACTION"
>>> - eval $DEFAULT_ACTION
>>> + if [ "$DEFAULT_ACTION" == "" ]; then
>>> + echo "Kdump: default action is not set."
>>> + else
>>> + echo "Kdump: Executing default action $DEFAULT_ACTION"
>>> + eval $DEFAULT_ACTION
>>> + fi
>>> }
>>>
>>> It makes sure that the final action can be called in case user doesn't
set the 'default poweroff'
>>> and dumping failure.
>>
>> Perhaps its only me but I find using two terms - 'default action' and
>> 'final action' confusing when used together.
>> Shouldn't the default action be practically the final action in normal
cases?
>>
>> I understand that perhaps this is useful for early dump, but in that
>> case shouldn't we be making the default action as power-off instead.
>>
>
> Normal kdump could face the same problem(crash loop), the 'poweroff' option
would still make sense for normal
> kdump.
Yes, but we shouldn't change the default value of 'default' option
because many users would be confused by the change.
Use a convention like 'failsafe_action' then instead of 'final_action'
which is confusing when used in conjunction with 'default_action'.
I also think the current terms are confusing and something like
'success_action' and 'failure_action' might be more intuitive.
So an idea I thought of is adding 'failure_action' as an alias of
'default' and removing 'default' in the future.
failure_action <reboot | halt | poweroff | shell | dump_to_rootfs>
default <reboot | halt | poweroff | shell | dump_to_rootfs>
- Action to perform in case dumping fails.
No difference between "failure_action" and "default"
but "default" directive will be removed in the future.
success_action <reboot | halt | poweroff>
- Action to perform in case dumping succeeds.
Any other ideas?
Thanks,
Kazu