On Tue, Apr 11, 2017 at 10:30 AM, Xunlei Pang <xpang(a)redhat.com> wrote:
On 04/11/2017 at 11:31 AM, Bhupesh Sharma wrote:
> This patch introduces the 'force_no_rebuild' option
> inside the 'kdump.conf' and its handling inside the 'kdumpctl'
> script.
>
> There might be several use cases, where a system admin
> decides that he doesn't need to rebuild the kdump initrd
> and wants to use an existing version of the same. In such cases,
> he can set the 'force_no_rebuild' option inside 'kdump.conf'
> to 1, to force the 'kdumpctl' script not to rebuild the kdump
> initrd.
>
> Signed-off-by: Bhupesh Sharma <bhsharma(a)redhat.com>
> ---
> kdump.conf | 8 ++++++++
> kdump.conf.5 | 10 ++++++++++
> kdumpctl | 22 +++++++++++++++++++++-
> 3 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/kdump.conf b/kdump.conf
> index ebf9587a6522..1e24e1bd9d61 100644
> --- a/kdump.conf
> +++ b/kdump.conf
> @@ -115,6 +115,13 @@
> # Specify 1 to force rebuilding kdump initrd every time when kdump
> # service starts.
> #
> +# force_no_rebuild <0 | 1>
> +# - By default, kdump initrd will be rebuilt when necessary.
> +# Specify 1 to bypass rebuilding of kdump initrd.
> +#
> +# force_no_rebuild and force_rebuild options are mutually
> +# exclusive and they should not be set to 1 simultaneously.
> +#
> # override_resettable <0 | 1>
> # - Usually an unresettable block device can't be a dump target.
> # Specifying 1 when you want to dump even though the block
> @@ -150,6 +157,7 @@ core_collector makedumpfile -l --message-level 1 -d 31
> #extra_modules gfs2
> #default shell
> #force_rebuild 1
> +#force_no_rebuild 1
> #dracut_args --omit-drivers "cfg80211 snd" --add-drivers "ext2
ext3"
> #fence_kdump_args -p 7410 -f auto -c 0 -i 10
> #fence_kdump_nodes node1 node2
> diff --git a/kdump.conf.5 b/kdump.conf.5
> index ca427699be53..b581964b906e 100644
> --- a/kdump.conf.5
> +++ b/kdump.conf.5
> @@ -166,6 +166,16 @@ By default, kdump initrd will only be rebuilt when necessary.
> Specify 1 to force rebuilding kdump initrd every time when kdump service starts.
> .RE
>
> +.B force_no_rebuild <0 | 1>
> +.RS
> +By default, kdump initrd will be rebuilt when necessary.
> +Specify 1 to bypass rebuilding of kdump initrd.
> +
> +.PP
> +force_no_rebuild and force_rebuild options are mutually exclusive and
> +they should not be set to 1 simultaneously.
> +.RE
> +
> .B override_resettable <0 | 1>
> .RS
> Usually an unresettable block device can't be a dump target. Specifying 1 means
> diff --git a/kdumpctl b/kdumpctl
> index 4d68be0c292d..3b85c0611d84 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -354,7 +354,7 @@ check_config()
> case "$config_opt" in
> \#* | "")
> ;;
> -
raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|default|force_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes)
> +
raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|default|force_rebuild|force_no_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes)
> [ -z "$config_val" ] && {
> echo "Invalid kdump config value for option
$config_opt."
> return 1;
> @@ -609,6 +609,7 @@ check_rebuild()
> {
> local extra_modules
> local _force_rebuild force_rebuild="0"
> + local _force_no_rebuild force_no_rebuild="0"
> local ret system_modified="0"
>
> check_boot_dir
> @@ -625,6 +626,15 @@ check_rebuild()
> return 1
> fi
>
> + _force_no_rebuild=`grep ^force_no_rebuild $KDUMP_CONFIG_FILE 2>/dev/null`
> + if [ $? -eq 0 ]; then
> + force_no_rebuild=`echo $_force_no_rebuild | cut -d' ' -f2`
> + if [ "$force_no_rebuild" != "0" ] && [
"$force_no_rebuild" != "1" ];then
> + echo "Error: force_no_rebuild value is invalid"
> + return 1
> + fi
> + fi
> +
> _force_rebuild=`grep ^force_rebuild $KDUMP_CONFIG_FILE 2>/dev/null`
> if [ $? -eq 0 ]; then
> force_rebuild=`echo $_force_rebuild | cut -d' ' -f2`
> @@ -634,6 +644,16 @@ check_rebuild()
> fi
> fi
>
> + if [[ "$force_no_rebuild" == "1" &&
"$force_rebuild" == "1" ]]; then
> + echo "Error: force_rebuild and force_no_rebuild are enabled
simultaneously in kdump.conf"
> + exit 1
Oh, better use "return 1" like other places, so it does not miss the
"Starting kdump: [FAILED]" output follows.
Otherwise the patch looks good to me.
I am wondering if starting with loading the kdump kernel is the right
behaviour in case both force_rebuild and force_no_rebuild are set to 1
in kdump.conf. It might be better to exit in such a case forcing the
system admin to correct the kdump.conf as existing or future scripts
which use kdump.conf should not execute contradictory code when both
these options are set in kdump.conf
Regards,
Bhupesh
>> + fi
>> +
>> + # Will not rebuild kdump initrd
>> + if [ "$force_no_rebuild" == "1" ]; then
>> + return 0
>> + fi
>> +
>> #will rebuild every time if extra_modules are specified
>> extra_modules=`grep ^extra_modules $KDUMP_CONFIG_FILE`
>> [ -n "$extra_modules" ] && force_rebuild="1"
>