On 07/28/2020 07:35 PM, Dave Young wrote:
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>
> ---
> 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
No sure if I
catch your meaning exactly. if /etc/kdump/pre.d is
mandatory, so is /etc/kdump. Both notes and code implicitly imply this
meaning.
should be enough for a corner case?
Yes, it would be enough.
BTW, we still need wrap lines so that do not over 80 columns.
same to the pre.d part ..
OK.
> +# 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.
OK.
Thanks for review. I will send V2