On 28/02/19 8:34 AM, Kairui Song wrote:
Hi Hari, thanks for the reply,
On Wed, Feb 27, 2019 at 1:56 PM Hari Bathini <hbathini(a)linux.ibm.com> wrote:
> On 26/02/19 12:53 PM, piliu wrote:
>> On 02/22/2019 11:26 AM, Kairui Song wrote:
>>> Hi Hari, Thanks for the update.
>>>
>>> On Fri, Feb 1, 2019 at 6:26 PM Hari Bathini <hbathini(a)linux.ibm.com>
wrote:
>>>> With kernel commit 0823c68b054b ("powerpc/fadump: re-register
firmware-
>>>> assisted dump if already registered") support is enabled to
re-register
>>>> when FADump is alredy registered. Leverage that option in kdump scripts.
>>>>
>>>> Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
>>>> ---
>>>> kdumpctl | 19 ++++++++++++++++++-
>>>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/kdumpctl b/kdumpctl
>>>> index de6da39..7b8013c 100755
>>>> --- a/kdumpctl
>>>> +++ b/kdumpctl
>>>> @@ -1047,7 +1047,8 @@ reload()
>>>> fi
>>>>
>>>> if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
>>>> - stop_fadump
>>>> + reload_fadump
>>>> + return $?
>>>> else
>>>> stop_kdump
>>>> fi
>>>> @@ -1103,6 +1104,22 @@ stop_kdump()
>>>> return 0
>>>> }
>>>>
>>>> +reload_fadump()
>>>> +{
>>>> + echo 1 > $FADUMP_REGISTER_SYS_NODE
>>>> + if [ $? == 0 ]; then
>>>> + echo "fadump: re-registered successfully"
>>>> + return 0
>>>> + else
>>>> + # FADump could fail on older kernel where re-register
>>>> + # support is not enabled. Try stop/start from userspace
>>>> + # to handle such scenario.
>>>> + stop_fadump
>>>> + start_fadump
>>> stop_fadump and start_fadump may also fail ), if stop_fadump failed
>> Yes, it could happen based on the kernel code.
>>> start_fadump should not be called or else it will wrongly report
>>> "registered successfully". Would it be better to let reload_fadump
>
> I think it is better to fall back to old method..
>
>>> return right after try to re-register and don't try to fallback to
>>> stop/start? Then callee would try to call reload_fadump() first, if
>>> successed it return, else fail back to ordinary routine.
>>>
>> What about return immediately after stop_fadump() if error happens, and
>> warning the user about it? I think if this error happens, then it is
>> unlikely to unregister successfully any longer.
>
> Yeah, I think it makes sense to return if stop_fadump failed..
>
> Will re-post v3 if you guys agree..
>
Yes please, that should be good.
Sure. Posted V3.
Thanks
Hari