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

[George Dunlap <dunlapg@xxxxxxxxx>]

On Wed, Jan 7, 2015 at 9:40 AM, Olaf Hering <olaf@xxxxxxxxx> wrote:
> 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.

I can confirm that

ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS"

does not work on a CentOS7 system with selinux enabled, but that

ExecStart=/usr/bin/env $XENSTORED --no-fork $XENSTORED_ARGS

does work.

A patch is on its way.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel