From 84adec4006d5a5888fabcd3a96a0160562919e54 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 30 Oct 2023 01:05:25 +0100 Subject: [PATCH] Fix #382: do not clear service conditions if only paused Introduced back in v4.3-rc2, 82cc10be8, the support for automatic service conditions have had a weird and unintended behavior. Any change in state (see doc/svc-machine.png) caused Finit to clear out *all* previously acquired service conditions. However, when moving between RUNNING and PAUSED states, a service should not have its conditions cleared. The PAUSED state, seen also by all conditions moving to FLUX, is only temporary while an `initctl reload` is processed. If a service has no changes to be applied it will move back to RUNNING. Also, we cannot clear the service conditions because other run/task or services may depend on it and clearing them would cause Finit to SIGTERM these processes (since they are no longer eligible to run). This patch not only adds this pre-condition to `cond_clearn()`, it also clarifies which state (before or after) the particular code is interested in. Signed-off-by: Joachim Wiberg --- src/service.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/service.c b/src/service.c index f118dac5..b455c5a6 100644 --- a/src/service.c +++ b/src/service.c @@ -2203,6 +2203,7 @@ static void service_retry(svc_t *svc) static void svc_set_state(svc_t *svc, svc_state_t new_state) { svc_state_t *state = (svc_state_t *)&svc->state; + const svc_state_t old_state = svc->state; /* if PID isn't collected within SVC_TERM_TIMEOUT msec, kill it! */ if (new_state == SVC_STOPPING_STATE) { @@ -2223,7 +2224,7 @@ static void svc_set_state(svc_t *svc, svc_state_t new_state) snprintf(failure, sizeof(failure), "%s/%s/failure", svc_typestr(svc), svc_ident(svc, NULL, 0)); /* create success/failure condition when entering SVC_DONE_STATE. */ - if (*state == SVC_DONE_STATE) { + if (new_state == SVC_DONE_STATE) { if (svc->started && !WEXITSTATUS(svc->status)) cond_set_oneshot(success); else @@ -2231,7 +2232,7 @@ static void svc_set_state(svc_t *svc, svc_state_t new_state) } /* clear all conditions when entering SVC_HALTED_STATE. */ - if (*state == SVC_HALTED_STATE) { + if (new_state == SVC_HALTED_STATE) { cond_clear(success); cond_clear(failure); } @@ -2241,9 +2242,14 @@ static void svc_set_state(svc_t *svc, svc_state_t new_state) char cond[MAX_COND_LEN]; snprintf(cond, sizeof(cond), "service/%s/", svc_ident(svc, NULL, 0)); - cond_clear(cond); - switch (svc->state) { + if ((old_state == SVC_RUNNING_STATE && new_state == SVC_PAUSED_STATE) || + (old_state == SVC_PAUSED_STATE && new_state == SVC_RUNNING_STATE)) + ; /* only paused during reload, don't clear conds. */ + else + cond_clear(cond); + + switch (new_state) { case SVC_HALTED_STATE: case SVC_RUNNING_STATE: strlcat(cond, svc_status(svc), sizeof(cond));