On Thu, Jun 12, 2014 at 08:26:15PM +0530, Hari Bathini wrote:
The current kdump infrastructure builds a separate initrd which then
gets loaded into memory by kexec-tools for use by kdump kernel. But
firmware assisted dump (FADUMP) does not use kexec-based approach.
After crash, firmware reboots the partition and loads grub loader
like the normal booting process does. Hence in the FADUMP approach,
the second kernel (after crash) will always use the default initrd
(OS built). So, to support FADUMP, change is required, as in to add
dump capturing steps, in this initrd.
The current kdumpctl script implementation already has the code to
build initrd using mkdumprd. This patch uses the new '--rebuild'
option introduced, in dracut, to incrementally build the initramfs
image. Before rebuilding, we may need to probe the initrd image for
fadump support, to avoid rebuilding the initrd image multiple times
unnecessarily. This can be done using "lsinitrd" tool with the newly
proposed '--mod' option & inspecting the presence of "kdumpbase"
in
the list of modules of default initrd image. We rebuild the image if
only "kdumpbase" module is missing in the initrd image. Also, before
rebuilding, a backup of default initrd image is taken.
Kexec-tools package in rhel7 is now enhanced to insert a out-of-tree
kdump module for dracut, which is responsible for adding vmcore
capture steps into initrd, if dracut is invoked with "IN_KDUMP"
environment variable set to 1. mkdumprd script exports "IN_KDUMP=1"
environment variable before invoking dracut to build kdump initrd.
This patch relies on this current mechanism of kdump init script.
Patch that adds "--mod" option to lsinitrd tool is posted upstream,
awaiting approval. Link for reference:
http://www.spinics.net/lists/linux-initramfs/msg03670.html
Signed-off-by: Mahesh Salgaonkar <mahesh(a)linux.vnet.ibm.com>
Signed-off-by: Hari Bathini <hbathini(a)linux.vnet.ibm.com>
---
kdumpctl | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 64 insertions(+), 8 deletions(-)
diff --git a/kdumpctl b/kdumpctl
index 4bc3952..e86bee0 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -9,6 +9,7 @@ MKDUMPRD="/sbin/mkdumprd -f"
SAVE_PATH=/var/crash
SSH_KEY_LOCATION="/root/.ssh/kdump_id_rsa"
DUMP_TARGET=""
+TARGET_INITRD=""
FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered"
#kdump shall be the default dump mode
@@ -133,13 +134,45 @@ function save_core()
fi
}
Hi Hari,
This patch is almost there. I have few more minor comments.
-function rebuild_initrd()
+rebuild_fadump_initrd()
{
- $MKDUMPRD $kdump_initrd $kdump_kver
+ # backup fadump initrd for reference before replacing it
+ backup_initrd
+
+ target_initrd_tmp="$TARGET_INITRD.tmp"
All the local variables please define the at beginning of function, like.
local target_initrd_tmp
+ $MKDUMPRD $target_initrd_tmp --rebuild $TARGET_INITRD --kver
$kdump_kver
+ if [ $? != 0 ]; then
+ echo "mkdumprd: failed to rebuild initrd with fadump support" >&2
+ return 1
+ fi
+
+ # updating fadump initrd
+ mv $target_initrd_tmp $TARGET_INITRD
+ sync
This system wide sync might harm us at some point of time. For the time
being it is fine. But ideally send a patch to util-linux maintainer and
write a small "fsync" utility and call "fsync $TARGET_INITRD". I
heard
that some unix variant had "fsync" as stand alone program.
+
+ return 0
+}
+
+rebuild_kdump_initrd()
+{
+ $MKDUMPRD $TARGET_INITRD $kdump_kver
if [ $? != 0 ]; then
echo "mkdumprd: failed to make kdump initrd" >&2
return 1
fi
+
+ return 0
+}
+
+rebuild_initrd()
+{
+ if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
+ rebuild_fadump_initrd
+ else
+ rebuild_kdump_initrd
+ fi
+
+ return $?
}
#$1: the files to be checked with IFS=' '
@@ -164,6 +197,17 @@ function check_executable()
done
}
+backup_initrd()
+{
+ target_initrd_bak="$TARGET_INITRD.default.bak"
Why not just "$TARGET_INITRD.bak". I am wondering what's the purpose of
"default" string.
+
+ # Check if backup initrd is already present.
+ if [ ! -e $target_initrd_bak ];then
+ echo "Backing up $TARGET_INITRD"
+ cp $TARGET_INITRD $target_initrd_bak
+ fi
So this backup happens only if backup is not already present?
+}
+
function check_config()
{
local nr
@@ -242,7 +286,17 @@ function check_rebuild()
fi
kdump_kernel="${KDUMP_BOOTDIR}/${KDUMP_IMG}-${kdump_kver}${KDUMP_IMG_EXT}"
- kdump_initrd="${KDUMP_BOOTDIR}/initramfs-${kdump_kver}kdump.img"
+ if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
+ TARGET_INITRD="${KDUMP_BOOTDIR}/initramfs-${kdump_kver}.img"
+ if [ ! -s "$TARGET_INITRD" ]; then
+ echo "Error: No initrd found to rebuild!"
+ return 1
+ fi
+ fadump_support=`lsinitrd -m $TARGET_INITRD | grep ^kdumpbase$ | wc -l`
fadump_support name is little confusing. How about "initramfs_has_fadump".
Also define this variable as "local" in the beginning of function.
+ else
+ TARGET_INITRD="${KDUMP_BOOTDIR}/initramfs-${kdump_kver}kdump.img"
+ fadump_support=-1
I don't think this is required. See below.
+ fi
Can we move above logic in a separate function, say setup_target_initrd().
And in that function you can also do error handling if default intird is
not found. In parent function check for return code and bail out.
_force_rebuild=`grep ^force_rebuild $KDUMP_CONFIG_FILE 2>/dev/null`
if [ $? -eq 0 ]; then
@@ -259,8 +313,8 @@ function check_rebuild()
#check to see if dependent files has been modified
#since last build of the image file
- if [ -f $kdump_initrd ]; then
- image_time=`stat -c "%Y" $kdump_initrd 2>/dev/null`
+ if [ -f $TARGET_INITRD ]; then
+ image_time=`stat -c "%Y" $TARGET_INITRD 2>/dev/null`
else
image_time=0
fi
@@ -287,8 +341,10 @@ function check_rebuild()
if [ $image_time -eq 0 ]; then
echo -n "No kdump initial ramdisk found."; echo
+ elif [ "$fadump_support" -eq "0" ]; then
This check could be combination of two checks. Something like.
elif [ DUMP_MODE == fadump && !initramfs_has_fadump ]
This is much more readable.
+ echo "$TARGET_INITRD has no fadump support"
elif [ "$force_rebuild" != "0" ]; then
- echo -n "Force rebuild $kdump_initrd"; echo
+ echo -n "Force rebuild $TARGET_INITRD"; echo
elif [ -n "$modified_files" ]; then
echo "Detected change(s) the following file(s):"
echo -n " "; echo "$modified_files" | sed 's/\s/\n /g'
@@ -296,7 +352,7 @@ function check_rebuild()
return 0
fi
- echo "Rebuilding $kdump_initrd"
+ echo "Rebuilding $TARGET_INITRD"
rebuild_initrd
return $?
}
@@ -349,7 +405,7 @@ function load_kdump()
$KEXEC $KEXEC_ARGS $standard_kexec_args \
--command-line="$KDUMP_COMMANDLINE" \
- --initrd=$kdump_initrd $kdump_kernel 2>/dev/null
+ --initrd=$TARGET_INITRD $kdump_kernel 2>/dev/null
if [ $? == 0 ]; then
echo "kexec: loaded kdump kernel"
return 0
Thanks
Vivek