Hi Tao,
as said earlier I don't think this is the right way to go. Here are
some more details why this approach is bad in my opinion.
On Thu, 28 Sep 2023 21:20:26 +0800
Tao Liu <ltao(a)redhat.com> wrote:
There are some functions which are common for mkdumprd and kdumpctl
test/test-check. For example, mkdumprd will check the target fs size
and warn if size is less than total memory. This involves remote fs mount,
same as test-check which will also mount remote fs to check if the test
timestamp id exist.
Also, this patch makes modifications on functions such as
"handle_default_dump_target" "check_user_configured_target", which
receives a
function as argument, so they can be used for both mkdumprd and kdumpctl
test/test-check.
Signed-off-by: Tao Liu <ltao(a)redhat.com>
---
kdump-lib.sh | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++
mkdumprd | 127 ++++-------------------------------------------
2 files changed, 149 insertions(+), 116 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh
index 042ac87..0eb5f03 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -1089,3 +1089,141 @@ get_all_kdump_crypt_dev()
get_luks_crypt_dev "$(kdump_get_maj_min "$_dev")"
done
}
+
+SAVE_PATH=$(get_save_path)
This is the same like ${OPT[path]}.
+handle_tmp_dir()
+{
+ TMPDIR="$(mktemp -d -t mkdumprd.XXXXXX)"
+ [ -d "$TMPDIR" ] || perror_exit "dracut: mktemp -p -d -t dracut.XXXXXX
failed."
+ TMPMNT="$TMPDIR/target"
+
+ trap '
+ ret=$?;
+ is_mounted $TMPMNT && umount -f $TMPMNT;
+ [[ -d $TMPDIR ]] && rm --one-file-system -rf -- "$TMPDIR";
+ exit $ret;
+ ' EXIT
+}
This will overwrite the trap handler set up at the beginning of
kdumpctl. Ultimately this will cause $KDUMP_TMPDIR to not be deleted.
+check_save_path_fs()
+{
+ local _path=$1
+
+ if [[ ! -d $_path ]]; then
+ perror_exit "Dump path $_path does not exist."
+ fi
+}
+
+#handle the case user does not specify the dump target explicitly
+# $1: function
+# ${2..}: arguments for the function
+handle_default_dump_target()
+{
+ local _target
+ local _mntpoint
+ local _func=$1
+
+ shift 1
+ is_user_configured_dump_target && return
+
+ check_save_path_fs "$SAVE_PATH"
+
+ _save_path=$(get_bind_mount_source "$SAVE_PATH")
+ _target=$(get_target_from_path "$_save_path")
+ _mntpoint=$(get_mntpoint_from_target "$_target")
+
+ SAVE_PATH=${_save_path##"$_mntpoint"}
+ $_func "$_target" "$_save_path" $@
+}
+
+mount_failure()
+{
+ local _target=$1
+ local _mnt=$2
+ local _fstype=$3
+ local msg="Failed to mount $_target"
+
+ if [[ -n $_mnt ]]; then
+ msg="$msg on $_mnt"
+ fi
+
+ msg="$msg for kdump preflight check."
+
+ if [[ $_fstype == "nfs" ]]; then
+ msg="$msg Please make sure nfs-utils has been installed."
+ fi
+
+ perror_exit "$msg"
+}
+
+# $1: fs target
+# $2: fs type
+# $3: function
+# ${4..}: arguments for the function
+check_user_configured_target()
+{
+ local _target=$1 _cfg_fs_type=$2 _mounted _func=$3
+ local _mnt _opt _fstype _ret
+
+ _mnt=$(get_mntpoint_from_target "$_target")
+ _opt=$(get_mntopt_from_target "$_target")
+ _fstype=$(get_fs_type_from_target "$_target")
+
+ if [[ -n $_fstype ]]; then
+ # In case of nfs4, nfs should be used instead, nfs* options is deprecated in
kdump.conf
+ [[ $_fstype == "nfs"* ]] && _fstype=nfs
+
+ if [[ -n $_cfg_fs_type ]] && [[ $_fstype != "$_cfg_fs_type" ]];
then
+ perror_exit "\"$_target\" have a wrong type config
\"$_cfg_fs_type\", expected \"$_fstype\""
+ fi
+ else
+ _fstype="$_cfg_fs_type"
+ _fstype="$_cfg_fs_type"
Double line.
+ fi
+
+ # For noauto mount, mount it inplace with default value.
+ # Else use the temporary target directory
+ if [[ -n $_mnt ]]; then
+ if ! is_mounted "$_mnt"; then
+ if [[ $_opt == *",noauto"* ]]; then
+ mount "$_mnt" || mount_failure "$_target" "$_mnt"
"$_fstype"
+ _mounted=$_mnt
+ else
+ perror_exit "Dump target \"$_target\" is neither mounted nor
configured as \"noauto\""
+ fi
+ fi
+ else
+ handle_tmp_dir
+ _mnt=$TMPMNT
+ mkdir -p "$_mnt"
+ mount "$_target" "$_mnt" -t "$_fstype" -o defaults ||
mount_failure "$_target" "" "$_fstype"
+ _mounted=$_mnt
+ fi
+
+ # For user configured target, use $SAVE_PATH as the dump path within the target
+ if [[ ! -d "$_mnt/$SAVE_PATH" ]]; then
+ perror_exit "Dump path \"$SAVE_PATH\" does not exist in dump target
\"$_target\""
+ fi
+
+ shift 3
this needs to be at the beginning of the function where you process
its arguments. Otherwise someone will add a new argument and will
forget to update this line as well. Personally I prefer something like
this
local _target _cfg_fs_type _mounted _func
_target=$1; shift
_cfg_fs_type=$1; shift
_func=$1; shift
as that makes it easiest to expand later.
+ $_func "$_target" "$_mnt/$SAVE_PATH" $@
+ _ret=$?
+
+ # Unmount it early, if function is interrupted and didn't reach here, the shell
trap will clear it up anyway
+ if [[ -n $_mounted ]]; then
+ umount -f -- "$_mounted"
+ fi
+
+ return $_ret
+}
+
+get_ssh_key_location()
+{
+ SSH_KEY_LOCATION=$DEFAULT_SSHKEY
+ keyfile=$(kdump_get_conf_val sshkey)
+ if [[ -f $keyfile ]]; then
+ # canonicalize the path
+ SSH_KEY_LOCATION=$(/usr/bin/readlink -m "$keyfile")
+ fi
+}
This is already done in parse_config. You can simply use ${OPT[sshkey]}.
Thanks
Philipp