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

[Umut Tezduyar]

On Thu, Jul 18, 2013 at 3:27 AM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Wed, 17.07.13 09:46, 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)
>> +                return;
>> +
>> +        if (se->state == SERVICE_FAILED && se->socket_fd < 0)
>> +                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) &&
>> +            se->socket_fd < 0)
>> +                socket_notify_service_dead(s, false);
>> +
>> +        if (!s->accept && se->state == SERVICE_RUNNING)
>> +                socket_set_state(s, SOCKET_RUNNING);
>> +}
>
> Hmm, hmm, so, the "se->socket_fd < 0" check will be true for all services
> where the socket has !s->accept set. (after all, s->accept=true indicates
> that we should accept the sockets, and then pass the connection socket
> to the service, and that is stored in se->socket_fd, since it becomes
> private property of that service. That is different for s->accept=false
> where the socket continues to be owned by the socket unit).
>
> Hence, since all three if blocks effectivel just check for !s->accept,
> could you split the check out and replace it with an early
>
> if (s->accept)
>         return;
>
> Immediately after the initial load/service type check?

Corrected in that way.

>
> Otherwise the patch looks great! Have you tested that everything works
> with it?

:) I tried different ways but I think it would be great if someone can
test it a bit more. Especially for the reason that I work on systemd
204 not 205. I have tried killing, stopping services that are socket
activated or not.
I have stumbled on assert error due to socket's coldplug function is
expecting socket to be in dead state. Next version of patch will
reflect this in a way that it will not change the socket state if
socket is in dead state.

>
> Thanks for working on this!

Absolutely.

>
> Lennart
>
> --
> Lennart Poettering - Red Hat, Inc.