6:08 a.m.
Hi Hari,
On Mon, 21 Nov 2022 17:48:01 +0530
Hari Bathini <hbathini(a)linux.ibm.com> wrote:
In case of fadump, default initrd is rebuilt with dump capturing
capability, as the same initrd is used for booting production kernel
as well as capture kernel.
The original initrd file is backed up with a checksum, to restore
it as the default initrd when fadump is disabled. As the checksum
file is not kernel version specific, switching between different
kernel versions and kdump/fadump dump mode breaks the default initrd
backup/restore logic. Fix this by having a kernel version specific
checksum file.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
Thanks for taking care!
Two small comments:
1) in backup_default_initrd you should remove the checksum file in the
error case, when the initrd cannot be backed up (this should
probably be done today already...)
2) why don't you simply update INITRD_CHECKSUM_OCATION one instead of
editing every line where it is used? For example consider the patch
below. I see multiple benefits (1) it's shorter, (2) the name shows
clearer to which initrd the checksum belongs to, (3) it reuses
KDUMP_BOOTDIR instead of hard coding /boot and (4)
INITRD_CHECKSUM_LOCATION now is basically local to setup_initrd now
making it easier for a clean up to remove the global variable later
on.
What do you think?
Thanks
Philipp
diff --git a/kdumpctl b/kdumpctl
index 7caaae5..94af3e2 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -9,7 +9,7 @@ KDUMP_LOG_PATH="/var/log"
MKDUMPRD="/sbin/mkdumprd -f"
MKFADUMPRD="/sbin/mkfadumprd"
DRACUT_MODULES_FILE="/usr/lib/dracut/modules.txt"
-INITRD_CHECKSUM_LOCATION="/boot/.fadump_initrd_checksum"
+INITRD_CHECKSUM_LOCATION=""
DEFAULT_INITRD=""
DEFAULT_INITRD_BAK=""
KDUMP_INITRD=""
@@ -166,7 +166,8 @@ backup_default_initrd()
sha1sum "$DEFAULT_INITRD" >
"$INITRD_CHECKSUM_LOCATION"
if ! cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK"; then
dwarn "WARNING: failed to backup $DEFAULT_INITRD."
- rm -f "$DEFAULT_INITRD_BAK"
+ rm -f -- "$INITRD_CHECKSUM_LOCATION"
+ rm -f -- "$DEFAULT_INITRD_BAK"
fi
fi
}
@@ -317,6 +318,7 @@ setup_initrd()
fi
DEFAULT_INITRD_BAK="$KDUMP_BOOTDIR/.$(basename
"$DEFAULT_INITRD").default"
+ INITRD_CHECKSUM_LOCATION="$DEFAULT_INITRD_BAK.checksum"
if [[ $DEFAULT_DUMP_MODE == "fadump" ]]; then
TARGET_INITRD="$DEFAULT_INITRD"
---
* New patch to fix default initrd backup/restore logic.
kdumpctl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kdumpctl b/kdumpctl
index 7caaae5..03c91f3 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -163,7 +163,7 @@ backup_default_initrd()
if [[ ! -e $DEFAULT_INITRD_BAK ]]; then
dinfo "Backing up $DEFAULT_INITRD before rebuild."
# save checksum to verify before restoring
- sha1sum "$DEFAULT_INITRD" > "$INITRD_CHECKSUM_LOCATION"
+ sha1sum "$DEFAULT_INITRD" >
"$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER"
if ! cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK"; then
dwarn "WARNING: failed to backup $DEFAULT_INITRD."
rm -f "$DEFAULT_INITRD_BAK"
@@ -181,14 +181,14 @@ restore_default_initrd()
# If a backup initrd exists, we must be switching back from
# fadump to kdump. Restore the original default initrd.
- if [[ -f $DEFAULT_INITRD_BAK ]] && [[ -f $INITRD_CHECKSUM_LOCATION ]]; then
+ if [[ -f $DEFAULT_INITRD_BAK ]] && [[ -f
$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER ]]; then
# verify checksum before restoring
backup_checksum=$(sha1sum "$DEFAULT_INITRD_BAK" | awk '{ print $1
}')
- default_checksum=$(awk '{ print $1 }' "$INITRD_CHECKSUM_LOCATION")
+ default_checksum=$(awk '{ print $1 }'
"$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER")
if [[ $default_checksum != "$backup_checksum" ]]; then
dwarn "WARNING: checksum mismatch! Can't restore original initrd.."
else
- rm -f $INITRD_CHECKSUM_LOCATION
+ rm -f $INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER
if mv "$DEFAULT_INITRD_BAK" "$DEFAULT_INITRD"; then
derror "Restoring original initrd as fadump mode is disabled."
sync -f "$DEFAULT_INITRD"