Why can't ExecStopPre= be used to abort stopping a (broken) service?

["Jóhann B. Guðmundsson"]

On 07/15/2013 08:55 PM, Lennart Poettering wrote:
> On Mon, 15.07.13 14:18, Paul Wouters (pwouters at redhat.com) wrote:
>
>> Hi,
>>
>> For daemons, it happens that people (or puppet/ansible) makes a config
>> change that causes the config file to not load and be invalid. When
>> restarting the service, it will stop but not start. Ideally, the stop
>> should be aborted.
>>
>> I was looking at ExecStopPre= (which is mentioned in the systemd.service
>> man page, although it does not have its own section, so is easilly
>> missed) but it did not abort the stop on a parse error in the daemon's
>> config file.
>>
>> I found this note by Lennart: http://osdir.com/ml/fedora-devel-list/2011-05/msg00696.html
>>
>> 	No, ExecStopPre= cannot be used for making shutdown of a service
>> 	conditional. Even if one of the pre lines fails we will go on with
>> 	shutting down the service, however we will store the exit code and the
>> 	service will be in "failed" state once fully shut down.
>>
>> My question is, why not? Various daemons (libreswan, bind, knot, nsd,
>> etc) have a "check config" option that could be used to prevent stopping
>> a service if the config file got messed up so it would prevent the
>> service from starting.
>>
>> I realise that it is not optimal to keep a service running that will fail to
>> restart on the next machine boot, but is that preferable over failing
>> immediately? If ExecStopPre= would fail and log a message, sysadmins
>> would be able to notice the issue and fix it. And there would be 0
>> downtime. As opposed to the current behaviour, which kills the service.
>>
>> However, if ExecStopPre= would support this, then every maintainer could
>> choose for themselves which situation is preferred.
> If I grok correctly what you are asking for, you are actually looing for
> an ExecRestartPre=, not an ExecStopPre=. You want somthing that is run
> before we stop a service when we intend to restart it. But when we
> shutdown the system and stop the service for that, or if the user wants
> to stop it manually, we shouldn't run it, correct?
>
> If that's what you want, then yes, it is on the TODO list to add
> something like this, but ExecStopPre= is not what you want, it would
> have very different semantics.
>

Adding that does not make much sense.

For the first in 99% use cases are covered via ExecStartPre= based on 
how this was handle in the legacy sysv init scripts.

Secondly the fact is that the admin would never notice the configuration 
error because he

Admin makes incorrect change to config file -->  then restart the 
service --> the service would not fail since it would be caught by 
either ExecStopPre= or ExecRestartPre= ( which I dont see the need for ) 
--> he then checks if the service is running ( which it will be since it 
never got restarted or stopped/started like most of the legacy sysvinit 
script did ) --> he then scratches his head why the changes he made were 
not applied.

The fact is the only way admin will notices and react to this, is if the 
service will hard fail upon restart and systemd should not be working 
around admin mistakes in configuration file.

The correct way here is to stay true to what is tried and tested in the 
legacy sysv init scripts and add the config parser to the ExecStartPre= 
line in the unit file.

JBG