tools/hotplug/Linux/init.d/xencommons.in | 2 +- tools/hotplug/Linux/launch-xenstore.in | 56 ++++++++++++++++++------ tools/hotplug/Linux/systemd/xenstored.service.in | 2 +- 3 files changed, 44 insertions(+), 16 deletions(-)
A hard to trigger race with another unrelated systemd service and
xenstored.service unveiled a bug in the way how xenstored is launched
with systemd.
launch-xenstore may start either a daemon or a domain. In case a domain
is used, systemd-notify was called. If another service triggered a
restart of systemd while xenstored.service was executed, systemd may
temporary lose track of services with Type=notify. As a result,
xenstored.service would be marked as failed and units that depend on it
will not be started anymore. This breaks the enire Xen toolstack.
The chain of events is basically: xenstored.service sends the
notification to systemd, this is a one-way event. Then systemd may be
restarted by the other unit. During this time xenstored.service is done
and exits. Once systemd is done with its restart it collects the pending
notifications and childs. If it does not find the unit which sent the
notification it will declare it as failed.
A workaround for this scenario is to wait for a short time after sending
to notification. If systemd happens to restart it will still find the
unit it launched.
Adjust the callers of launch-xenstore to specifiy the init system.
Do not fork xenstored with systemd, preserve pid.
Be verbose about xenstored startup only with sysv to avoid interleaved
output in systemd journal. Do the same also for domain case, even if is
not strictly needed because init-xenstore-domain has no output.
The fix for upstream systemd which is supposed to fix it:
575b300b795b6 ("pid1: rework how we dispatch SIGCHLD and other signals")
v02:
- preserve Type=notify
Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
tools/hotplug/Linux/init.d/xencommons.in | 2 +-
tools/hotplug/Linux/launch-xenstore.in | 56 ++++++++++++++++++------
tools/hotplug/Linux/systemd/xenstored.service.in | 2 +-
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in
index 7fd6903b98..dcb0ce4b73 100644
--- a/tools/hotplug/Linux/init.d/xencommons.in
+++ b/tools/hotplug/Linux/init.d/xencommons.in
@@ -60,7 +60,7 @@ do_start () {
mkdir -m700 -p ${XEN_LOCK_DIR}
mkdir -p ${XEN_LOG_DIR}
- @XEN_SCRIPT_DIR@/launch-xenstore || exit 1
+ @XEN_SCRIPT_DIR@/launch-xenstore 'sysv' || exit 1
echo Setting domain 0 name, domid and JSON config...
${LIBEXEC_BIN}/xen-init-dom0 ${XEN_DOM0_UUID}
diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
index 991dec8d25..8ff243670d 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -15,6 +15,16 @@
# License along with this library; If not, see <http://www.gnu.org/licenses/>.
#
+initd=$1
+
+case "$initd" in
+ sysv|systemd) ;;
+ *)
+ echo "first argument must be 'sysv' or 'systemd'"
+ exit 1
+ ;;
+esac
+
XENSTORED=@XENSTORED@
. @XEN_SCRIPT_DIR@/hotplugpath.sh
@@ -44,15 +54,9 @@ timeout_xenstore () {
return 0
}
-test_xenstore && exit 0
-
-test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
-
-[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon
+run_xenstored () {
+ local maybe_exec=$1
-/bin/mkdir -p @XEN_RUN_DIR@
-
-[ "$XENSTORETYPE" = "daemon" ] && {
[ -z "$XENSTORED_ROOTDIR" ] && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
[ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log"
[ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
@@ -60,13 +64,30 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
echo "No xenstored found"
exit 1
}
+ $maybe_exec $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
+}
- echo -n Starting $XENSTORED...
- $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
+if test "$initd" = 'sysv' ; then
+ test_xenstore && exit 0
+fi
- systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED || exit 1
+test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
- exit 0
+[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon
+
+/bin/mkdir -p @XEN_RUN_DIR@
+
+[ "$XENSTORETYPE" = "daemon" ] && {
+ if test "$initd" = 'sysv' ; then
+ echo -n Starting $XENSTORED...
+ run_xenstored ''
+ timeout_xenstore $XENSTORED || exit 1
+ exit 0
+ else
+ XENSTORED_ARGS="$XENSTORED_ARGS -N"
+ run_xenstored 'exec'
+ exit 1
+ fi
}
[ "$XENSTORETYPE" = "domain" ] && {
@@ -76,9 +97,16 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory $XENSTORE_DOMAIN_SIZE"
[ -z "$XENSTORE_MAX_DOMAIN_SIZE" ] || XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --maxmem $XENSTORE_MAX_DOMAIN_SIZE"
- echo -n Starting $XENSTORE_DOMAIN_KERNEL...
+ if test "$initd" = 'sysv' ; then
+ echo -n Starting $XENSTORE_DOMAIN_KERNEL...
+ else
+ echo Starting $XENSTORE_DOMAIN_KERNEL...
+ fi
${LIBEXEC_BIN}/init-xenstore-domain $XENSTORE_DOMAIN_ARGS || exit 1
- systemd-notify --ready 2>/dev/null
+ if test "$initd" = 'systemd' ; then
+ systemd-notify --ready
+ sleep 9
+ fi
exit 0
}
diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in
index 80c1d408a5..c226eb3635 100644
--- a/tools/hotplug/Linux/systemd/xenstored.service.in
+++ b/tools/hotplug/Linux/systemd/xenstored.service.in
@@ -11,7 +11,7 @@ Type=notify
NotifyAccess=all
RemainAfterExit=true
ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
-ExecStart=@XEN_SCRIPT_DIR@/launch-xenstore
+ExecStart=@XEN_SCRIPT_DIR@/launch-xenstore 'systemd'
[Install]
WantedBy=multi-user.target
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Ping? On Fri, May 17, Olaf Hering wrote: > A hard to trigger race with another unrelated systemd service and > xenstored.service unveiled a bug in the way how xenstored is launched > with systemd. > > launch-xenstore may start either a daemon or a domain. In case a domain > is used, systemd-notify was called. If another service triggered a > restart of systemd while xenstored.service was executed, systemd may > temporary lose track of services with Type=notify. As a result, > xenstored.service would be marked as failed and units that depend on it > will not be started anymore. This breaks the enire Xen toolstack. > > The chain of events is basically: xenstored.service sends the > notification to systemd, this is a one-way event. Then systemd may be > restarted by the other unit. During this time xenstored.service is done > and exits. Once systemd is done with its restart it collects the pending > notifications and childs. If it does not find the unit which sent the > notification it will declare it as failed. > > A workaround for this scenario is to wait for a short time after sending > to notification. If systemd happens to restart it will still find the > unit it launched. > > Adjust the callers of launch-xenstore to specifiy the init system. > Do not fork xenstored with systemd, preserve pid. > Be verbose about xenstored startup only with sysv to avoid interleaved > output in systemd journal. Do the same also for domain case, even if is > not strictly needed because init-xenstore-domain has no output. > > The fix for upstream systemd which is supposed to fix it: > 575b300b795b6 ("pid1: rework how we dispatch SIGCHLD and other signals") > > v02: > - preserve Type=notify > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > --- > tools/hotplug/Linux/init.d/xencommons.in | 2 +- > tools/hotplug/Linux/launch-xenstore.in | 56 ++++++++++++++++++------ > tools/hotplug/Linux/systemd/xenstored.service.in | 2 +- > 3 files changed, 44 insertions(+), 16 deletions(-) > > diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in > index 7fd6903b98..dcb0ce4b73 100644 > --- a/tools/hotplug/Linux/init.d/xencommons.in > +++ b/tools/hotplug/Linux/init.d/xencommons.in > @@ -60,7 +60,7 @@ do_start () { > mkdir -m700 -p ${XEN_LOCK_DIR} > mkdir -p ${XEN_LOG_DIR} > > - @XEN_SCRIPT_DIR@/launch-xenstore || exit 1 > + @XEN_SCRIPT_DIR@/launch-xenstore 'sysv' || exit 1 > > echo Setting domain 0 name, domid and JSON config... > ${LIBEXEC_BIN}/xen-init-dom0 ${XEN_DOM0_UUID} > diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in > index 991dec8d25..8ff243670d 100644 > --- a/tools/hotplug/Linux/launch-xenstore.in > +++ b/tools/hotplug/Linux/launch-xenstore.in > @@ -15,6 +15,16 @@ > # License along with this library; If not, see <http://www.gnu.org/licenses/>. > # > > +initd=$1 > + > +case "$initd" in > + sysv|systemd) ;; > + *) > + echo "first argument must be 'sysv' or 'systemd'" > + exit 1 > + ;; > +esac > + > XENSTORED=@XENSTORED@ > > . @XEN_SCRIPT_DIR@/hotplugpath.sh > @@ -44,15 +54,9 @@ timeout_xenstore () { > return 0 > } > > -test_xenstore && exit 0 > - > -test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons > - > -[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon > +run_xenstored () { > + local maybe_exec=$1 > > -/bin/mkdir -p @XEN_RUN_DIR@ > - > -[ "$XENSTORETYPE" = "daemon" ] && { > [ -z "$XENSTORED_ROOTDIR" ] && XENSTORED_ROOTDIR="@XEN_LIB_STORED@" > [ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log" > [ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@ > @@ -60,13 +64,30 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF > echo "No xenstored found" > exit 1 > } > + $maybe_exec $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS > +} > > - echo -n Starting $XENSTORED... > - $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS > +if test "$initd" = 'sysv' ; then > + test_xenstore && exit 0 > +fi > > - systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED || exit 1 > +test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons > > - exit 0 > +[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon > + > +/bin/mkdir -p @XEN_RUN_DIR@ > + > +[ "$XENSTORETYPE" = "daemon" ] && { > + if test "$initd" = 'sysv' ; then > + echo -n Starting $XENSTORED... > + run_xenstored '' > + timeout_xenstore $XENSTORED || exit 1 > + exit 0 > + else > + XENSTORED_ARGS="$XENSTORED_ARGS -N" > + run_xenstored 'exec' > + exit 1 > + fi > } > > [ "$XENSTORETYPE" = "domain" ] && { > @@ -76,9 +97,16 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF > XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory $XENSTORE_DOMAIN_SIZE" > [ -z "$XENSTORE_MAX_DOMAIN_SIZE" ] || XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --maxmem $XENSTORE_MAX_DOMAIN_SIZE" > > - echo -n Starting $XENSTORE_DOMAIN_KERNEL... > + if test "$initd" = 'sysv' ; then > + echo -n Starting $XENSTORE_DOMAIN_KERNEL... > + else > + echo Starting $XENSTORE_DOMAIN_KERNEL... > + fi > ${LIBEXEC_BIN}/init-xenstore-domain $XENSTORE_DOMAIN_ARGS || exit 1 > - systemd-notify --ready 2>/dev/null > + if test "$initd" = 'systemd' ; then > + systemd-notify --ready > + sleep 9 > + fi > > exit 0 > } > diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in > index 80c1d408a5..c226eb3635 100644 > --- a/tools/hotplug/Linux/systemd/xenstored.service.in > +++ b/tools/hotplug/Linux/systemd/xenstored.service.in > @@ -11,7 +11,7 @@ Type=notify > NotifyAccess=all > RemainAfterExit=true > ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities > -ExecStart=@XEN_SCRIPT_DIR@/launch-xenstore > +ExecStart=@XEN_SCRIPT_DIR@/launch-xenstore 'systemd' > > [Install] > WantedBy=multi-user.target > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.