[PATCH V2 0/4] Rebuild initrd in case of watchdog state change

[Pratyush Anand]

Fedora dracut-044-75 release has all the component needed to support initrd
rebuild in case of watchdog state change. Therefore sending these patches
for inclusion in kexec-tools.

Please note that these patches should only be added in those fedora
versions which has dracut version dracut-044-75 or latter. Anyway, we
already have dependency specified in spec file as well.

Changes since V1:
 - Removed patch 4/5 which was updating kdumpctl man page.
 - Added all necessary information in kdump.conf man page (patch 2/4).

Pratyush Anand (4):
  kdumpctl: force rebuild in case of watchdog state change
  watchdog: Add nowdt argument in kdump.conf
  watchdog: do not add watchdog module in rd.driver.pre now
  spec: Update to add dependency on dracut version >= 044-75

 dracut-module-setup.sh |  9 ---------
 kdump.conf             | 14 ++++++++++++++
 kdump.conf.5           | 11 +++++++++++
 kdumpctl               | 34 +++++++++++++++++++++++++++++++++-
 kexec-tools.spec       |  3 ++-
 mkdumprd               |  8 +++++++-
 6 files changed, 67 insertions(+), 12 deletions(-)

-- 
2.5.5

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Pratyush Anand]

If state of a watchdog device is changed by an user after kdumpctl restart
then initramfs must be rebuilt on the basis of new watchdog status.

Testing:
-------------------------------------------------------
Initramfs	wdt state
		Prev	Current		Result
-------------------------------------------------------
Not Exist	NA	X		Rebuild
Exist		Inact	Inact		No Rebuild
Exist		Inact	Act		Force Rebuild
Exist		Act	Inact		Force Rebuild
Exist		Act	Act(Same wdt)	No Rebuild
Exist		Act	Act(Diff wdt)	Force Rebuild

Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
Acked-by: Dave Young <dyoung(a)redhat.com&gt;
---
 kdumpctl | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/kdumpctl b/kdumpctl
index c8fc54fc7677..12e7aa005c5d 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -427,6 +427,32 @@ check_dump_fs_modified()
 	return 1
 }
 
+check_wdt_modified()
+{
+	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' '  -f2)
+	[[ -z $NOWDT ]] || return 0
+	[[ -d /sys/class/watchdog/ ]] || return 0
+
+	for dir in /sys/class/watchdog/*; do
+		[[ -d "$dir" ]] || continue
+		[[ -f "$dir/state" ]] || continue
+		wdtdrv=$(< "$dir/device/modalias")
+		wdtdrv=$(modinfo $wdtdrv | grep filename | awk -F"kernel/" '{print
$2}')
+		active=$(< "$dir/state")
+		# rebuild when:
+		# module for this watchdog is not found and watchdog is active
+		# module for this watchdog is found and watchdog is inactive
+		lsinitrd $TARGET_INITRD | grep $wdtdrv &> /dev/null
+		if [ $? -ne 0 ]; then
+			[[ "$active" =  "active" ]] && return 1
+		else
+			[[ "$active" =  "inactive" ]] && return 1
+		fi
+	done
+
+	return 0
+}
+
 # returns 0 if system is not modified
 # returns 1 if system is modified
 # returns 2 if system modification is invalid
@@ -448,6 +474,12 @@ check_system_modified()
 		return $ret
 	fi
 
+	check_wdt_modified
+	if [ $? -ne 0 ]; then
+		echo "Detected change in watchdog state"
+		return 1
+	fi
+
 	return 0
 }
 
-- 
2.5.5

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Pratyush Anand]

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&gt;
---
 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.
 
 #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
 #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

[PATCH V2 3/4] watchdog: do not add watchdog module in rd.driver.pre now

[Pratyush Anand]

Now dracut takes care to add module for active watchdog.  Therefore we do
not need to pass iTCO_wdt  and lpc_ich module in rd.driver.pre specifically
here.

Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
Acked-by: Dave Young <dyoung(a)redhat.com&gt;
---
 dracut-module-setup.sh | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
index ecf7d3faeb66..7854af060dca 100755
--- a/dracut-module-setup.sh
+++ b/dracut-module-setup.sh
@@ -721,12 +721,3 @@ install() {
     # at some point of time.
     kdump_check_iscsi_targets
 }
-
-installkernel() {
-    wdt=$(lsmod|cut -f1 -d' '|grep "wdt$")
-    if [ -n "$wdt" ]; then
-        [ "$wdt" = "iTCO_wdt" ] && instmods lpc_ich
&&
-            echo "rd.driver.pre=lpc_ich,iTCO_wdt " >>
${initdir}/etc/cmdline.d/00-wdt.conf
-        instmods $wdt
-    fi
-}
-- 
2.5.5

[PATCH V2 4/4] spec: Update to add dependency on dracut version >= 044-75

[Pratyush Anand]

Fedora dracut version >= 044-75 must be installed to work with this set of
kexec-tools patches. Therefore adding dependency for them.

Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
---
 kexec-tools.spec | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kexec-tools.spec b/kexec-tools.spec
index c74e489b2032..63d9bc9aad19 100644
--- a/kexec-tools.spec
+++ b/kexec-tools.spec
@@ -45,7 +45,8 @@ Requires(post): systemd-units
 Requires(preun): systemd-units
 Requires(postun): systemd-units
 Requires(pre): coreutils sed zlib 
-Requires: dracut, dracut-network, ethtool
+Requires: dracut >= 044-75
+Requires: dracut-network, ethtool
 BuildRequires: zlib-devel zlib zlib-static elfutils-devel-static glib2-devel bzip2-devel
ncurses-devel bison flex lzo-devel snappy-devel
 BuildRequires: pkgconfig intltool gettext 
 BuildRequires: systemd-units
-- 
2.5.5

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Dave Young]

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&gt;
>  ---
>   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.

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.

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.

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?

>   
>   #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?

>   #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
>   
Thanks
Dave

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Xunlei Pang]

On 2016/07/11 at 10:47, Pratyush Anand wrote:
>  If state of a watchdog device is changed by an user after kdumpctl
> restart
>  then initramfs must be rebuilt on the basis of new watchdog status.
>
>  Testing:
>  -------------------------------------------------------
>  Initramfs	wdt state
>  		Prev	Current		Result
>  -------------------------------------------------------
>  Not Exist	NA	X		Rebuild
>  Exist		Inact	Inact		No Rebuild
>  Exist		Inact	Act		Force Rebuild
>  Exist		Act	Inact		Force Rebuild
>  Exist		Act	Act(Same wdt)	No Rebuild
>  Exist		Act	Act(Diff wdt)	Force Rebuild
>
>  Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
>  Acked-by: Dave Young <dyoung(a)redhat.com&gt;
>  ---
>   kdumpctl | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
>
>  diff --git a/kdumpctl b/kdumpctl
>  index c8fc54fc7677..12e7aa005c5d 100755
>  --- a/kdumpctl
>  +++ b/kdumpctl
>  @@ -427,6 +427,32 @@ check_dump_fs_modified()
>   	return 1
>   }
>   
>  +check_wdt_modified()
>  +{
>  +	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' '  -f2)
>  +	[[ -z $NOWDT ]] || return 0
>  +	[[ -d /sys/class/watchdog/ ]] || return 0
>  +
>  +	for dir in /sys/class/watchdog/*; do
>  +		[[ -d "$dir" ]] || continue 
If the wdt module was unloaded leaving an empty /sys/class/watchdog/,  it will hit here
and return,
then if the initramfs contains any wdt, I think we will miss a necessary rebuild.

Regards,
Xunlei

>  +		[[ -f "$dir/state" ]] || continue
>  +		wdtdrv=$(< "$dir/device/modalias")
>  +		wdtdrv=$(modinfo $wdtdrv | grep filename | awk -F"kernel/" '{print
> $2}')
>  +		active=$(< "$dir/state")
>  +		# rebuild when:
>  +		# module for this watchdog is not found and watchdog is active
>  +		# module for this watchdog is found and watchdog is inactive
>  +		lsinitrd $TARGET_INITRD | grep $wdtdrv &> /dev/null
>  +		if [ $? -ne 0 ]; then
>  +			[[ "$active" =  "active" ]] && return 1
>  +		else
>  +			[[ "$active" =  "inactive" ]] && return 1
>  +		fi
>  +	done
>  +
>  +	return 0
>  +}
>  +
>   # returns 0 if system is not modified
>   # returns 1 if system is modified
>   # returns 2 if system modification is invalid
>  @@ -448,6 +474,12 @@ check_system_modified()
>   		return $ret
>   	fi
>   
>  +	check_wdt_modified
>  +	if [ $? -ne 0 ]; then
>  +		echo "Detected change in watchdog state"
>  +		return 1
>  +	fi
>  +
>   	return 0
>   }
>

[Don Zickus]

On Mon, Jul 11, 2016 at 08:17:57AM +0530, Pratyush Anand wrote:
>  Fedora dracut-044-75 release has all the component needed to support
> initrd
>  rebuild in case of watchdog state change. Therefore sending these patches
>  for inclusion in kexec-tools.
>  
>  Please note that these patches should only be added in those fedora
>  versions which has dracut version dracut-044-75 or latter. Anyway, we
>  already have dependency specified in spec file as well. 
Hi Pratyush,

Based on comments, would it make sense to list some of the test cases you
did.  There are lots of oddball corner cases here, perhaps it makes sense
for us to review them and make sure you covered the big ones?

Cheers,
Don

> 
> Changes since V1:
>  - Removed patch 4/5 which was updating kdumpctl man page.
>  - Added all necessary information in kdump.conf man page (patch 2/4).
> 
> Pratyush Anand (4):
>   kdumpctl: force rebuild in case of watchdog state change
>   watchdog: Add nowdt argument in kdump.conf
>   watchdog: do not add watchdog module in rd.driver.pre now
>   spec: Update to add dependency on dracut version >= 044-75
> 
>  dracut-module-setup.sh |  9 ---------
>  kdump.conf             | 14 ++++++++++++++
>  kdump.conf.5           | 11 +++++++++++
>  kdumpctl               | 34 +++++++++++++++++++++++++++++++++-
>  kexec-tools.spec       |  3 ++-
>  mkdumprd               |  8 +++++++-
>  6 files changed, 67 insertions(+), 12 deletions(-)
> 
> -- 
> 2.5.5
>

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Pratyush Anand]

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&gt;
>  > ---
>  >  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....

>  
>  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?

>  
>  >  #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

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Pratyush Anand]

Hi Xunlei,

Thanks for your comment.

On 11/07/2016:08:42:49 PM, Xunlei Pang wrote:
>  On 2016/07/11 at 10:47, Pratyush Anand wrote:
>  > If state of a watchdog device is changed by an user after kdumpctl restart
>  > then initramfs must be rebuilt on the basis of new watchdog status.
>  >
>  > Testing:
>  > -------------------------------------------------------
>  > Initramfs	wdt state
>  > 		Prev	Current		Result
>  > -------------------------------------------------------
>  > Not Exist	NA	X		Rebuild
>  > Exist		Inact	Inact		No Rebuild
>  > Exist		Inact	Act		Force Rebuild
>  > Exist		Act	Inact		Force Rebuild
>  > Exist		Act	Act(Same wdt)	No Rebuild
>  > Exist		Act	Act(Diff wdt)	Force Rebuild
>  >
>  > Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
>  > Acked-by: Dave Young <dyoung(a)redhat.com&gt;
>  > ---
>  >  kdumpctl | 32 ++++++++++++++++++++++++++++++++
>  >  1 file changed, 32 insertions(+)
>  >
>  > diff --git a/kdumpctl b/kdumpctl
>  > index c8fc54fc7677..12e7aa005c5d 100755
>  > --- a/kdumpctl
>  > +++ b/kdumpctl
>  > @@ -427,6 +427,32 @@ check_dump_fs_modified()
>  >  	return 1
>  >  }
>  >  
>  > +check_wdt_modified()
>  > +{
>  > +	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' '  -f2)
>  > +	[[ -z $NOWDT ]] || return 0
>  > +	[[ -d /sys/class/watchdog/ ]] || return 0
>  > +
>  > +	for dir in /sys/class/watchdog/*; do
>  > +		[[ -d "$dir" ]] || continue
>  
>  If the wdt module was unloaded leaving an empty /sys/class/watchdog/,  it will hit here
> and return,
>  then if the initramfs contains any wdt, I think we will miss a necessary rebuild. 
Yes, you are right, I did not consider this test case. But not sure, how to
check that. Because, when there was not any directory in /sys/class/watchdog/,
then we must rebuild if there is any watchdog driver in kdump kernel initrd,
which is not easy to check.
So, either we need to keep a variable in initrd which tells us that there
exists atleast one watchdog module in initrd.
OR, we can leave it as it is, because I do not see any disadvantage even when a
watchdog module is added for inactive wdt (In fact we add iTCO_wdt currently
without checking its state).

>  
>  > +		[[ -f "$dir/state" ]] || continue
>  > +		wdtdrv=$(< "$dir/device/modalias")
>  > +		wdtdrv=$(modinfo $wdtdrv | grep filename | awk -F"kernel/" '{print
> $2}')
>  > +		active=$(< "$dir/state")
>  > +		# rebuild when:
>  > +		# module for this watchdog is not found and watchdog is active
>  > +		# module for this watchdog is found and watchdog is inactive
>  > +		lsinitrd $TARGET_INITRD | grep $wdtdrv &> /dev/null
>  > +		if [ $? -ne 0 ]; then
>  > +			[[ "$active" =  "active" ]] && return 1
>  > +		else
>  > +			[[ "$active" =  "inactive" ]] && return 1
>  > +		fi
>  > +	done
>  > +
>  > +	return 0
>  > +}
>  > +
>  >  # returns 0 if system is not modified
>  >  # returns 1 if system is modified
>  >  # returns 2 if system modification is invalid
>  > @@ -448,6 +474,12 @@ check_system_modified()
>  >  		return $ret
>  >  	fi
>  >  
>  > +	check_wdt_modified
>  > +	if [ $? -ne 0 ]; then
>  > +		echo "Detected change in watchdog state"
>  > +		return 1
>  > +	fi
>  > +
>  >  	return 0
>  >  }
>  >   
~Pratyush

[Pratyush Anand]

Hi Don,

On 11/07/2016:11:02:06 AM, Don Zickus wrote:
>  On Mon, Jul 11, 2016 at 08:17:57AM +0530, Pratyush Anand wrote:
>  > Fedora dracut-044-75 release has all the component needed to support initrd
>  > rebuild in case of watchdog state change. Therefore sending these patches
>  > for inclusion in kexec-tools.
>  > 
>  > Please note that these patches should only be added in those fedora
>  > versions which has dracut version dracut-044-75 or latter. Anyway, we
>  > already have dependency specified in spec file as well.
>  
>  Hi Pratyush,
>  
>  Based on comments, would it make sense to list some of the test cases you
>  did.  There are lots of oddball corner cases here, perhaps it makes sense
>  for us to review them and make sure you covered the big ones? 
I have mentioned test cases which I have tested in commit log of patch 1/4.

Yes, I agree there might be some corner cases as one pointed by Xunlei, which
would still need further deliberations.

~Pratyush
> 
> Cheers,
> Don
> 
> > 
> > Changes since V1:
> >  - Removed patch 4/5 which was updating kdumpctl man page.
> >  - Added all necessary information in kdump.conf man page (patch 2/4).
> > 
> > Pratyush Anand (4):
> >   kdumpctl: force rebuild in case of watchdog state change
> >   watchdog: Add nowdt argument in kdump.conf
> >   watchdog: do not add watchdog module in rd.driver.pre now
> >   spec: Update to add dependency on dracut version >= 044-75
> > 
> >  dracut-module-setup.sh |  9 ---------
> >  kdump.conf             | 14 ++++++++++++++
> >  kdump.conf.5           | 11 +++++++++++
> >  kdumpctl               | 34 +++++++++++++++++++++++++++++++++-
> >  kexec-tools.spec       |  3 ++-
> >  mkdumprd               |  8 +++++++-
> >  6 files changed, 67 insertions(+), 12 deletions(-)
> > 
> > -- 
> > 2.5.5
> >

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Dave Young]

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&gt;
> >> ---
> >>  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

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Pratyush Anand]

On 12/07/2016:02:18:21 PM, Dave Young wrote:
>  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&gt;
>  >>> ---
>  >>>  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? 
I think, we can avoid using these two conflicting options. Will go with
dracut_args in next revision.

~Pratyush
> 
> > 
> >>
> >> 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
>

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Pratyush Anand]

On 12/07/2016:02:18:21 PM, Dave Young wrote:
>  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&gt;
>  >>> ---
>  >>>  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"? 
OK, now going with dracut_argas, nowdt will not be used, but just for my
knowledge, I was not knowing that we need to provide default option in comment.
I thought, default is already there, so I should add a comment which one will
need to change the default behavior.

~Pratyush

> 
> > 
> >>
> >>>  #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
>

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Dave Young]

Hi, Pratyush

> >>>>  
> >>>>  #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"?
>  
>  OK, now going with dracut_argas, nowdt will not be used, but just for my
>  knowledge, I was not knowing that we need to provide default option in comment.
>  I thought, default is already there, so I should add a comment which one will
>  need to change the default behavior.
>   
Hmm, apologize that I did not notice that they are not consistent, some
of them use default value, other of them do not use default value. Just
like force_rebuild above it uses 1 so please ignore the comment.

But I still think dracut_args is better than a new option.

-- 
Thanks
Dave

[PATCH V2 2/4] watchdog: Add nowdt argument in kdump.conf

[Pratyush Anand]

On 12/07/2016:03:37:24 PM, Dave Young wrote:
>  Hi, Pratyush
>  
>  >>>>>  
>  >>>>>  #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"?
>  > 
>  > OK, now going with dracut_argas, nowdt will not be used, but just for my
>  > knowledge, I was not knowing that we need to provide default option in comment.
>  > I thought, default is already there, so I should add a comment which one will
>  > need to change the default behavior.
>  > 
>  
>  Hmm, apologize that I did not notice that they are not consistent, some
>  of them use default value, other of them do not use default value. Just
>  like force_rebuild above it uses 1 so please ignore the comment.
>  
>  But I still think dracut_args is better than a new option. 
Yes, yes, I agreed on that. I will modify accordingly in my next revision.

~Pratyush

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Xunlei Pang]

On 2016/07/12 at 13:24, Pratyush Anand wrote:
>  Hi Xunlei,
>
>  Thanks for your comment.
>
>  On 11/07/2016:08:42:49 PM, Xunlei Pang wrote:
> > On 2016/07/11 at 10:47, Pratyush Anand wrote:
> >> If state of a watchdog device is changed by an user after kdumpctl restart
> >> then initramfs must be rebuilt on the basis of new watchdog status.
> >>
> >> Testing:
> >> -------------------------------------------------------
> >> Initramfs	wdt state
> >> 		Prev	Current		Result
> >> -------------------------------------------------------
> >> Not Exist	NA	X		Rebuild
> >> Exist		Inact	Inact		No Rebuild
> >> Exist		Inact	Act		Force Rebuild
> >> Exist		Act	Inact		Force Rebuild
> >> Exist		Act	Act(Same wdt)	No Rebuild
> >> Exist		Act	Act(Diff wdt)	Force Rebuild
> >>
> >> Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
> >> Acked-by: Dave Young <dyoung(a)redhat.com&gt;
> >> ---
> >>  kdumpctl | 32 ++++++++++++++++++++++++++++++++
> >>  1 file changed, 32 insertions(+)
> >>
> >> diff --git a/kdumpctl b/kdumpctl
> >> index c8fc54fc7677..12e7aa005c5d 100755
> >> --- a/kdumpctl
> >> +++ b/kdumpctl
> >> @@ -427,6 +427,32 @@ check_dump_fs_modified()
> >>  	return 1
> >>  }
> >>  
> >> +check_wdt_modified()
> >> +{
> >> +	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' '  -f2)
> >> +	[[ -z $NOWDT ]] || return 0
> >> +	[[ -d /sys/class/watchdog/ ]] || return 0
> >> +
> >> +	for dir in /sys/class/watchdog/*; do
> >> +		[[ -d "$dir" ]] || continue
> > If the wdt module was unloaded leaving an empty /sys/class/watchdog/,  it will hit
> here and return,
> > then if the initramfs contains any wdt, I think we will miss a necessary rebuild.
>  Yes, you are right, I did not consider this test case. But not sure, how to
>  check that. Because, when there was not any directory in /sys/class/watchdog/,
>  then we must rebuild if there is any watchdog driver in kdump kernel initrd,
>  which is not easy to check.
>  So, either we need to keep a variable in initrd which tells us that there
>  exists atleast one watchdog module in initrd.
>  OR, we can leave it as it is, because I do not see any disadvantage even when a
>  watchdog module is added for inactive wdt (In fact we add iTCO_wdt currently
>  without checking its state). 

Can we utilize "etc/cmdline.d/00-watchdog.conf" to achieve this?

Regards,
Xunlei

> >> +		[[ -f "$dir/state" ]] || continue
> >> +		wdtdrv=$(< "$dir/device/modalias")
> >> +		wdtdrv=$(modinfo $wdtdrv | grep filename | awk -F"kernel/"
> '{print $2}')
> >> +		active=$(< "$dir/state")
> >> +		# rebuild when:
> >> +		# module for this watchdog is not found and watchdog is active
> >> +		# module for this watchdog is found and watchdog is inactive
> >> +		lsinitrd $TARGET_INITRD | grep $wdtdrv &> /dev/null
> >> +		if [ $? -ne 0 ]; then
> >> +			[[ "$active" =  "active" ]] && return 1
> >> +		else
> >> +			[[ "$active" =  "inactive" ]] && return 1
> >> +		fi
> >> +	done
> >> +
> >> +	return 0
> >> +}
> >> +
> >>  # returns 0 if system is not modified
> >>  # returns 1 if system is modified
> >>  # returns 2 if system modification is invalid
> >> @@ -448,6 +474,12 @@ check_system_modified()
> >>  		return $ret
> >>  	fi
> >>  
> >> +	check_wdt_modified
> >> +	if [ $? -ne 0 ]; then
> >> +		echo "Detected change in watchdog state"
> >> +		return 1
> >> +	fi
> >> +
> >>  	return 0
> >>  }
> >>  
>  ~Pratyush

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Pratyush Anand]

On 15/07/2016:04:16:17 PM, Xunlei Pang wrote:
>  On 2016/07/12 at 13:24, Pratyush Anand wrote:
>  > Hi Xunlei,
>  >
>  > Thanks for your comment.
>  >
>  > On 11/07/2016:08:42:49 PM, Xunlei Pang wrote:
>  >> On 2016/07/11 at 10:47, Pratyush Anand wrote:
>  >>> If state of a watchdog device is changed by an user after kdumpctl restart
>  >>> then initramfs must be rebuilt on the basis of new watchdog status.
>  >>>
>  >>> Testing:
>  >>> -------------------------------------------------------
>  >>> Initramfs	wdt state
>  >>> 		Prev	Current		Result
>  >>> -------------------------------------------------------
>  >>> Not Exist	NA	X		Rebuild
>  >>> Exist		Inact	Inact		No Rebuild
>  >>> Exist		Inact	Act		Force Rebuild
>  >>> Exist		Act	Inact		Force Rebuild
>  >>> Exist		Act	Act(Same wdt)	No Rebuild
>  >>> Exist		Act	Act(Diff wdt)	Force Rebuild
>  >>>
>  >>> Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
>  >>> Acked-by: Dave Young <dyoung(a)redhat.com&gt;
>  >>> ---
>  >>>  kdumpctl | 32 ++++++++++++++++++++++++++++++++
>  >>>  1 file changed, 32 insertions(+)
>  >>>
>  >>> diff --git a/kdumpctl b/kdumpctl
>  >>> index c8fc54fc7677..12e7aa005c5d 100755
>  >>> --- a/kdumpctl
>  >>> +++ b/kdumpctl
>  >>> @@ -427,6 +427,32 @@ check_dump_fs_modified()
>  >>>  	return 1
>  >>>  }
>  >>>  
>  >>> +check_wdt_modified()
>  >>> +{
>  >>> +	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' '  -f2)
>  >>> +	[[ -z $NOWDT ]] || return 0
>  >>> +	[[ -d /sys/class/watchdog/ ]] || return 0
>  >>> +
>  >>> +	for dir in /sys/class/watchdog/*; do
>  >>> +		[[ -d "$dir" ]] || continue
>  >> If the wdt module was unloaded leaving an empty /sys/class/watchdog/,  it will
> hit here and return,
>  >> then if the initramfs contains any wdt, I think we will miss a necessary
> rebuild.
>  > Yes, you are right, I did not consider this test case. But not sure, how to
>  > check that. Because, when there was not any directory in /sys/class/watchdog/,
>  > then we must rebuild if there is any watchdog driver in kdump kernel initrd,
>  > which is not easy to check.
>  > So, either we need to keep a variable in initrd which tells us that there
>  > exists atleast one watchdog module in initrd.
>  > OR, we can leave it as it is, because I do not see any disadvantage even when a
>  > watchdog module is added for inactive wdt (In fact we add iTCO_wdt currently
>  > without checking its state).
>  
>  
>  Can we utilize "etc/cmdline.d/00-watchdog.conf" to achieve this? 
I think yes. A non-empty etc/cmdline.d/00-watchdog.conf or even its existence
will reflect that there is atleast one active wdt.

Thanks!! Will send a topup patch.

~Pratyush

> 
> Regards,
> Xunlei
> 
> >>> +		[[ -f "$dir/state" ]] || continue
> >>> +		wdtdrv=$(< "$dir/device/modalias")
> >>> +		wdtdrv=$(modinfo $wdtdrv | grep filename | awk -F"kernel/"
'{print $2}')
> >>> +		active=$(< "$dir/state")
> >>> +		# rebuild when:
> >>> +		# module for this watchdog is not found and watchdog is active
> >>> +		# module for this watchdog is found and watchdog is inactive
> >>> +		lsinitrd $TARGET_INITRD | grep $wdtdrv &> /dev/null
> >>> +		if [ $? -ne 0 ]; then
> >>> +			[[ "$active" =  "active" ]] && return 1
> >>> +		else
> >>> +			[[ "$active" =  "inactive" ]] && return
1
> >>> +		fi
> >>> +	done
> >>> +
> >>> +	return 0
> >>> +}
> >>> +
> >>>  # returns 0 if system is not modified
> >>>  # returns 1 if system is modified
> >>>  # returns 2 if system modification is invalid
> >>> @@ -448,6 +474,12 @@ check_system_modified()
> >>>  		return $ret
> >>>  	fi
> >>>  
> >>> +	check_wdt_modified
> >>> +	if [ $? -ne 0 ]; then
> >>> +		echo "Detected change in watchdog state"
> >>> +		return 1
> >>> +	fi
> >>> +
> >>>  	return 0
> >>>  }
> >>>  
> > ~Pratyush

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Dave Young]

On 07/15/16 at 02:57pm, Pratyush Anand wrote:
>  On 15/07/2016:04:16:17 PM, Xunlei Pang wrote:
>  > On 2016/07/12 at 13:24, Pratyush Anand wrote:
>  > > Hi Xunlei,
>  > >
>  > > Thanks for your comment.
>  > >
>  > > On 11/07/2016:08:42:49 PM, Xunlei Pang wrote:
>  > >> On 2016/07/11 at 10:47, Pratyush Anand wrote:
>  > >>> If state of a watchdog device is changed by an user after kdumpctl
> restart
>  > >>> then initramfs must be rebuilt on the basis of new watchdog status.
>  > >>>
>  > >>> Testing:
>  > >>> -------------------------------------------------------
>  > >>> Initramfs	wdt state
>  > >>> 		Prev	Current		Result
>  > >>> -------------------------------------------------------
>  > >>> Not Exist	NA	X		Rebuild
>  > >>> Exist		Inact	Inact		No Rebuild
>  > >>> Exist		Inact	Act		Force Rebuild
>  > >>> Exist		Act	Inact		Force Rebuild
>  > >>> Exist		Act	Act(Same wdt)	No Rebuild
>  > >>> Exist		Act	Act(Diff wdt)	Force Rebuild
>  > >>>
>  > >>> Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
>  > >>> Acked-by: Dave Young <dyoung(a)redhat.com&gt;
>  > >>> ---
>  > >>>  kdumpctl | 32 ++++++++++++++++++++++++++++++++
>  > >>>  1 file changed, 32 insertions(+)
>  > >>>
>  > >>> diff --git a/kdumpctl b/kdumpctl
>  > >>> index c8fc54fc7677..12e7aa005c5d 100755
>  > >>> --- a/kdumpctl
>  > >>> +++ b/kdumpctl
>  > >>> @@ -427,6 +427,32 @@ check_dump_fs_modified()
>  > >>>  	return 1
>  > >>>  }
>  > >>>  
>  > >>> +check_wdt_modified()
>  > >>> +{
>  > >>> +	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' '  -f2)
>  > >>> +	[[ -z $NOWDT ]] || return 0
>  > >>> +	[[ -d /sys/class/watchdog/ ]] || return 0
>  > >>> +
>  > >>> +	for dir in /sys/class/watchdog/*; do
>  > >>> +		[[ -d "$dir" ]] || continue
>  > >> If the wdt module was unloaded leaving an empty /sys/class/watchdog/,  it
> will hit here and return,
>  > >> then if the initramfs contains any wdt, I think we will miss a necessary
> rebuild.
>  > > Yes, you are right, I did not consider this test case. But not sure, how to
>  > > check that. Because, when there was not any directory in /sys/class/watchdog/,
>  > > then we must rebuild if there is any watchdog driver in kdump kernel initrd,
>  > > which is not easy to check.
>  > > So, either we need to keep a variable in initrd which tells us that there
>  > > exists atleast one watchdog module in initrd.
>  > > OR, we can leave it as it is, because I do not see any disadvantage even when
> a
>  > > watchdog module is added for inactive wdt (In fact we add iTCO_wdt currently
>  > > without checking its state).
>  > 
>  > 
>  > Can we utilize "etc/cmdline.d/00-watchdog.conf" to achieve this?
>  
>  I think yes. A non-empty etc/cmdline.d/00-watchdog.conf or even its existence
>  will reflect that there is atleast one active wdt.
>  
>  Thanks!! Will send a topup patch. 
Pratyush, if you will send a new version, care to update the depends() in
module-setup.sh so that we can depends on wdt only when we need instead of
always add wdt dracut module? Though it only adds several hundreds Kb but
ideally we should only add it when we need it.

Thanks
Dave

[PATCH V2 1/4] kdumpctl: force rebuild in case of watchdog state change

[Pratyush Anand]

Hi Dave,

On 18/07/2016:03:20:55 PM, Dave Young wrote:
>  On 07/15/16 at 02:57pm, Pratyush Anand wrote:
>  > On 15/07/2016:04:16:17 PM, Xunlei Pang wrote:
>  > > On 2016/07/12 at 13:24, Pratyush Anand wrote:
>  > > > Hi Xunlei,
>  > > >
>  > > > Thanks for your comment.
>  > > >
>  > > > On 11/07/2016:08:42:49 PM, Xunlei Pang wrote:
>  > > >> On 2016/07/11 at 10:47, Pratyush Anand wrote:
>  > > >>> If state of a watchdog device is changed by an user after kdumpctl
> restart
>  > > >>> then initramfs must be rebuilt on the basis of new watchdog
> status.
>  > > >>>
>  > > >>> Testing:
>  > > >>> -------------------------------------------------------
>  > > >>> Initramfs	wdt state
>  > > >>> 		Prev	Current		Result
>  > > >>> -------------------------------------------------------
>  > > >>> Not Exist	NA	X		Rebuild
>  > > >>> Exist		Inact	Inact		No Rebuild
>  > > >>> Exist		Inact	Act		Force Rebuild
>  > > >>> Exist		Act	Inact		Force Rebuild
>  > > >>> Exist		Act	Act(Same wdt)	No Rebuild
>  > > >>> Exist		Act	Act(Diff wdt)	Force Rebuild
>  > > >>>
>  > > >>> Signed-off-by: Pratyush Anand <panand(a)redhat.com&gt;
>  > > >>> Acked-by: Dave Young <dyoung(a)redhat.com&gt;
>  > > >>> ---
>  > > >>>  kdumpctl | 32 ++++++++++++++++++++++++++++++++
>  > > >>>  1 file changed, 32 insertions(+)
>  > > >>>
>  > > >>> diff --git a/kdumpctl b/kdumpctl
>  > > >>> index c8fc54fc7677..12e7aa005c5d 100755
>  > > >>> --- a/kdumpctl
>  > > >>> +++ b/kdumpctl
>  > > >>> @@ -427,6 +427,32 @@ check_dump_fs_modified()
>  > > >>>  	return 1
>  > > >>>  }
>  > > >>>  
>  > > >>> +check_wdt_modified()
>  > > >>> +{
>  > > >>> +	NOWDT=$(grep ^nowdt $KDUMP_CONFIG_FILE | cut -d' ' 
> -f2)
>  > > >>> +	[[ -z $NOWDT ]] || return 0
>  > > >>> +	[[ -d /sys/class/watchdog/ ]] || return 0
>  > > >>> +
>  > > >>> +	for dir in /sys/class/watchdog/*; do
>  > > >>> +		[[ -d "$dir" ]] || continue
>  > > >> If the wdt module was unloaded leaving an empty /sys/class/watchdog/, 
> it will hit here and return,
>  > > >> then if the initramfs contains any wdt, I think we will miss a
> necessary rebuild.
>  > > > Yes, you are right, I did not consider this test case. But not sure, how
> to
>  > > > check that. Because, when there was not any directory in
> /sys/class/watchdog/,
>  > > > then we must rebuild if there is any watchdog driver in kdump kernel
> initrd,
>  > > > which is not easy to check.
>  > > > So, either we need to keep a variable in initrd which tells us that there
>  > > > exists atleast one watchdog module in initrd.
>  > > > OR, we can leave it as it is, because I do not see any disadvantage even
> when a
>  > > > watchdog module is added for inactive wdt (In fact we add iTCO_wdt
> currently
>  > > > without checking its state).
>  > > 
>  > > 
>  > > Can we utilize "etc/cmdline.d/00-watchdog.conf" to achieve this?
>  > 
>  > I think yes. A non-empty etc/cmdline.d/00-watchdog.conf or even its existence
>  > will reflect that there is atleast one active wdt.
>  > 
>  > Thanks!! Will send a topup patch.
>  
>  Pratyush, if you will send a new version, care to update the depends() in
>  module-setup.sh so that we can depends on wdt only when we need instead of
>  always add wdt dracut module? Though it only adds several hundreds Kb but
>  ideally we should only add it when we need it. 
Good idea, and that gives me another point to ponder. So, now we will not be
using "-a watchdog". We will just update dependency on dracut watchdog module
in
depend() when there exists atleast one active watchdog device.

Now, will it be good idea to parse dracut_args for "-o watchdog"? I think,
NOWDT
approach would be better to go with, because we are no longer updating
dracut_args.

What do you say?

~Pratyush
> 
> Thanks
> Dave