On 04/11/17 at 02:41pm, Bhupesh Sharma wrote:
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
I previously misunderstand check_rebuild as checking if rebuild is
needed. But actually it is check_and_do_rebuild, if it return 1 then
kdumpctl will exit with an error "Starting kdump: [FAILED]" and it will
not load kdump kernel.
So I would tend to agree with Xunlei, return 1 we can have the error
message of "Starting kdump: [FAILED]"
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"
>
_______________________________________________
kexec mailing list -- kexec(a)lists.fedoraproject.org
To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
Thanks
Dave