Re: [PATCH] kdumpctl: warn users not to delete pre.d and post.d

Hi Pingfan, On 07/28/20 at 11:33am, Pingfan Liu wrote: > It is hard to detect the time that /etc/kdump is removed. And this failure > may cause out-of-date kdump.initrd. To keep things simple, just make > /etc/kdump/pre.d and post.d mandotary and warn users to re-create them if > they are be deleted. > > Signed-off-by: Pingfan Liu <piliu(a)redhat.com&gt; > --- > kdump.conf | 12 ++++++++---- > kdump.conf.5 | 12 ++++++++---- > kdumpctl | 4 ++++ > 3 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/kdump.conf b/kdump.conf > index 9f35e31..583cc82 100644 > --- a/kdump.conf > +++ b/kdump.conf > @@ -79,8 +79,10 @@ > # or script after the vmcore dump process terminates. > # The exit status of the current dump process is fed to > # the executable binary or script as its first argument. > -# If /etc/kdump/post.d directory exists, all files in > -# the directory are collectively sorted and executed in It looks better just replace above line with: All files under /etc/kdump/post.d are collectively sorted and executed in > +# Besides the directive "kdump_post", there could be some extended > +# scripts in the directory "/etc/kdump/post.d". > +# The directory is mandatory and should not be deleted. /etc/kdump should be also mandatory, but maybe no need to explicitly add the above. We just add an error checking in code and fail out, it

should be enough for a corner case?

BTW, we still need wrap lines so that do not over 80 columns. same to the pre.d part ..

> +# All files in it are collectively sorted and executed in > # lexical order, before binary or script specified > # kdump_post parameter is executed. > # > @@ -90,8 +92,10 @@ > # Exit status of this binary is interpreted as follows: > # 0 - continue with dump process as usual > # non 0 - run the final action (reboot/poweroff/halt) > -# If /etc/kdump/pre.d directory exists, all files in > -# the directory are collectively sorted and executed in > +# Besides the directive "kdump_pre", there could be some extended > +# scripts in the directory "/etc/kdump/pre.d". > +# The directory is mandatory and should not be deleted. > +# All files in it are collectively sorted and executed in > # lexical order, after binary or script specified > # kdump_pre parameter is executed. > # Even if the binary or script in /etc/kdump/pre.d directory > diff --git a/kdump.conf.5 b/kdump.conf.5 > index c362963..b8f2771 100644 > --- a/kdump.conf.5 > +++ b/kdump.conf.5 > @@ -109,8 +109,10 @@ status of the current dump process is fed to the kdump_post > executable as its first argument($1). Executable can modify > it to indicate the new exit status of succeeding dump process, > .PP > -If /etc/kdump/post.d directory exists, All files in > -the directory are collectively sorted and executed in > +Besides the directive "kdump_post", there could be some extended > +scripts in the directory "/etc/kdump/post.d". > +The directory is mandatory and should not be deleted. > +All files in it are collectively sorted and executed in > lexical order, before binary or script specified > kdump_post parameter is executed. > .PP > @@ -129,8 +131,10 @@ as follows: > .PP > non 0 - run the final action (reboot/poweroff/halt) > .PP > -If /etc/kdump/pre.d directory exists, all files in > -the directory are collectively sorted and executed in > +Besides the directive "kdump_pre", there could be some extended > +scripts in the directory "/etc/kdump/pre.d". > +The directory is mandatory and should not be deleted. > +All files in it are collectively sorted and executed in > lexical order, after binary or script specified > kdump_pre parameter is executed. > Even if the binary or script in /etc/kdump/pre.d directory > diff --git a/kdumpctl b/kdumpctl > index 42c11d3..43acf10 100755 > --- a/kdumpctl > +++ b/kdumpctl > @@ -339,6 +339,10 @@ 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 ] || [ ! -d /etc/kdump/pre.d ]; then > + echo "/etc/kdump/pre.d and post.d is mandatory, please re-create them before rebuilding" It is not suggested to use "please", it would be good just fail like: if [ ! -d /etc/kdump/post.d ].. echo "Error: /ec/kdump/post.d/ not found." .. Just use the function check_exist() will be good enough instead of check them one by one with separate code.