On Mon, 10.10.11 22:33, Ankur Sinha (sanjay.an...@gmail.com) wrote:

> Hi folks, 
> 
> I've written these service files for deluge. Could someone please review
> them and tell me what corrections I need to do here?
> 
> http://ankursinha.fedorapeople.org/deluge/deluged.service

I am not a big fan of the HUP killing. If there's another, synchronous
way to reload the configuration this would be much neither to use. Note
that killing via HUP is async, and hence if you issue "systemctl reload
foo.service" the reloading might not be complete when the command
returns.

Of course many daemons only have the async way to reload the
configuration, but I encourage people to think about the problem, and
check if there is a sync way and if so make use of it.

Please drop StandardError=syslog, as we now connect stdout/stderr of all
services with syslog by default anyway. Also, if the user changes that
global setting this service should honour that and not opt out of the
default for no reason.

Unless really necessary please drop After=network.target. Applications
that just bind on 0.0.0.0 don't need to synchronize on the network. And
apps that follow netlink network changes don't have to either. It's a
good idea to make the system robust and minimize the influence external
factors (such as the dhcp server being able to delay system bootup) can
have on the system.

> http://ankursinha.fedorapeople.org/deluge/deluge-webui.service

The ">/dev/null 2>&1 &" is a shell construct and not understood by
systemd. systemd will pass that as three parameters to your binary,
which is probably not what you want.

The "&" at the end indicates that you don't want Type=fork, but
Type=simple here, which means you can drop the Type= line entirely,
since that's the default anyway.

Here too, please remove the StandardError=syslog and
After=network.target lines.

If deluge-webui tries to contact deluge it might make sense to order the
latter after the former, and maybe unconditionally pull in the former by
the latter:

After=deluge.service
Requires=deluge.service

In the [Unit] section.

Hope this helps,

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
-- 
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Reply via email to