[systemd-devel] [PATCH] systemd restart patch

[Zbigniew Jędrzejewski-Szmek]

On 07/02/2011 02:21 AM, Lennart Poettering wrote:
> On Fri, 01.07.11 15:35, Cristian Patrascu (Cristian.Patrascu at windriver.com) wrote:
> 
> Heya!
> 
> (BTW, got your first post of this patch too, just didn't find the time
> to review the patch, sorry. It was still in my mail queue however.)
> 
>>      After "x" restarts have happened, the "OnFailure" unit will start
>> (if specified) but only after all unsuccessfull restarts (after "x"
>> restarts reached).
> 
> Patch looks pretty good. A few comments though.
>>
>> This patch is made for systemd-29, on master branch with SHA1 ID:
>> 3661ac04b4f2840d3345605aa35963bbde3c469d
>>
>> ________systemd-restart-on-fail.patch : _______________________________
>>
>> Index: systemd-29/src/dbus-service.c
>> ===================================================================
>> --- systemd-29.orig/src/dbus-service.c	2011-06-22 10:47:14.000000000 +0300
>> +++ systemd-29/src/dbus-service.c	2011-06-22 13:48:51.292321742 +0300
>> @@ -42,6 +42,8 @@
>>          "<property name=\"PIDFile\" type=\"s\" access=\"read\"/>\n"   \
>>          "<property name=\"NotifyAccess\" type=\"s\" access=\"read\"/>\n" \
>>          "<property name=\"RestartUSec\" type=\"t\" access=\"read\"/>\n" \
>> +        "<property name=\"MaxRestartRetries\" type=\"i\" access=\"read\"/>\n" \
>> +        "<property name=\"RestartRetry\" type=\"i\"
>> access=\"read\"/>\n" \
> 
> I Think it would make sense to make both of these unsigned, in order to
> avoid confusion whether these actually ever can be negative.
> 
> If I read your patch correctly right now MaxRestartRetries=-1 means that
> there is no limit on the number of retries. I'd use 0 for that instead
> (0 is not needed to indicate disabling, since we can indicate that with
> Restart=no already.)
Hi,

OTOH, it would probably be more consistent to make MaxRestartRetries=0
equivalent to Restart=no, just to avoid ambiguity and surprise to the user.

Zbyszek