Hi, Pratyush
On 07/12/2016 11:18 AM, Pratyush Anand wrote:
Hi Dave,
Thanks for your review.
On 11/07/2016:02:09:38 PM, Dave Young wrote:
> Hi, Pratyush
>
> On 07/11/16 at 08:17am, Pratyush Anand wrote:
>> An user can pass "nowdt 1" in kdump.conf if watchdog driver for active
>> watchdog is not needed in kdump initramfs.
>>
>> Signed-off-by: Pratyush Anand <panand(a)redhat.com>
>> ---
>> kdump.conf | 14 ++++++++++++++
>> kdump.conf.5 | 11 +++++++++++
>> kdumpctl | 2 +-
>> mkdumprd | 8 +++++++-
>> 4 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/kdump.conf b/kdump.conf
>> index 54b581daf93b..29d3e50aec63 100644
>> --- a/kdump.conf
>> +++ b/kdump.conf
>> @@ -141,6 +141,19 @@
>> # - List of cluster node(s) separated by space to send fence_kdump
>> # notification to (this option is mandatory to enable fence_kdump).
>> #
>> +# nowdt <0 | 1>
>> +# - If a watchdog is active in first kernel then, we
>> +# must have it's module loaded in crash kernel, so
>> +# that either watchdog is deactivated or started
>> +# being kicked in second kernel. Otherwise, we might
>> +# face watchdog reboot when vmcore was being saved.
>> +# By default, nowdt is false and kdump initrd will
>> +# contain modules for active watchdog. If a watchdog
>> +# module (such as hp_wdt) has not been written using
>> +# watchdog-core framework then this option will not
>> +# have any effect and module will not be added.
>> +# Please note that only systemd watchdog daemon is
>> +# supported as watchdog kick application.
>
> Seems in kdumpctl code there is not checking with nowdt value, it just
> checks [[ -z $NOWDT ]], but here nowdt need a value of 0 or 1.
Yes, its wrong. Thanks. Will do it like:
[[ $NOWDT != 1 ]] && return 0
>
> Also, considering we have kdump.conf manpage, it sounds redundant to
> copy everything from comments to manpage. IMHO we can describe briefly
> in kdump.conf comments and add more details in the manpage.
ok.
>
> However, rethink about the param, since we have dracut_args. So it should
> be better to leave user to specify -o wdt in dracut_args, we can document
> it in kdump howto.
Hummm..you mean we may not use NOWDT and instead pass "-a watchdog" or
"-o
watchdog" to dracut_args in kdump.conf..
Currently, we take "-a watchdog" as default behavior. So, when there is
"nowdt"
in kdump.conf (or "nowdt 0") we pass "-a watchdog". If we do it
using
"dracut_args" argument of kdump.conf then after parsing config arguments we
will
have to check and parse "dracut_args" again to see if there was
"watchdog"
string in it, and if not found then we will have to add "-a watchdog".
Not sure if that would look cleaner....
I thought dracut wdt module will be included when there is active wdt in
1st kernel in hostonly mode even we do not add it explictly.., then we
only need document it if one do not want the wdt he can use dracut_args
to omit the module.
But we can also do like what you said above, I feel we should avoid new
options in kdump.conf if we can. What if one specify -o watchgog but he
do not modify the nowdt in kdump.conf?
>
> What is the default behavior of the drauct watchdog module if we do not
> add it explictly in kdump mkdumprd? Will the module being added if there
> is active wdt modules in 1st kernel?
By default watchdog module is not included, as it's check function returns 255.
>
>>
>> #raw /dev/vg/lv_kdump
>> #ext4 /dev/vg/lv_kdump
>> @@ -158,6 +171,7 @@ core_collector makedumpfile -l --message-level 1 -d 31
>> #extra_modules gfs2
>> #default shell
>> #force_rebuild 1
>> +#nowdt 1
>
> 1 should means true because it is not a shell script. So it conflicts
> with our design?
Yes, 1 means true, so when nowdt is true then do not include watchdog module,
right?
Right, but the default behavior is including wdt module, so the comment
should be "nowdt 0"?
>
>> #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 f1c2a2c1d24e..b01566df8aff 100644
>> --- a/kdump.conf.5
>> +++ b/kdump.conf.5
>> @@ -199,6 +199,17 @@ List of cluster node(s) separated by space to send
fence_kdump notification
>> to (this option is mandatory to enable fence_kdump).
>> .RE
>>
>> +.B nowdt <0 | 1>
>> +.RS
>> +If a watchdog is active in first kernel then, we must have it's module
>> +loaded in crash kernel, so that either watchdog is deactivated or started
>> +being kicked in second kernel. Otherwise, we might face watchdog reboot
>> +when vmcore was being saved. By default, nowdt is false and kdump initrd
>> +will contain modules for active watchdog. If a watchdog module (such as
>> +hp_wdt) has not been written using watchdog-core framework then this option
>> +will not have any effect and module will not be added. Please note that
>> +only systemd watchdog daemon is supported as watchdog kick application.
>> +.RE
>>
>> .SH DEPRECATED OPTIONS
>>
>> diff --git a/kdumpctl b/kdumpctl
>> index 12e7aa005c5d..06eb8d6af798 100755
>> --- a/kdumpctl
>> +++ b/kdumpctl
>> @@ -248,7 +248,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|dracut_args|fence_kdump_args|fence_kdump_nodes|nowdt)
>> [ -z "$config_val" ] && {
>> echo "Invalid kdump config value for option $config_opt."
>> return 1;
>> diff --git a/mkdumprd b/mkdumprd
>> index eb0d5e06fac9..f79733c3fd7e 100644
>> --- a/mkdumprd
>> +++ b/mkdumprd
>> @@ -15,9 +15,15 @@ SAVE_PATH=$(grep ^path $conf_file| cut -d' ' -f2)
>> [ -z "$SAVE_PATH" ] && SAVE_PATH=$DEFAULT_PATH
>> # strip the duplicated "/"
>> SAVE_PATH=$(echo $SAVE_PATH | tr -s /)
>> +NOWDT=$(grep ^nowdt $conf_file| cut -d' ' -f2)
>> +if [ -z $NOWDT ];then
>> + NOWDT="-a watchdog"
>> +else
>> + NOWDT="-o watchdog"
>> +fi
>>
>> extra_modules=""
>> -dracut_args=("--hostonly" "-o" "plymouth dash resume
ifcfg")
>> +dracut_args=("--hostonly" "-o" "plymouth dash resume
ifcfg" $NOWDT)
>> OVERRIDE_RESETTABLE=0
>>
>> add_dracut_arg() {
>> --
>> 2.5.5
>>
~Pratyush
--
Thanks
Dave