Hi Hari,
On Mon, 21 Nov 2022 17:48:01 +0530 Hari Bathini hbathini@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@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"@@ -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")
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"