From 924bf041bcf45a04b682533a8b467f9e9d9e4927 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 22 Nov 2015 10:11:27 +0100 Subject: [PATCH] On reload/restart we must wait for services to stop first This patch adds support for declaring *why* a service is being stopped or reloaded. This is later used when all stopped services have been collected by the `service_monitor()` to issue start or postponed SIGHUP. As an added benefit we can now also see why a service is not running. If it has been paused by a user, halted when moving to another runlevel, waiting for a condition (event), or similar. Signed-off-by: Joachim Nilsson --- README.md | 7 ++++- api.c | 11 +++++-- event.c | 3 +- inetd.c | 2 +- initctl.c | 2 +- service.c | 87 ++++++++++++++++++++++++++++++++++++++----------------- service.h | 2 +- sig.c | 1 + svc.h | 13 ++++++++- 9 files changed, 92 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 8952cdda..d94a7312 100644 --- a/README.md +++ b/README.md @@ -597,7 +597,7 @@ not return until the given command has fully completed. reload Reload *.conf in /etc/finit.d/, like SIGHUP runlevel <0-9> Change runlevel: 0 (halt), 6 (reboot) start Start a stopped service - stop Stop a running service + stop Stop/Pause a running service restart Restart (stop/start) a running service reload Reload (SIGHUP) a running service status | show List status of all services known to Finit @@ -610,6 +610,11 @@ their command counterparts. The `reload` command reloads all `*.conf` files in `/etc/finit.d/` *and* activates the changes, with `emit RELOAD` only the `*.conf` files are reloaded. +**Note:** The `emit STOP` event is more like "prepare" than "stop". + Depending on how the service is declared, `` or not, the service + may be stopped, or skipped to be be sent `SIGHUP` later when `emit + START` is issued. + To achieve the same result as `initctl reload` emit all three events: ```shell diff --git a/api.c b/api.c index 954ea224..73c931bd 100644 --- a/api.c +++ b/api.c @@ -108,8 +108,13 @@ static int call(int (*action)(svc_t *), char *buf, size_t len) return result; } +static int service_pause(svc_t *svc) +{ + return service_stop(svc, SVC_PAUSED_STATE); +} + static int do_start (char *buf, size_t len) { return call(service_start, buf, len); } -static int do_stop (char *buf, size_t len) { return call(service_stop, buf, len); } +static int do_pause (char *buf, size_t len) { return call(service_pause, buf, len); } static int do_reload (char *buf, size_t len) { return call(service_reload, buf, len); } static int do_restart(char *buf, size_t len) { return call(service_restart, buf, len); } @@ -258,7 +263,7 @@ static void cb(uev_t *w, void *UNUSED(arg), int UNUSED(events)) debug = !debug; break; - case INIT_CMD_RELOAD: + case INIT_CMD_RELOAD: /* 'init q' and 'initctl reload' */ service_reload_dynamic(); break; @@ -267,7 +272,7 @@ static void cb(uev_t *w, void *UNUSED(arg), int UNUSED(events)) break; case INIT_CMD_STOP_SVC: - result = do_stop(rq.data, sizeof(rq.data)); + result = do_pause(rq.data, sizeof(rq.data)); break; case INIT_CMD_RELOAD_SVC: diff --git a/event.c b/event.c index 6b2c526f..eb2b7084 100644 --- a/event.c +++ b/event.c @@ -220,12 +220,11 @@ void event_dispatch(char *msg) service_restart(svc); } else { /* change == -1 */ if (svc->pid && !event_service_cond(svc->events)) - service_stop(svc); + service_stop(svc, SVC_CONDHALT_STATE); } } } - /** * Local Variables: * version-control: t diff --git a/inetd.c b/inetd.c index 3720c907..a191710c 100644 --- a/inetd.c +++ b/inetd.c @@ -212,7 +212,7 @@ void inetd_stop(inetd_t *inetd) /* Stop any running service, not allowed anymore. */ if (svc->pid) - service_stop(svc); + service_stop(svc, SVC_WAITING_STATE); } static int getent(char *service, char *proto, struct servent **sv, struct protoent **pv) diff --git a/initctl.c b/initctl.c index 1e38245e..9465ca74 100644 --- a/initctl.c +++ b/initctl.c @@ -218,7 +218,7 @@ static int usage(int rc) " runlevel [0-9] Show or set runlevel: 0 halt, 6 reboot\n" " status | show Show status of services\n" " start Start stopped job or service\n" - " stop Stop running job or service\n" + " stop Stop/Pause running job or service\n" " restart Restart (stop/start) job or service\n" " reload Reload (SIGHUP) job or service\n" " version Show Finit version\n\n", __progname); diff --git a/service.c b/service.c index 2a279018..f85f6d2b 100644 --- a/service.c +++ b/service.c @@ -90,6 +90,8 @@ void service_bootstrap(void) */ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg) { + svc_cmd_t cmd = SVC_START; /* Default to start, since listed in finit.conf */ + if (!svc) { errno = EINVAL; return SVC_STOP; @@ -104,6 +106,11 @@ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg) if (runlevel && !event_service_cond(svc->events)) return SVC_STOP; + if (svc->state == SVC_RELOAD_STATE) + cmd = SVC_RELOAD; + if (svc->state == SVC_PAUSED_STATE) + cmd = SVC_STOP; + /* Is there a service plugin registered? */ if (svc->cb) { int status; @@ -125,8 +132,10 @@ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg) } /* Callback normally exits here. */ - if (WIFEXITED(status)) - return WEXITSTATUS(status); + if (WIFEXITED(status)) { + svc_cmd_t tmp = WEXITSTATUS(status); + return tmp == SVC_START ? cmd : tmp; + } /* Check for SEGFAULT or other error ... */ if (WIFSIGNALED(status) && WCOREDUMP(status)) @@ -137,8 +146,7 @@ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg) return SVC_STOP; } - /* No service plugin, default to start, since listed in finit.conf */ - return SVC_START; + return cmd; } /* Remember: service_enabled() must be called before calling service_start() */ @@ -297,6 +305,7 @@ int service_start(svc_t *svc) exit(status); } svc->pid = pid; + svc->state = SVC_RUNNING_STATE; if (svc_is_inetd(svc)) { if (svc->inetd.type == SOCK_STREAM) @@ -318,9 +327,9 @@ int service_start(svc_t *svc) return 0; } -int service_stop(svc_t *svc) +int service_stop(svc_t *svc, int state) { - int res = 1; + int res = 0; if (!svc) return 1; @@ -334,6 +343,12 @@ int service_stop(svc_t *svc) if (SVC_TYPE_SERVICE != svc->type) return 0; + _d("Service %s state %d, new state %d, sighup %d", svc->cmd, svc->state, state, svc->sighup); + if (state == SVC_RELOAD_STATE && svc->sighup) + goto exit; + if (state == SVC_RESTART_STATE && svc->sighup) + return service_reload(svc); + if (runlevel != 1 && verbose) print_desc("Stopping ", svc->desc); @@ -343,7 +358,8 @@ int service_stop(svc_t *svc) if (runlevel != 1 && verbose) print_result(res); exit: - svc->pid = 0; + if (!res) + svc->state = state; svc->restart_counter = 0; return res; @@ -381,6 +397,10 @@ static int service_stop_done(svc_t *svc) if (!dyn_stop_cnt) { _d("All disabled/removed services have been stoppped, calling reconf hooks ..."); plugin_run_hooks(HOOK_SVC_RECONF); /* Reconfigure HW/VLANs/etc here */ + + /* Finish off by starting/reload modified/new services */ + if (svc->state == SVC_RELOAD_STATE) + service_start_dynamic(); } return 1; @@ -388,6 +408,10 @@ static int service_stop_done(svc_t *svc) /** * service_stop_dynamic - Stop disabled/removed dynamic services + * + * We call it "stop", but in reality it could be "skip" as well, if the + * service supports SIGHUP. This function is just one step on the road + * to reload all modified services. */ void service_stop_dynamic(void) { @@ -397,7 +421,8 @@ void service_stop_dynamic(void) for (svc = svc_dynamic_iterator(1); svc; svc = svc_dynamic_iterator(0)) { if (svc_is_changed(svc)) { dyn_stop_cnt++; - service_stop(svc); + _d("Marking service %s as state %d", svc->cmd, SVC_RELOAD_STATE); + service_stop(svc, SVC_RELOAD_STATE); } } @@ -405,23 +430,30 @@ void service_stop_dynamic(void) service_stop_done(NULL); } -/* stop + start +/** + * service_restart - Restart or SIGHUP a process * - * XXX: This should be refactored into a background job AND marked such - * so that the service monitor doesn't attempt to restart it! + * Unfortunately we cannot call service_start() here because we must + * wait for the svc to actually be collected by the service_monitor() + * first. Currently this means that an initctl user will not get + * correct feedback when restarting a service. (Prompt may return + * before we've actually started upp the service again.) + * + * XXX: This should be refactored to let initctl check the sighup + * flag and then to stop+start or reload. */ int service_restart(svc_t *svc) { - int result = 0; - if (!svc) return 1; - result += service_stop(svc); - sleep(1); - result += service_start(svc); + /* Ignore if finit is SIGSTOP'ed */ + if (is_norespawn()) + return 0; - return result; + svc->restart_counter = 0; + + return service_stop(svc, SVC_RESTART_STATE); } /** @@ -440,16 +472,15 @@ int service_reload(svc_t *svc) if (is_norespawn()) return 0; - if (!svc) { - _e("Failed, no svc pointer."); + if (!svc) return 1; - } if (svc->pid <= 1) { _d("Bad PID %d for %s, SIGHUP", svc->pid, svc->cmd); svc->pid = 0; return 1; } + svc->state = SVC_RUNNING_STATE; _d("Sending SIGHUP to PID %d", svc->pid); return kill(svc->pid, SIGHUP); @@ -470,8 +501,12 @@ void service_reload_dynamic(void) /* Then stop any disabled/removed services and non-reloadable */ service_stop_dynamic(); - /* Finish off by starting/reload modified/new services */ - service_start_dynamic(); + /* + * Finish off by starting/reloading modified/new services. + * Postponed to service_stop_done() to make sure all services + * are guaranteed to have been stopped before being started + * again. + */ } /** @@ -508,12 +543,12 @@ void service_runlevel(int newlevel) inetd_stop(&svc->inetd); else #endif - service_stop(svc); + service_stop(svc, SVC_HALTED_STATE); } /* ... or disabled/removed services from /etc/finit.d/ */ if (svc_is_dynamic(svc) && svc_is_changed(svc)) - service_stop(svc); + service_stop(svc, SVC_HALTED_STATE); } /* Prev runlevel services stopped, call hooks before starting new runlevel ... */ @@ -787,7 +822,7 @@ int service_register(int type, char *line, time_t mtime, char *username) void service_unregister(svc_t *svc) { - service_stop(svc); + service_stop(svc, SVC_HALTED_STATE); svc_del(svc); } @@ -890,7 +925,7 @@ static void svc_dance(svc_t *svc) if (svc->pid) { if (SVC_STOP == cmd) - service_stop(svc); + service_stop(svc, SVC_HALTED_STATE); else if (SVC_RELOAD == cmd) service_reload(svc); } else { diff --git a/service.h b/service.h index 84ada061..cf684ce3 100644 --- a/service.h +++ b/service.h @@ -33,7 +33,7 @@ void service_unregister (svc_t *svc); svc_cmd_t service_enabled (svc_t *svc, int event, void *arg); int service_start (svc_t *svc); -int service_stop (svc_t *svc); +int service_stop (svc_t *svc, int state); void service_start_dynamic (void); void service_stop_dynamic (void); int service_restart (svc_t *svc); diff --git a/sig.c b/sig.c index e0b6d4e5..977ff36d 100644 --- a/sig.c +++ b/sig.c @@ -96,6 +96,7 @@ void do_shutdown(int sig) */ static void sighup_cb(uev_t *UNUSED(w), void *UNUSED(arg), int UNUSED(events)) { + /* INIT_CMD_RELOAD: 'init q', 'initctl reload', and SIGHUP */ service_reload_dynamic(); } diff --git a/svc.h b/svc.h index 066f204e..1a6ed49f 100644 --- a/svc.h +++ b/svc.h @@ -40,12 +40,22 @@ typedef enum { typedef enum { SVC_TYPE_FREE = 0, /* Free to allocate */ - SVC_TYPE_SERVICE , /* Monitored, will be respawned */ + SVC_TYPE_SERVICE, /* Monitored, will be respawned */ SVC_TYPE_TASK, /* One-shot, runs in parallell */ SVC_TYPE_RUN, /* Like task, but wait for completion */ SVC_TYPE_INETD /* Classic inetd service */ } svc_type_t; +typedef enum { + SVC_HALTED_STATE = 0, /* Not allowed in runlevel, or not enabled. */ + SVC_WAITING_STATE, /* Waiting for connection (inetd service) */ + SVC_PAUSED_STATE, /* Stopped/Paused by user started on reload */ + SVC_CONDHALT_STATE, /* Not allowed to run atm. event/state lost */ + SVC_RESTART_STATE, /* Restarting service waiting to be stopped */ + SVC_RELOAD_STATE, /* Reloading services, after .conf changed */ + SVC_RUNNING_STATE, /* Currently running service, see svc->pid */ +} svc_state_t; + #define FINIT_SHM_ID 0x494E4954 /* "INIT", see ascii(7) */ #define MAX_ARG_LEN 64 #define MAX_STR_LEN 64 @@ -63,6 +73,7 @@ typedef struct svc { /* Service details */ pid_t pid; + svc_state_t state; /* Paused, Reloading, Restart, Running, ... */ svc_type_t type; time_t mtime; /* Modification time for .conf from /etc/finit.d/ */ int dirty; /* Set if old mtime != new mtime => reloaded,