Kairui Song (2): kdumpctl: add rebuild support Update man page for new kdumpctl command: reload / rebuild
kdumpctl | 26 +++++++++++++++++++++++++- kdumpctl.8 | 6 ++++++ 2 files changed, 31 insertions(+), 1 deletion(-)
Use "kdumpctl rebuild" to rebuild the image directly. This could help admins to rebuild kdump image directly.
Signed-off-by: Kairui Song kasong@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 + + echo "Rebuilding $TARGET_INITRD" + rebuild_initrd + return $? +} + if [ ! -f "$KDUMP_CONFIG_FILE" ]; then echo "Error: No kdump config file found!" >&2 exit 1 @@ -1185,6 +1206,9 @@ main () stop start ;; + rebuild) + rebuild + ;; condrestart) ;; propagate) @@ -1194,7 +1218,7 @@ main () show_reserved_mem ;; *) - echo $"Usage: $0 {start|stop|status|restart|reload|propagate|showmem}" + echo $"Usage: $0 {start|stop|status|restart|reload|rebuild|propagate|showmem}" exit 1 esac }
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@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.
- echo "Rebuilding $TARGET_INITRD"
- rebuild_initrd
- return $?
+}
if [ ! -f "$KDUMP_CONFIG_FILE" ]; then echo "Error: No kdump config file found!" >&2 exit 1 @@ -1185,6 +1206,9 @@ main () stop start ;;
rebuild)
rebuild
condrestart) ;; propagate);;
@@ -1194,7 +1218,7 @@ main () show_reserved_mem ;; *)
echo $"Usage: $0 {start|stop|status|restart|reload|propagate|showmem}"
exit 1 esacecho $"Usage: $0 {start|stop|status|restart|reload|rebuild|propagate|showmem}"
}
2.20.1
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@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...
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@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?
Thanks Dave
On Wed, Mar 27, 2019 at 11:28 AM Dave Young dyoung@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@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 anyway (may fail silently on normal->fadump switch or error out on fadump->normal), but after all user should really trigger a rebuild after mode change.
The -w "$KDUMP_BOOTDIR" could be moved to rebuild_initrd indeed, that should be good.
-- Best Regards, Kairui Song
On 27/03/19 12:12 PM, Kairui Song wrote:
On Wed, Mar 27, 2019 at 11:28 AM Dave Young dyoung@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@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
On Wed, Mar 27, 2019 at 4:34 PM Hari Bathini hbathini@linux.ibm.com wrote:
On 27/03/19 12:12 PM, Kairui Song wrote:
On Wed, Mar 27, 2019 at 11:28 AM Dave Young dyoung@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@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..
Yes I had some misunderstanding, please ignore my concern about reload...
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
This looks good, I'll update V2, thanks!
-- Best Regards, Kairui Song
These two command are not documented, update the man page to let user know how to use them.
Signed-off-by: Kairui Song kasong@redhat.com --- kdumpctl.8 | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/kdumpctl.8 b/kdumpctl.8 index 023562b..ae97af7 100644 --- a/kdumpctl.8 +++ b/kdumpctl.8 @@ -32,6 +32,12 @@ It returns non-zero value if kdump is not operational. Is equal to .I start; stop .TP +.I reload +reload crash kernel image and initramfs without triggering a rebuild. +.TP +.I rebuild +rebuild the crash kernel initramfs. +.TP .I propagate Helps to setup key authentication for ssh storage since it's impossible to use password authentication during kdump.