Xen project Mailing List

[Xen-devel] [PATCH v1] hotplug/Linux: fix starting of xenstored with systemd

From: Olaf Hering <olaf@xxxxxxxxx>

Date: Wed, 15 May 2019 17:06:32 +0200

Cc: Wei Liu <wei.liu2@xxxxxxxxxx>, Olaf Hering <olaf@xxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Delivery-date: Wed, 15 May 2019 15:06:55 +0000

List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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. Currently the decision which variant of xenstore should be used is controlled via /etc/sysconfig/xencommons:XENSTORETYPE=[domain|daemon]. This change preserves this functionality for the sysv and systemd. One way to fix it is to handle the domain case as Type=oneshot because there is nothing to monitor for systemd. The daemon case has to be handled as Type=simple, which is the default if no Type= is specified. A single unit can have just one type, so a new unit for the daemon case has to be created to preserve existing setups during upgrading of Xen. This new xenstored-daemon.service is started on demand by the existing xenstored.service. Since it is a separate unit, systemd will supervise the xenstored daemon in dom0. launch-xenstore expects now two arguments, the type of the init system, and the optional xenstore type. The systemd-notify calls are removed because systemd does not expect any notification with the updated .service files. In case the init system is systemd the daemon or init-xenstore-domain is started via exec to make sure systemd monitors the process it has just launched. Various things specific to either sysv or systemd are now handled separately. The start of xenstored-daemon.service in launch-xenstore will essentially call this helper twice. A separate internal state is introduced to deal with this. This is required to handle the xencommons case explained above. A followup change will remove the code which calls to sd_notify, they are not needed after the separation of Type=oneshot for domain and Type=simple for daemon. Signed-off-by: Olaf Hering <olaf@xxxxxxxxx> --- Tested with systemd on SLE15. sysv case untested because support for SLE11 was dropped a while ago. tools/configure | 3 +- tools/configure.ac | 1 + tools/hotplug/Linux/init.d/xencommons.in | 2 +- tools/hotplug/Linux/launch-xenstore.in | 66 +++++++++++++++++----- .../Linux/systemd/xenstored-daemon.service.in | 17 ++++++ tools/hotplug/Linux/systemd/xenstored.service.in | 5 +- 6 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 tools/hotplug/Linux/systemd/xenstored-daemon.service.in diff --git a/tools/configure b/tools/configure index 0be0be75de..2612c11490 100755 --- a/tools/configure +++ b/tools/configure @@ -9761,7 +9761,7 @@ fi if test "x$systemd" = "xy"; then : - ac_config_files="$ac_config_files hotplug/Linux/systemd/proc-xen.mount hotplug/Linux/systemd/var-lib-xenstored.mount hotplug/Linux/systemd/xen-init-dom0.service hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service hotplug/Linux/systemd/xen-watchdog.service hotplug/Linux/systemd/xenconsoled.service hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.service" + ac_config_files="$ac_config_files hotplug/Linux/systemd/proc-xen.mount hotplug/Linux/systemd/var-lib-xenstored.mount hotplug/Linux/systemd/xen-init-dom0.service hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service hotplug/Linux/systemd/xen-watchdog.service hotplug/Linux/systemd/xenconsoled.service hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.service hotplug/Linux/systemd/xenstored-daemon.service" fi @@ -10516,6 +10516,7 @@ do "hotplug/Linux/systemd/xendomains.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xendomains.service" ;; "hotplug/Linux/systemd/xendriverdomain.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xendriverdomain.service" ;; "hotplug/Linux/systemd/xenstored.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xenstored.service" ;; + "hotplug/Linux/systemd/xenstored-daemon.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xenstored-daemon.service" ;; *) as_fn_error $? "invalid argument: \`$ac_config_target'" "$LINENO" 5;; esac diff --git a/tools/configure.ac b/tools/configure.ac index fcf282e74e..c61a327d73 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -481,6 +481,7 @@ AS_IF([test "x$systemd" = "xy"], [ hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.service + hotplug/Linux/systemd/xenstored-daemon.service ]) ]) diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in index 7fd6903b98..e5a91350a2 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' "${XENSTORETYPE}" || 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..e97e339481 100644 --- a/tools/hotplug/Linux/launch-xenstore.in +++ b/tools/hotplug/Linux/launch-xenstore.in @@ -15,6 +15,26 @@ # License along with this library; If not, see <http://www.gnu.org/licenses/>. # +initd=$1 +xenstore_type=$2 +maybe_exec= + +case "$initd" in + sysv) ;; + systemd) maybe_exec='exec' ;; + *) + echo "first argument must be 'sysv' or 'systemd'" + exit 1 + ;; +esac +case "$xenstore_type" in + ""|daemon|domain|supervised-by-systemd) ;; + *) + echo "second argument must be one of 'daemon', 'domain', 'supervised-by-systemd', or empty" + exit 1 + ;; +esac + XENSTORED=@XENSTORED@ . @XEN_SCRIPT_DIR@/hotplugpath.sh @@ -44,15 +64,7 @@ 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 - -/bin/mkdir -p @XEN_RUN_DIR@ - -[ "$XENSTORETYPE" = "daemon" ] && { +run_xenstored () { [ -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@ @@ -61,15 +73,40 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF exit 1 } - echo -n Starting $XENSTORED... - $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS + ${maybe_exec} $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 + +[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon +[ "$xenstore_type" = "" ] && xenstore_type="$XENSTORETYPE" + +/bin/mkdir -p @XEN_RUN_DIR@ + +[ "$xenstore_type" = "supervised-by-systemd" ] && { + XENSTORED_ARGS="$XENSTORED_ARGS -N" + run_xenstored + exit 1 +} + +[ "$xenstore_type" = "daemon" ] && { + + if test "$initd" = 'sysv' ; then + echo -n Starting $XENSTORED... + run_xenstored + timeout_xenstore $XENSTORED || exit 1 + else + systemctl start xenstored-daemon.service + fi exit 0 } -[ "$XENSTORETYPE" = "domain" ] && { +[ "$xenstore_type" = "domain" ] && { [ -z "$XENSTORE_DOMAIN_KERNEL" ] && XENSTORE_DOMAIN_KERNEL=@LIBEXEC@/boot/xenstore-stubdom.gz XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --kernel $XENSTORE_DOMAIN_KERNEL" [ -z "$XENSTORE_DOMAIN_SIZE" ] && XENSTORE_DOMAIN_SIZE=8 @@ -77,8 +114,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF [ -z "$XENSTORE_MAX_DOMAIN_SIZE" ] || XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --maxmem $XENSTORE_MAX_DOMAIN_SIZE" echo -n Starting $XENSTORE_DOMAIN_KERNEL... - ${LIBEXEC_BIN}/init-xenstore-domain $XENSTORE_DOMAIN_ARGS || exit 1 - systemd-notify --ready 2>/dev/null + ${maybe_exec} ${LIBEXEC_BIN}/init-xenstore-domain $XENSTORE_DOMAIN_ARGS || exit 1 exit 0 } diff --git a/tools/hotplug/Linux/systemd/xenstored-daemon.service.in b/tools/hotplug/Linux/systemd/xenstored-daemon.service.in new file mode 100644 index 0000000000..5158df304b --- /dev/null +++ b/tools/hotplug/Linux/systemd/xenstored-daemon.service.in @@ -0,0 +1,17 @@ +[Unit] +Description=The xenstore daemon in dom0 +Requires=proc-xen.mount var-lib-xenstored.mount +After=proc-xen.mount var-lib-xenstored.mount +Before=libvirtd.service libvirt-guests.service +RefuseManualStop=true +ConditionPathExists=/proc/xen/capabilities + +[Service] +PIDFile=@XEN_RUN_DIR@/xenstored.pid +ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities +ExecStart=@XEN_SCRIPT_DIR@/launch-xenstore 'systemd' 'supervised-by-systemd' + +[Install] +WantedBy=multi-user.target +Also=proc-xen.mount +Also=var-lib-xenstored.mount diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in index 80c1d408a5..268e33399b 100644 --- a/tools/hotplug/Linux/systemd/xenstored.service.in +++ b/tools/hotplug/Linux/systemd/xenstored.service.in @@ -7,11 +7,10 @@ RefuseManualStop=true ConditionPathExists=/proc/xen/capabilities [Service] -Type=notify -NotifyAccess=all +Type=oneshot 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@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.