On Tue, Apr 14, 2015 at 10:59:51AM +0800, Liu Yuan wrote:
>
> Hi Alexander,
>
> Could you please give this patch a review?
>
Hey Yuan / Vasiliy,
Sorry for the slow reply.
> On Mon, Apr 13, 2015 at 01:32:56PM +0300, Vasiliy Tolstov wrote:
> > [Service]
> > +Environment="SHEEPDOG_ARGS=--cluster local --log dst=syslog -f --upgrade"
> > "SHEEPDOG_PATH=@LOCALSTATEDIR@/lib/sheepdog"
[snip]
> > -ExecStart=/bin/sh -c 'ulimit -n 32768; @SBINDIR@/sheep --pidfile
> > @LOCALSTATEDIR@/run/sheep.pid $(if [ -z "$SHEEP_OPTS" ]; then echo
> > "--cluster local --log dst=syslog --upgrade @LOCALSTATEDIR@/lib/sheepdog";
> > else echo $SHEEP_OPTS; fi)'
Removing the shell script from the ExecStart and using Environment is a good
idea.
Changing SHEEP_OPTS to SHEEPDOG_ARGS and SHEEPDOG_PATH is going to break
existing installs that use this service file. What's the functional benefit to
doing this? We already have a separate service file for Debian packages (i.e.
debian/sheepdog.service), that uses another naming scheme.
> > -PIDFile=@LOCALSTATEDIR@/run/sheep.pid
> > -Type=forking
> > +ExecStart=@SBINDIR@/sheep $SHEEPDOG_ARGS $SHEEPDOG_PATH
I don't think this is a good idea. I went and double-checked the systemd
documentation:
http://www.freedesktop.org/software/systemd/man/systemd.service.html
If set to simple (the default if neither Type= nor BusName=, but ExecStart= are
specified), it is expected that the process configured with ExecStart= is the
main process of the service. In this mode, if the process offers functionality
to other processes on the system, its communication channels should be
installed before the daemon is started up (e.g. sockets set up by systemd, via
socket activation), as systemd will immediately proceed starting follow-up
units.
... so if I'm understanding this right:
1) By removing 'Type=forking', it defaults to 'Type=simple'
2) 'Type=simple' expects to have communication setup before the daemon is
started (e.g. by systemd managing the sockets via socket activation).
#2 means that (unless we implement socket activation) we're going to have an
even bigger start-up race condition than we already have, for services that
depend on sheepdog (e.g. qemu guests that start at bootup) because systemd will
have no way of knowing when sheep is actually ready for work.
So overall, I NACK these changes.
Thanks.
Alexander
--
sheepdog mailing list
[email protected]
https://lists.wpkg.org/mailman/listinfo/sheepdog
