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

[Lennart Poettering]

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?

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

Thanks for working on this!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.