From patchwork Wed May 15 15:06:32 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olaf Hering X-Patchwork-Id: 10944901 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5E0ED1398 for ; Wed, 15 May 2019 15:09:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4756528A13 for ; Wed, 15 May 2019 15:09:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3AF2A28AC4; Wed, 15 May 2019 15:09:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.0 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 972A028AB7 for ; Wed, 15 May 2019 15:09:21 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hQvUW-0004Wm-NU; Wed, 15 May 2019 15:06:48 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hQvUT-0004Wh-PB for xen-devel@lists.xenproject.org; Wed, 15 May 2019 15:06:46 +0000 X-Inumbo-ID: 0c3a383b-7723-11e9-8980-bc764e045a96 Received: from mo6-p00-ob.smtp.rzone.de (unknown [2a01:238:20a:202:5300::5]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTPS id 0c3a383b-7723-11e9-8980-bc764e045a96; Wed, 15 May 2019 15:06:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1557932802; s=strato-dkim-0002; d=aepfle.de; h=Message-Id:Date:Subject:Cc:To:From:X-RZG-CLASS-ID:X-RZG-AUTH:From: Subject:Sender; bh=EI2sv9uL8hwDaaKTspU7HU5tpMumzFYKjCTIIbhN4ws=; b=VeS8TYTQmMTNnroYs6wz7C3NcIyPU7eEpBUC8v62HbOILBGciU1mnWPaR9U4t7nNK9 pQ0A++82Dmj9HVUtfL9EAKgY1Dzko7LOpHRzab1ESr4tawsuCih7vYUcZ87ptlGJ7F7r Xa/v7uNrcatog5z0q3qfFIKuEWHl4iGLBLI/NJ0hI9UuMcZSJyS/23Kwf5NhUyFao9gk SsKTraa2+IPrnMzGm0rMX9Xq/o/qpl/Up2lUOG+fGoAMu4A3IDrvt2QRO9XL+qvYn0YF iW6VDBnAfd/Ilpt6rR30frt5lp4WaoxlnU5jr5bcTxv6KN7QXtSXRZQKFU1bDK+6iEPU FkMQ== X-RZG-AUTH: ":P2EQZWCpfu+qG7CngxMFH1J+3q8wa/QXkBR9MXjAuzBW/OdlBZQ4AHSS3GpFjw==" X-RZG-CLASS-ID: mo00 Received: from sender by smtp.strato.de (RZmta 44.20 DYNA|AUTH) with ESMTPSA id U080cav4FF6aBIk (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Wed, 15 May 2019 17:06:36 +0200 (CEST) From: Olaf Hering To: xen-devel@lists.xenproject.org Date: Wed, 15 May 2019 17:06:32 +0200 Message-Id: <20190515150632.16269-1-olaf@aepfle.de> X-Mailer: git-send-email 2.16.4 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v1] hotplug/Linux: fix starting of xenstored with systemd X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Wei Liu , Olaf Hering , Ian Jackson Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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 . # +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