On 27/03/19 12:12 PM, Kairui Song wrote:
On Wed, Mar 27, 2019 at 11:28 AM Dave Young <dyoung(a)redhat.com>
wrote:
> Hi Hari,
> On 03/26/19 at 11:23pm, Hari Bathini wrote:
>> Hi Dave,
>>
>>
>> On 25/03/19 11:17 AM, Dave Young wrote:
>>> Ccing Hari for the dump mode issue.
>>>
>>> On 03/13/19 at 08:28pm, Kairui Song wrote:
>>>> Use "kdumpctl rebuild" to rebuild the image directly. This
could help
>>>> admins to rebuild kdump image directly.
>>>>
>>>> Signed-off-by: Kairui Song <kasong(a)redhat.com>
>>>> ---
>>>> kdumpctl | 26 +++++++++++++++++++++++++-
>>>> 1 file changed, 25 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/kdumpctl b/kdumpctl
>>>> index 1cfbe31..e223224 100755
>>>> --- a/kdumpctl
>>>> +++ b/kdumpctl
>>>> @@ -1141,6 +1141,27 @@ stop()
>>>> return 0
>>>> }
>>>> +rebuild() {
>>>> + setup_initrd
>>>> + if [ $? -ne 0 ]; then
>>>> + return 1
>>>> + fi
>>>> +
>>>> + if [[ ! -w "$KDUMP_BOOTDIR" ]];then
>>>> + echo "$KDUMP_BOOTDIR does not have write permission. Can
not rebuild $TARGET_INITRD"
>>>> + return 1
>>>> + fi
>>>> +
>>>> + handle_mode_switch
>>>> + if [ $? -ne 0 ]; then
>>>> + return 1
>>>> + fi
>>> Maybe handle_mode_switch can be moved to rebuild_initrd()
>>> then we do not need it in this function.
>> IIUC, remove handle_mode_switch() function call here and check_rebuild()
functions and
>> call it from rebuild_initrd() instead..? That change is likely to ignore scenario
where
>> a rebuild is not necessary (no change in /etc/kdump.conf file) but a restore is
needed
>> when we switch from FADump to KDump? We may eventually restore when initrd is
rebuilt
>> though...
>>
> Hmm, then maybe move it to setup_initrd? In setup_initrd, we have the
> if else for dump mode then we can just move to backup/restore there and
> drop this handle_mode_switch function.
>
> And the -w -w "$KDUMP_BOOTDIR" can be moved to rebuild_initrd(), kairui?
Hi, Dave, just one thing, move handle_mode_switch to setup_initrd this
will make "kdumpctl reload" possible trigger a backup / restore but
don't rebuild anything, doesn't seems very reasonable.
Currently kdumpctl reload is not behaving very well on mode switch
Can you share the sequence in which this is happening..
anyway (may fail silently on normal->fadump switch or error out on
I think backup & restore functions don't return any error explicitly..
fadump->normal), but after all user should really trigger a
rebuild
Restore may error out only when `sync` fails and if `sync` fails,
admin has filesystem related issues to sort out first..
With commit da6b75f59bdd ("fadump: use the original initrd to rebuild
fadump initrdfrom"), error check in setup_initrd() is no longer needed.
So, I think the below snippet is not a bad idea based on Dave's suggestion:
--
diff --git a/kdumpctl b/kdumpctl
index 1cfbe31..1dd7fa3 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -298,12 +298,17 @@ setup_initrd()
DEFAULT_INITRD_BAK="${KDUMP_BOOTDIR}/.initramfs-`uname
-r`.img.default"
if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
TARGET_INITRD="$DEFAULT_INITRD"
- if [ ! -s "$TARGET_INITRD" ]; then
- echo "Error: No initrd found to rebuild!"
- return 1
- fi
+
+ # backup initrd for reference before replacing it
+ # with fadump aware initrd
+ backup_default_initrd
else
TARGET_INITRD="${KDUMP_BOOTDIR}/initramfs-${kdump_kver}kdump.img"
+
+ # check if a backup of default initrd exists. If yes,
+ # it signifies a switch from fadump mode. So, restore
+ # the backed up default initrd.
+ restore_default_initrd
fi
}
@@ -602,8 +607,6 @@ check_rebuild()
system_modified="1"
fi
- handle_mode_switch
-
if [ $image_time -eq 0 ]; then
echo -n "No kdump initial ramdisk found."; echo
elif [ "$capture_capable_initrd" == "0" ]; then
@@ -745,20 +748,6 @@ show_reserved_mem()
echo "Reserved "$mem_mb"MB memory for crash kernel"
}
-handle_mode_switch()
-{
- if [ "$DEFAULT_DUMP_MODE" == "fadump" ]; then
- # backup initrd for reference before replacing it
- # with fadump aware initrd
- backup_default_initrd
- else
- # check if a backup of default initrd exists. If yes,
- # it signifies a switch from fadump mode. So, restore
- # the backed up default initrd.
- restore_default_initrd
- fi
-}
-
check_current_fadump_status()
{
# Check if firmware-assisted dump has been registered.
--
Thanks
Hari