Re: [PATCH 7/7] tools/hotplug: add wrapper to start xenstored

[Olaf Hering <olaf@aepfle.de>]

On Tue, Jan 06, Ian Campbell wrote:

> On Fri, 2014-12-19 at 12:25 +0100, Olaf Hering wrote:
> > The shell wrapper in xenstored.service does not handle XENSTORE_TRACE.
> > 
> > Create a separate wrapper script which is used in the sysv runlevel
> > script and in systemd xenstored.service. It preserves existing
> > behaviour by handling the XENSTORE_TRACE boolean. It also implements
> > the handling of XENSTORED_ARGS=. This variable has to be added to
> > sysconfig/xencommons.
> 
> Why don't we just drop XENSTORED_* in favour of XENSTORED_ARGS (with an
> example in the sysconfig file of enabling tracing if you like)?

After having two weeks to think about this I came to the same
conclusion. I think whatever the outcome is, the boolean should be
removed. The sysconfig file should get a XENSTORED_ARGS="" along with a
help text which mentions "-T /path" and "xenstored --help" to get other
options because there is no man page.

> Going to a wrapper script just to make some fairly uncommon debugging
> option marginally more convenient seems like overkill to me, plus
> XENSTORED_ARGS would allow for passing other useful options to
> xenstored.

If I recall correctly the point of the current 'sh -c "exec ..."' stunt
was to expand the XENSTORE variable from the sysconfig file. But this
approach leads to failures with SELinux because the socket passing does
not work this way. Up to now I have not seen a success report for
selinux+systemd+xenstored. Maybe its already somewhere in the other
unread mails.

In my cover letter I provided some possible ways to handle
selinux+systemd+xenstored. Ideally the approach "Exec=/usr/bin/env
$XENSTORED --no-fork $XENSTORED_ARGS" works because it means its
possible to select a binary via the sysconfig file. But it also means
the XENSTORE_TRACE boolean has to be removed in favour of the plain
XENSTORED_ARGS= approach mentioned above. Finally this would avoid the
need for a wrapper script.

Hopefully someone with access to a SELinux enabled system will report
which approach actually works.

> > The wrapper uses exec unconditionally. This works because the
> > systemd service file passes --no-fork, which has the desired effect
> > that the binary launched by systemd becomes the final daemon
> > process. The sysv script does not pass --no-fork, which causes
> > xenstored to fork internally to return to the caller of the wrapper
> > script.
> > 
> > The place of the wrapper is currently LIBEXEC_BIN, it has to be
> > decided what the final location is supposed to be. IanJ wants it in
> > "/etc".
> 
> If we go this route then I agree with Ian J. (/etc/xen/scripts, I
> suppose).

I have not heard back which location has to be used. If /etc/xen/scripts
is the place, so be it. I thought this is just for hotplug scripts.


Olaf