[v2] tools/hotplug: allow tuning of xenwatchdogd arguments

[PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Olaf Hering 3 years, 4 months ago

Currently the arguments for xenwatchdogd are hardcoded with 15s
keep-alive interval and 30s timeout.

It is not possible to tweak these values via
/etc/systemd/system/xen-watchdog.service.d/*.conf because ExecStart
can not be replaced. The only option would be a private copy
/etc/systemd/system/xen-watchdog.service, which may get out of sync
with the Xen provided xen-watchdog.service.

Adjust the service file to recognize XENWATCHDOGD_ARGS= in a
private unit configuration file.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---

v2: fix "test -n" in init.d

 tools/hotplug/Linux/init.d/xen-watchdog.in          | 7 ++++++-
 tools/hotplug/Linux/systemd/xen-watchdog.service.in | 4 +++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/xen-watchdog.in b/tools/hotplug/Linux/init.d/xen-watchdog.in
index c05f1f6b6a..b36a94bd8e 100644
--- a/tools/hotplug/Linux/init.d/xen-watchdog.in
+++ b/tools/hotplug/Linux/init.d/xen-watchdog.in
@@ -19,6 +19,11 @@
 
 . @XEN_SCRIPT_DIR@/hotplugpath.sh
 
+xencommons_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
+
+test -f $xencommons_config/xencommons && . $xencommons_config/xencommons
+
+test -n "$XENWATCHDOGD_ARGS" || XENWATCHDOGD_ARGS='15 30'
 DAEMON=${sbindir}/xenwatchdogd
 base=$(basename $DAEMON)
 
@@ -46,7 +51,7 @@ start() {
 	local r
 	echo -n $"Starting domain watchdog daemon: "
 
-	$DAEMON 30 15
+	$DAEMON $XENWATCHDOGD_ARGS
 	r=$?
 	[ "$r" -eq 0 ] && success $"$base startup" || failure $"$base startup"
 	echo
diff --git a/tools/hotplug/Linux/systemd/xen-watchdog.service.in b/tools/hotplug/Linux/systemd/xen-watchdog.service.in
index 1eecd2a616..637ab7fd7f 100644
--- a/tools/hotplug/Linux/systemd/xen-watchdog.service.in
+++ b/tools/hotplug/Linux/systemd/xen-watchdog.service.in
@@ -6,7 +6,9 @@ ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=forking
-ExecStart=@sbindir@/xenwatchdogd 30 15
+Environment="XENWATCHDOGD_ARGS=30 15"
+EnvironmentFile=-@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
+ExecStart=@sbindir@/xenwatchdogd $XENWATCHDOGD_ARGS
 KillSignal=USR1
 
 [Install]

Re: [PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Wei Liu 3 years, 4 months ago

On Thu, Dec 03, 2020 at 07:34:36AM +0100, Olaf Hering wrote:
> Currently the arguments for xenwatchdogd are hardcoded with 15s
> keep-alive interval and 30s timeout.
> 
> It is not possible to tweak these values via
> /etc/systemd/system/xen-watchdog.service.d/*.conf because ExecStart
> can not be replaced. The only option would be a private copy
> /etc/systemd/system/xen-watchdog.service, which may get out of sync
> with the Xen provided xen-watchdog.service.
> 
> Adjust the service file to recognize XENWATCHDOGD_ARGS= in a
> private unit configuration file.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> 
> v2: fix "test -n" in init.d
> 
>  tools/hotplug/Linux/init.d/xen-watchdog.in          | 7 ++++++-
>  tools/hotplug/Linux/systemd/xen-watchdog.service.in | 4 +++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/hotplug/Linux/init.d/xen-watchdog.in b/tools/hotplug/Linux/init.d/xen-watchdog.in
> index c05f1f6b6a..b36a94bd8e 100644
> --- a/tools/hotplug/Linux/init.d/xen-watchdog.in
> +++ b/tools/hotplug/Linux/init.d/xen-watchdog.in
> @@ -19,6 +19,11 @@
>  
>  . @XEN_SCRIPT_DIR@/hotplugpath.sh
>  
> +xencommons_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
> +
> +test -f $xencommons_config/xencommons && . $xencommons_config/xencommons
> +
> +test -n "$XENWATCHDOGD_ARGS" || XENWATCHDOGD_ARGS='15 30'
>  DAEMON=${sbindir}/xenwatchdogd
>  base=$(basename $DAEMON)
>  
> @@ -46,7 +51,7 @@ start() {
>  	local r
>  	echo -n $"Starting domain watchdog daemon: "
>  
> -	$DAEMON 30 15
> +	$DAEMON $XENWATCHDOGD_ARGS

Did you accidentally swap 15 and 30 in XENWATCHDOGD_ARGS above? I see no
reasoning in the commit message for this change.

No need to resend.  I can fix it for you. But please confirm if that's a
mistake.

Wei.

>  	r=$?
>  	[ "$r" -eq 0 ] && success $"$base startup" || failure $"$base startup"
>  	echo
> diff --git a/tools/hotplug/Linux/systemd/xen-watchdog.service.in b/tools/hotplug/Linux/systemd/xen-watchdog.service.in
> index 1eecd2a616..637ab7fd7f 100644
> --- a/tools/hotplug/Linux/systemd/xen-watchdog.service.in
> +++ b/tools/hotplug/Linux/systemd/xen-watchdog.service.in
> @@ -6,7 +6,9 @@ ConditionPathExists=/proc/xen/capabilities
>  
>  [Service]
>  Type=forking
> -ExecStart=@sbindir@/xenwatchdogd 30 15
> +Environment="XENWATCHDOGD_ARGS=30 15"
> +EnvironmentFile=-@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
> +ExecStart=@sbindir@/xenwatchdogd $XENWATCHDOGD_ARGS
>  KillSignal=USR1
>  
>  [Install]

Re: [PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Olaf Hering 3 years, 4 months ago

Am Fri, 4 Dec 2020 10:53:15 +0000
schrieb Wei Liu <wl@xen.org>:

> Did you accidentally swap 15 and 30 in XENWATCHDOGD_ARGS above?

This is indeed a mistake. Thanks for spotting.

Olaf

Re: [PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Wei Liu 3 years, 4 months ago

On Fri, Dec 04, 2020 at 12:46:20PM +0100, Olaf Hering wrote:
> Am Fri, 4 Dec 2020 10:53:15 +0000
> schrieb Wei Liu <wl@xen.org>:
> 
> > Did you accidentally swap 15 and 30 in XENWATCHDOGD_ARGS above?
> 
> This is indeed a mistake. Thanks for spotting.

Fixed and pushed. Thanks.

Wei.

Re: [PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Jürgen Groß 3 years, 4 months ago

On 03.12.20 07:34, Olaf Hering wrote:
> Currently the arguments for xenwatchdogd are hardcoded with 15s
> keep-alive interval and 30s timeout.
> 
> It is not possible to tweak these values via
> /etc/systemd/system/xen-watchdog.service.d/*.conf because ExecStart
> can not be replaced. The only option would be a private copy
> /etc/systemd/system/xen-watchdog.service, which may get out of sync
> with the Xen provided xen-watchdog.service.
> 
> Adjust the service file to recognize XENWATCHDOGD_ARGS= in a
> private unit configuration file.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> 
> v2: fix "test -n" in init.d
> 
>   tools/hotplug/Linux/init.d/xen-watchdog.in          | 7 ++++++-
>   tools/hotplug/Linux/systemd/xen-watchdog.service.in | 4 +++-
>   2 files changed, 9 insertions(+), 2 deletions(-)

Could you please add a section for XENWATCHDOGD_ARGS in
tools/hotplug/Linux/init.d/sysconfig.xencommons.in ?


Juergen

Re: [PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Olaf Hering 3 years, 4 months ago

Am Thu, 3 Dec 2020 07:47:58 +0100
schrieb Jürgen Groß <jgross@suse.com>:

> Could you please add a section for XENWATCHDOGD_ARGS in
> tools/hotplug/Linux/init.d/sysconfig.xencommons.in ?

No. Such details have to go into a to-be-written xencommons(8).

There will be a xenwatchdogd(8) shortly, which will cover this knob.


Olaf

Re: [PATCH v2] tools/hotplug: allow tuning of xenwatchdogd arguments

Posted by Wei Liu 3 years, 4 months ago

On Thu, Dec 03, 2020 at 08:19:39AM +0100, Olaf Hering wrote:
> Am Thu, 3 Dec 2020 07:47:58 +0100
> schrieb Jürgen Groß <jgross@suse.com>:
> 
> > Could you please add a section for XENWATCHDOGD_ARGS in
> > tools/hotplug/Linux/init.d/sysconfig.xencommons.in ?
> 
> No. Such details have to go into a to-be-written xencommons(8).
> 
> There will be a xenwatchdogd(8) shortly, which will cover this knob.

The more manpages the better. :-)

Wei.

> 
> 
> Olaf