On 05/05/16 at 05:20pm, Pratyush Anand wrote:
Relevant kdump files are also part of system. Therefore, moving logic
of
file modification checking in is_system_modified() function now.
Signed-off-by: Pratyush Anand <panand(a)redhat.com>
---
kdumpctl | 69 +++++++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 42 insertions(+), 27 deletions(-)
This looks a really good clean up. I like it.
diff --git a/kdumpctl b/kdumpctl
index 6bd2608343c0..793774d79a61 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -14,6 +14,7 @@ FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered"
#kdump shall be the default dump mode
DEFAULT_DUMP_MODE="kdump"
+image_time=0
. /lib/kdump/kdump-lib.sh
@@ -275,7 +276,6 @@ check_config()
# return list of modified file for fence_kdump modified in Pacemaker cluster
get_pcs_cluster_modified_files()
{
- local image_time=$1
local time_stamp
local modified_files
@@ -327,19 +327,59 @@ setup_target_initrd()
fi
}
+is_files_modified()
+{
+ local modified_files=""
+
+ #also rebuild when Pacemaker cluster conf is changed and fence kdump is enabled.
+ modified_files=$(get_pcs_cluster_modified_files)
+
+ EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\ -f2`
+ CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\ -f2`
+ EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
+ CHECK_FILES=`grep ^extra_bins $KDUMP_CONFIG_FILE | cut -d\ -f2-`
+ EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
+ files="$KDUMP_CONFIG_FILE $kdump_kernel $EXTRA_BINS /etc/fstab"
+
+ check_exist "$files" && check_executable "$EXTRA_BINS"
+ [ $? -ne 0 ] && return 2
If return value checking is following the
function invocation, using "$?"
direclty in checking is fine to me, Just like it is doing in this place,
I say so because there's a different place in below block.
+
+ for file in $files; do
+ time_stamp=`stat -c "%Y" $file`
+ if [ "$time_stamp" -gt "$image_time" ]; then
+ modified_files="$modified_files $file"
+ fi
+ done
+ if [ -n "$modified_files" ]; then
+ echo "Detected change(s) in the following file(s):"
+ echo -n " "; echo "$modified_files" | sed 's/\s/\n /g'
+ return 1
+ fi
+
+ return 0
+}
+
# returns 0 if system is not modified
# returns 1 if system is modified
# returns 2 if system modification is invalid
is_system_modified()
{
+ local ret
+
[[ -f $TARGET_INITRD ]] || return 1
+ is_files_modified
+ ret=$?
Here, I would like to not use temporary variable "ret" here.
Really
unnecessary.
> + if [ $ret -ne 0 ]; then
> + return $ret
> + fi
> +
> return 0
> }
>
> check_rebuild()
> {
> - local extra_modules modified_files=""
> + local extra_modules
> local _force_rebuild force_rebuild="0"
> local ret system_modified="0"
> local initramfs_has_fadump
> @@ -375,30 +415,8 @@ check_rebuild()
> #since last build of the image file
> if [ -f $TARGET_INITRD ]; then
> image_time=`stat -c "%Y" $TARGET_INITRD 2>/dev/null`
> - else
> - image_time=0
> fi
>
> - #also rebuild when Pacemaker cluster conf is changed and fence kdump is enabled.
> - modified_files=$(get_pcs_cluster_modified_files $image_time)
> -
> - EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\ -f2`
> - CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\ -f2`
> - EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
> - CHECK_FILES=`grep ^extra_bins $KDUMP_CONFIG_FILE | cut -d\ -f2-`
> - EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
> - files="$KDUMP_CONFIG_FILE $kdump_kernel $EXTRA_BINS /etc/fstab"
> -
> - check_exist "$files" && check_executable "$EXTRA_BINS"
> - [ $? -ne 0 ] && return 1
> -
> - for file in $files; do
> - time_stamp=`stat -c "%Y" $file`
> - if [ "$time_stamp" -gt "$image_time" ]; then
> - modified_files="$modified_files $file"
> - fi
> - done
> -
> is_system_modified
> ret=$?
> if [ $ret -eq 2 ]; then
> @@ -420,9 +438,6 @@ check_rebuild()
> echo -n "Force rebuild $TARGET_INITRD"; echo
> elif [ "$system_modified" != "0" ]; then
> :
> - elif [ -n "$modified_files" ]; then
> - echo "Detected change(s) in the following file(s):"
> - echo -n " "; echo "$modified_files" | sed 's/\s/\n
/g'
> else
> return 0
> fi
> --
> 2.5.5
>