Re: [PATCH 5/5] tools/hotplug: support XENSTORED_TRACE in systemd

[Ian Campbell <Ian.Campbell@citrix.com>]

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
>