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>

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 <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
--- 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);
+