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

[Lukáš Nykrýn]

Lennart Poettering píše v Pá 03. 08. 2012 v 20:46 +0200:
> 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
> 

Thanks for the feedback. I have altered name of the option to
RestartIgnoreExitStatus, but feel free to change to anything else and I
have also stored the codes to a set instead of an array.

Lukas