[systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

[Lennart Poettering]

On Tue, 24.07.12 16:43, Lukas Nykryn (lnykryn at redhat.com) wrote:

> In some cases, like wrong configuration, restarting after error
> exit code does not help, so administrator can specify RestartIgnoreCodes
> which will not cause restart of a service.

I am not happy with the term "Code" here, it's a bit too generic to be
self explanatory.

And I think I agree with Zbigniew to a certain degree: we might want to
think about whether we should also allow configuring non-failure exit
codes or so. I mean, I am not suggesting we should implement that
right-away, but just keep in mind how that setting would look like so
that we can keep things uniform if we add it later. (In fact, I'd
recommend not to implement it right away: adding an option nobody so far
needed seems like a bad idea if we want to keep our set of options
minimal).

Also, I think we need to think further than just exit codes,
i.e. signals and stuff.

Maybe DontRestartExitStatus=? The libc calls the generalization of
exit code and exit signal the "exit status", so that sounds like the
best term to use here.

And then people could write:

   DontRestartExitStatus=SIGTERM 1 2 3 4

or something like this?

(Of course, the "don't" in the name is a negative option, which we try
to avoid, but it appears weird to have a positive option for this, so i
think it would be ok...)

One day, if we really need to make the failure/success sets configurable
too, we could then add:
    
   DontFailExitStatus=SIGSEGV 1 2

(But please, don't implement this bit just yet, let's wait for somebody
actually needing this. Note though, that Upstart actually does have
functionality like this).

> +        *ignored = new(int, k);

Maybe use a bitfield here, like we do for the syscalls list?

(Actualy two bitfields, one for the exit codes, one for the exit signals).

> +static int check_ignored_rc(Service *s){

Please don't use acronyms in symbols if it's easy to avoid them and no
strong incentive to use them.

> +        int n_restart_ignored_codes;

Generally we try to used "unsigned" rather than "int" for values where
negative values really never make sense, like in this case. It gives
readers a bit of a hint that this field never can be negative and no
special cases like that are used.

Otherwise looks good.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.