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