[systemd-devel] [PATCHv3] core: notify triggered by socket of a service

[Umut Tezduyar]

On Thu, Jul 18, 2013 at 8:11 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Thu, 18.07.13 15:27, Umut Tezduyar (umut at tezduyar.com) wrote:
>
>> +static void socket_trigger_notify(Unit *u, Unit *other) {
>> +        Socket *s = SOCKET(u);
>> +        Service *se = SERVICE(other);
>> +
>> +        assert(u);
>> +        assert(other);
>> +
>> +        if (other->load_state != UNIT_LOADED ||
>> +            other->type != UNIT_SERVICE ||
>> +            s->accept)
>> +                return;
>> +
>> +        if (se->state == SERVICE_FAILED)
>> +                socket_notify_service_dead(s, se->result == SERVICE_FAILURE_START_LIMIT);
>> +
>> +        if (se->state == SERVICE_DEAD ||
>> +            se->state == SERVICE_STOP ||
>> +            se->state == SERVICE_STOP_SIGTERM ||
>> +            se->state == SERVICE_STOP_SIGKILL ||
>> +            se->state == SERVICE_STOP_POST ||
>> +            se->state == SERVICE_FINAL_SIGTERM ||
>> +            se->state == SERVICE_FINAL_SIGKILL ||
>> +            se->state == SERVICE_AUTO_RESTART)
>> +                socket_notify_service_dead(s, false);
>> +
>> +        if (s->state != SOCKET_DEAD &&
>> +            se->state == SERVICE_RUNNING)
>> +                socket_set_state(s, SOCKET_RUNNING);
>> +}
>
> Hmm, so I am pretty sure that the most recent change you did should
> actually apply to all three if blocks... i.e. we shouldn't propagate the
> state changes from the service to the socket if the socket is already
> dead or failed. Or in fact, even stronger: we shouldn't propagate the
> state unless the socket is actually fully up. Hence, it appears to me
> that there should be an early return condition at the head
> of the function that does:
>
>         /* Don't propagate state changes from the service if we are
>            already down. */
>         if (s->state !=  SOCKET_RUNNING && s->state != SOCKET_LISTENING)
>                 return;
>
> Or something like that? Could you rework the patch and give it a bit of
> testing? If things work for you I'll then merge it, and we can fix
> everything that might still pop up in git.
>
> Lennart
>
> --
> Lennart Poettering - Red Hat, Inc.

Hi. I finally got a chance to test it on 205. I have tested "systemctl
daemon-reload", "systemctl daemon-reexec", "stopping journal service",
"disabling journal service", "killing journal process", "stopping ssh
socket", "killing ssh service", "stopping per connection ssh service",
"replacing journal service with bin/false to simulate start failure",
etc
I hope nothing is broken.
Thanks,
Umut