On Thu, Jul 18, 2013 at 3:27 AM, Lennart Poettering <[email protected]> wrote: > On Wed, 17.07.13 09:46, Umut Tezduyar ([email protected]) 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. _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
