Improve use of systemd's states

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 <jose.bollo@iot.bzh>

diff --git a/src/afm-binding.c b/src/afm-binding.c
index 55010a2..1771520 100644 (file)
--- 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 (file)
--- a/src/afm-urun.c
+++ b/src/afm-urun.c
@@ -26,6 +26,7 @@
 #include <stdio.h>
 #include <limits.h>
 #include <string.h>
+#include <time.h>
 
 #include <json-c/json.h>
 
@@ -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 (file)
--- 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 (file)
--- 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);
+