From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 5/5] tools/hotplug: support XENSTORED_TRACE in systemd Date: Fri, 12 Dec 2014 10:10:56 +0000 Message-ID: <1418379056.23309.45.camel@citrix.com> References: <1417781152-9926-1-git-send-email-olaf@aepfle.de> <1417781152-9926-6-git-send-email-olaf@aepfle.de> <21633.41977.399042.691409@mariner.uk.xensource.com> <20141208123736.GA3691@aepfle.de> <21639.7875.160312.349247@mariner.uk.xensource.com> <20141209162740.GA14288@aepfle.de> <21639.10108.666942.407514@mariner.uk.xensource.com> <20141210091534.GA24974@aepfle.de> <1418205728.19809.40.camel@citrix.com> <20141210175251.GA4441@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20141210175251.GA4441@aepfle.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Olaf Hering Cc: Ian Jackson , xen-devel@lists.xen.org, Wei Liu , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Wed, 2014-12-10 at 18:52 +0100, Olaf Hering wrote: > On Wed, Dec 10, Ian Campbell wrote: > > > That results in a wrapper which unconditionally execs, the systemd unit > > just calls that while the sysv script runs the wrapper and then does the > > xenstore-read -s loop. > > Since systemd handles the socket there is already a listener. > http://lists.freedesktop.org/archives/systemd-devel/2014-December/026157.html Not sure I understand what any of that means, but I'll assume it's good ;-) > I came up with this, works in my testing with SLE11 sysv and Factory > systemd. The beef looks like shown below. Let me know if thats good > enough to handle XENSTORED_TRACE also in systemd. I will add some > comments to the wrapper why it is there. Seems ok. I wonder if the wrapper ought to source @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons to obtain XENSTORED_* itself rather than relying on the initscript and unit file to do so. Especially in the initscript case it looks a bit ugly to have to manually propagate things. > > Olaf > > diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in > index a1095c2..f57bfd3 100644 > --- a/tools/hotplug/Linux/init.d/xencommons.in > +++ b/tools/hotplug/Linux/init.d/xencommons.in > @@ -66,11 +66,13 @@ do_start () { > then > test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@" > rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null > - test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T /var/log/xen/xenstored-trace.log" > > if [ -n "$XENSTORED" ] ; then > echo -n Starting $XENSTORED... > - $XENSTORED --pid-file /var/run/xenstored.pid $XENSTORED_ARGS > + XENSTORED=$XENSTORED \ > + XENSTORED_TRACE=$XENSTORED_TRACE \ > + XENSTORED_ARGS=$XENSTORED_ARGS \ > + ${LIBEXEC_BIN}/xenstored.sh --pid-file /var/run/xenstored.pid > else > echo "No xenstored found" > exit 1 > diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in > index 0f0ac58..d906ea2 100644 > --- a/tools/hotplug/Linux/systemd/xenstored.service.in > +++ b/tools/hotplug/Linux/systemd/xenstored.service.in > @@ -8,13 +8,12 @@ ConditionPathExists=/proc/xen/capabilities > > [Service] > Type=notify > -Environment=XENSTORED_ARGS= > Environment=XENSTORED=@XENSTORED@ > -EnvironmentFile=-@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons > +EnvironmentFile=@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons > ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities > ExecStartPre=-/bin/rm -f @XEN_LIB_STORED@/tdb* > ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@ > -ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS" > +ExecStart=-@LIBEXEC_BIN@/xenstored.sh --no-fork > > [Install] > WantedBy=multi-user.target > diff --git a/tools/hotplug/Linux/xenstored.sh.in b/tools/hotplug/Linux/xenstored.sh.in > new file mode 100644 > index 0000000..dc806ee > --- /dev/null > +++ b/tools/hotplug/Linux/xenstored.sh.in > @@ -0,0 +1,6 @@ > +#!/bin/sh > +if test -n "$XENSTORED_TRACE" > +then > + XENSTORED_ARGS=" -T /var/log/xen/xenstored-trace.log" > +fi > +exec $XENSTORED $@ $XENSTORED_ARGS > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >