From f8f2338aba84394e132fff55b6ebdef7d884292b Mon Sep 17 00:00:00 2001 From: José Bollo Date: Tue, 19 May 2020 11:22:26 +0200 Subject: Improve use of systemd's states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A better handling of systemd state is need to treat correctly transient states. That change includes: - Management of states with numeric identifiers instead of names - Handling of the state "inactive" as a stable state. Most of previous seen problems were coming from that miss. - Returning no error but also no info on the process if it falled to "inactive" meaning that it stopped quickly. Bug-AGL: SPEC-3323 Change-Id: Ibf35eb6257c5583596d675cad0bec2869f5fd5f7 Signed-off-by: José Bollo --- src/afm-binding.c | 9 ++++--- src/afm-urun.c | 57 +++++++++++++++++++++++++++--------------- src/utils-systemd.c | 71 +++++++++++++++++++++++++++++++++-------------------- src/utils-systemd.h | 19 ++++++++------ 4 files changed, 99 insertions(+), 57 deletions(-) diff --git a/src/afm-binding.c b/src/afm-binding.c index 55010a2..1771520 100644 --- a/src/afm-binding.c +++ b/src/afm-binding.c @@ -402,7 +402,7 @@ static void start(afb_req_t req) /* launch the application */ runid = afm_urun_start(appli, afb_req_get_uid(req)); - if (runid <= 0) { + if (runid < 0) { cant_start(req); return; } @@ -412,7 +412,8 @@ static void start(afb_req_t req) #if 0 wrap_json_pack(&resp, "{si}", _runid_, runid); #else - wrap_json_pack(&resp, "i", runid); + if (runid) + wrap_json_pack(&resp, "i", runid); #endif afb_req_success(req, resp, NULL); } @@ -439,13 +440,13 @@ static void once(afb_req_t req) /* launch the application */ runid = afm_urun_once(appli, afb_req_get_uid(req)); - if (runid <= 0) { + if (runid < 0) { cant_start(req); return; } /* returns the state */ - resp = afm_urun_state(afudb, runid, afb_req_get_uid(req)); + resp = runid ? afm_urun_state(afudb, runid, afb_req_get_uid(req)) : NULL; afb_req_success(req, resp, NULL); } diff --git a/src/afm-urun.c b/src/afm-urun.c index 7f8ad16..051ce7e 100644 --- a/src/afm-urun.c +++ b/src/afm-urun.c @@ -26,6 +26,7 @@ #include #include #include +#include #include @@ -144,18 +145,29 @@ error: return -1; } -static const char *wait_state_stable(int isuser, const char *dpath) +static enum SysD_State wait_state_stable(int isuser, const char *dpath) { - int trial, count; - const char *state = NULL; - - count = 10; - for (trial = 1 ; trial <= count ; trial++) { + int trial; + enum SysD_State state = SysD_State_INVALID; + struct timespec tispec; + const int period_ms = 10; + const int trial_s = 10; + const int trial_count = (trial_s * 1000) / period_ms; + const int period_ns = period_ms * 1000000; + + for (trial = 1 ; trial <= trial_count ; trial++) { state = systemd_unit_state_of_dpath(isuser, dpath); - if (state == NULL || state == SysD_State_Active - || state == SysD_State_Failed) + switch (state) { + case SysD_State_Active: + case SysD_State_Failed: + case SysD_State_Inactive: return state; - sleep(1); + default: + tispec.tv_sec = 0; + tispec.tv_nsec = period_ns; + nanosleep(&tispec, NULL); + break; + } } return state; } @@ -169,7 +181,7 @@ static const char *wait_state_stable(int isuser, const char *dpath) * * Returns the created object or NULL in case of error. */ -static json_object *mkstate(const char *id, int runid, int pid, const char *state) +static json_object *mkstate(const char *id, int runid, int pid, enum SysD_State state) { struct json_object *result, *pids; @@ -239,7 +251,8 @@ int afm_urun_start(struct json_object *appli, int uid) */ int afm_urun_once(struct json_object *appli, int uid) { - const char *udpath, *state, *uscope, *uname; + const char *udpath, *uscope, *uname; + enum SysD_State state; int rc, isuser; /* retrieve basis */ @@ -257,24 +270,28 @@ int afm_urun_once(struct json_object *appli, int uid) } state = wait_state_stable(isuser, udpath); - if (state == NULL) { + switch (state) { + case SysD_State_Active: + case SysD_State_Inactive: + break; + case SysD_State_Failed: j_read_string_at(appli, "unit-scope", &uscope); j_read_string_at(appli, "unit-name", &uname); - ERROR("can't wait %s unit %s for uid %d: %m", uscope, uname, uid); + ERROR("start error %s unit %s for uid %d: %s", uscope, uname, uid, + systemd_state_name(state)); goto error; - } - if (state != SysD_State_Active) { + default: j_read_string_at(appli, "unit-scope", &uscope); j_read_string_at(appli, "unit-name", &uname); - ERROR("start error %s unit %s for uid %d: %s", uscope, uname, uid, state); + ERROR("can't wait %s unit %s for uid %d: %m", uscope, uname, uid); goto error; } rc = systemd_unit_pid_of_dpath(isuser, udpath); - if (rc <= 0) { + if (rc < 0) { j_read_string_at(appli, "unit-scope", &uscope); j_read_string_at(appli, "unit-name", &uname); - ERROR("can't getpid of %s unit %s for uid %d: %m", uscope, uname, uid); + ERROR("can't get pid of %s unit %s for uid %d: %m", uscope, uname, uid); goto error; } @@ -334,7 +351,7 @@ struct json_object *afm_urun_list(struct afm_udb *db, int all, int uid) int i, n, isuser, pid; const char *udpath; const char *id; - const char *state; + enum SysD_State state; struct json_object *desc; struct json_object *appli; struct json_object *apps; @@ -380,7 +397,7 @@ struct json_object *afm_urun_state(struct afm_udb *db, int runid, int uid) char *dpath; const char *udpath; const char *id; - const char *state; + enum SysD_State state; struct json_object *appli; struct json_object *apps; struct json_object *result; diff --git a/src/utils-systemd.c b/src/utils-systemd.c index cfa7a6a..0c22810 100644 --- a/src/utils-systemd.c +++ b/src/utils-systemd.c @@ -69,12 +69,15 @@ static const char sdbm_load_unit[] = "LoadUnit"; static const char sdbp_active_state[] = "ActiveState"; static const char sdbp_exec_main_pid[] = "ExecMainPID"; -const char SysD_State_Inactive[] = "inactive"; -const char SysD_State_Activating[] = "activating"; -const char SysD_State_Active[] = "active"; -const char SysD_State_Deactivating[] = "deactivating"; -const char SysD_State_Reloading[] = "reloading"; -const char SysD_State_Failed[] = "failed"; +static const char *sds_state_names[] = { + NULL, + "inactive", + "activating", + "active", + "deactivating", + "reloading", + "failed" +}; static struct sd_bus *sysbus; static struct sd_bus *usrbus; @@ -257,34 +260,46 @@ static int unit_pid(struct sd_bus *bus, const char *dpath) return rc < 0 ? rc : (int)u; } -static const char *unit_state(struct sd_bus *bus, const char *dpath) +static enum SysD_State unit_state(struct sd_bus *bus, const char *dpath) { int rc; char *st; - const char *resu; + enum SysD_State resu; sd_bus_error err = SD_BUS_ERROR_NULL; + resu = SysD_State_INVALID; rc = sd_bus_get_property_string(bus, sdb_destination, dpath, sdbi_unit, sdbp_active_state, &err, &st); if (rc < 0) { errno = -rc; - resu = NULL; } else { - if (!strcmp(st, SysD_State_Active)) - resu = SysD_State_Active; - else if (!strcmp(st, SysD_State_Reloading)) - resu = SysD_State_Reloading; - else if (!strcmp(st, SysD_State_Inactive)) - resu = SysD_State_Inactive; - else if (!strcmp(st, SysD_State_Failed)) - resu = SysD_State_Failed; - else if (!strcmp(st, SysD_State_Activating)) - resu = SysD_State_Activating; - else if (!strcmp(st, SysD_State_Deactivating)) - resu = SysD_State_Deactivating; - else { - errno = EBADMSG; - resu = NULL; + switch (st[0]) { + case 'a': + if (!strcmp(st, sds_state_names[SysD_State_Active])) + resu = SysD_State_Active; + else if (!strcmp(st, sds_state_names[SysD_State_Activating])) + resu = SysD_State_Activating; + break; + case 'd': + if (!strcmp(st, sds_state_names[SysD_State_Deactivating])) + resu = SysD_State_Deactivating; + break; + case 'f': + if (!strcmp(st, sds_state_names[SysD_State_Failed])) + resu = SysD_State_Failed; + break; + case 'i': + if (!strcmp(st, sds_state_names[SysD_State_Inactive])) + resu = SysD_State_Inactive; + break; + case 'r': + if (!strcmp(st, sds_state_names[SysD_State_Reloading])) + resu = SysD_State_Reloading; + break; + default: + break; } + if (resu == NULL) + errno = EBADMSG; free(st); } return resu; @@ -591,12 +606,16 @@ int systemd_unit_pid_of_dpath(int isuser, const char *dpath) return rc < 0 ? rc : unit_pid(bus, dpath); } -const char *systemd_unit_state_of_dpath(int isuser, const char *dpath) +enum SysD_State systemd_unit_state_of_dpath(int isuser, const char *dpath) { int rc; struct sd_bus *bus; rc = systemd_get_bus(isuser, &bus); - return rc < 0 ? NULL : unit_state(bus, dpath); + return rc < 0 ? SysD_State_INVALID : unit_state(bus, dpath); } +const char *systemd_state_name(enum SysD_State state) +{ + return sds_state_names[state]; +} diff --git a/src/utils-systemd.h b/src/utils-systemd.h index cf95097..3bddfd6 100644 --- a/src/utils-systemd.h +++ b/src/utils-systemd.h @@ -18,12 +18,15 @@ #pragma once -extern const char SysD_State_Inactive[]; -extern const char SysD_State_Activating[]; -extern const char SysD_State_Active[]; -extern const char SysD_State_Deactivating[]; -extern const char SysD_State_Reloading[]; -extern const char SysD_State_Failed[]; +enum SysD_State { + SysD_State_INVALID, + SysD_State_Inactive, + SysD_State_Activating, + SysD_State_Active, + SysD_State_Deactivating, + SysD_State_Reloading, + SysD_State_Failed +}; struct sd_bus; extern int systemd_get_bus(int isuser, struct sd_bus **ret); @@ -48,8 +51,10 @@ extern int systemd_unit_stop_name(int isuser, const char *name); extern int systemd_unit_stop_pid(int isuser, unsigned pid); extern int systemd_unit_pid_of_dpath(int isuser, const char *dpath); -extern const char *systemd_unit_state_of_dpath(int isuser, const char *dpath); +extern enum SysD_State systemd_unit_state_of_dpath(int isuser, const char *dpath); extern int systemd_unit_list(int isuser, int (*callback)(void *closure, const char *name, const char *path, int isuser), void *closure); extern int systemd_unit_list_all(int (*callback)(void *closure, const char *name, const char *path, int isuser), void *closure); +extern const char *systemd_state_name(enum SysD_State state); + -- cgit 1.2.3-korg