I think this one looks good. Just a typo.
On Thu, Jul 16, 2020 at 12:11 PM piliu <piliu(a)redhat.com> wrote:
On 07/16/2020 11:27 AM, Kairui Song wrote:
> Hi Pingfan,
>
> On Tue, Jul 7, 2020 at 1:23 PM Pingfan Liu <piliu(a)redhat.com> wrote:
>>
>> Checking modification against a file can not detect a removing file in
>> "/etc/kdump/post.d/ /etc/kdump/pre.d/". Hence for directory,
resorting to
>> modified time of directory.
>>
>> Signed-off-by: Pingfan Liu <piliu(a)redhat.com>
>> ---
>> kdumpctl | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/kdumpctl b/kdumpctl
>> index d3ec4d7..181c7d6 100755
>> --- a/kdumpctl
>> +++ b/kdumpctl
>> @@ -338,6 +338,7 @@ check_files_modified()
>>
>> EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\ -f2`
>> CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\ -f2`
>> + HOOKS_DIR="/etc/kdump/post.d/ /etc/kdump/pre.d/"
>> if [ -d /etc/kdump/post.d ]; then
>> for file in /etc/kdump/post.d/*; do
>> if [ -x "$file" ]; then
>> @@ -354,7 +355,7 @@ check_files_modified()
>> fi
>> CORE_COLLECTOR=`grep ^core_collector $KDUMP_CONFIG_FILE | cut -d\ -f2`
>> CORE_COLLECTOR=`type -P $CORE_COLLECTOR`
>> - EXTRA_BINS="$EXTRA_BINS $CHECK_FILES $POST_FILES $PRE_FILES"
>> + EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
>
> This seems break the detection of timestamp of each file, with this change:
> [root@localhost post.d]# touch test.sh
> [root@localhost post.d]# kdumpctl restart
> kexec: unloaded kdump kernel
> Stopping kdump: [OK]
> kexec: loaded kdump kernel
> Starting kdump: [OK]
>
> kdumpctl can't see the file change now. If I revert only this line, it
> works again. I think we need to check both the timestamp of the folder
> and files in it.
What about the following:
diff --git a/kdumpctl b/kdumpctl
index d3ec4d7..b1e0908 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -338,6 +338,7 @@ check_files_modified()
EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\ -f2`
CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\ -f2`
+ HOOKS="/etc/kdump/post.d/ /etc/kdump/pre.d/"
if [ -d /etc/kdump/post.d ]; then
for file in /etc/kdump/post.d/*; do
if [ -x "$file" ]; then
@@ -352,9 +353,11 @@ check_files_modified()
fi
done
fi
+ HOOKS="$HOOKS $POST_FILES $PRE_FILES"
CORE_COLLECTOR=`grep ^core_collector $KDUMP_CONFIG_FILE | cut
-d\ -f2`
CORE_COLLECTOR=`type -P $CORE_COLLECTOR`
- EXTRA_BINS="$EXTRA_BINS $CHECK_FILES $POST_FILES $PRE_FILES"
+ # POST_FILES and PRE_FILES are already checked against
executable, need not to check again.
+ 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 $CORE_COLLECTOR"
@@ -385,6 +388,8 @@ check_files_modified()
check_exist "$files" && check_executable
"$EXTRA_BINS"
[ $? -ne 0 ] && return 2
+ # HOOKS only need to check the modification here
+ files="$files $HOOK"
Should be HOOKS here right?
If it is ok, I will post V3.
Thanks,
Pingfan
>
>> 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
$CORE_COLLECTOR"
>> @@ -382,10 +383,11 @@ check_files_modified()
>> done
>> fi
>>
>> - check_exist "$files" && check_executable
"$EXTRA_BINS"
>> + check_exist "$files" && check_executable
"$EXTRA_BINS $PRE_FILES $POST_FILES"
>> [ $? -ne 0 ] && return 2
>>
>> - for file in $files; do
>> + check_list="$files $HOOKS_DIR"
>> + for file in $check_list; do
>> if [ -e "$file" ]; then
>> time_stamp=`stat -c "%Y" $file`
>> if [ "$time_stamp" -gt "$image_time"
]; then
>> --
>> 2.25.4
>>
>
>
--
Best Regards,
Kairui Song