core: remember first unit failure, not last unit failure

Previously, the result value of a unit was overriden with each failure that
took place, so that the result always reported the last failure that took
place.

With this commit this is changed, so that the first failure taking place is
stored instead. This should normally not matter much as multiple failures are
sufficiently uncommon. However, it improves one behaviour: if we send SIGABRT
to a service due to a watchdog timeout, then this currently would be reported
as "coredump" failure, rather than the "watchodg" failure it really is. Hence,
in order to report information about the type of the failure, and not about
the effect of it, let's change this from all unit type to store the first, not
the last failure.

This addresses the issue pointed out here:

https://github.com/systemd/systemd/pull/3818#discussion_r73433520

diff --git a/src/core/automount.c b/src/core/automount.c
index 20a73c76f94ab755c3ae89fcd480bef82091ced7..00295cf7691f21cfae43483376feda18778a66f6 100644 (file)
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -301,7 +301,7 @@ static void automount_dump(Unit *u, FILE *f, const char *prefix) {
 static void automount_enter_dead(Automount *a, AutomountResult f) {
         assert(a);
 
-        if (f != AUTOMOUNT_SUCCESS)
+        if (a->result == AUTOMOUNT_SUCCESS)
                 a->result = f;
 
         automount_set_state(a, a->result != AUTOMOUNT_SUCCESS ? AUTOMOUNT_FAILED : AUTOMOUNT_DEAD);

diff --git a/src/core/busname.c b/src/core/busname.c
index 730be2ee141647da091e45e24702c9a1ae20c468..7952cd31aad1c948a3d0fb2cef16627c912d4171 100644 (file)
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -442,7 +442,7 @@ fail:
 static void busname_enter_dead(BusName *n, BusNameResult f) {
         assert(n);
 
-        if (f != BUSNAME_SUCCESS)
+        if (n->result == BUSNAME_SUCCESS)
                 n->result = f;
 
         busname_set_state(n, n->result != BUSNAME_SUCCESS ? BUSNAME_FAILED : BUSNAME_DEAD);
@@ -454,7 +454,7 @@ static void busname_enter_signal(BusName *n, BusNameState state, BusNameResult f
 
         assert(n);
 
-        if (f != BUSNAME_SUCCESS)
+        if (n->result == BUSNAME_SUCCESS)
                 n->result = f;
 
         kill_context_init(&kill_context);
@@ -882,7 +882,7 @@ static void busname_sigchld_event(Unit *u, pid_t pid, int code, int status) {
         log_unit_full(u, f == BUSNAME_SUCCESS ? LOG_DEBUG : LOG_NOTICE, 0,
                       "Control process exited, code=%s status=%i", sigchld_code_to_string(code), status);
 
-        if (f != BUSNAME_SUCCESS)
+        if (n->result == BUSNAME_SUCCESS)
                 n->result = f;
 
         switch (n->state) {

diff --git a/src/core/mount.c b/src/core/mount.c
index 3f6ac7fcf9aa8f6c3d8f355406a2e067ea24f93e..f3ccf6d48a91171428e7d976796beba14cd97e3c 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -759,7 +759,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) {
 static void mount_enter_dead(Mount *m, MountResult f) {
         assert(m);
 
-        if (f != MOUNT_SUCCESS)
+        if (m->result == MOUNT_SUCCESS)
                 m->result = f;
 
         mount_set_state(m, m->result != MOUNT_SUCCESS ? MOUNT_FAILED : MOUNT_DEAD);
@@ -775,7 +775,7 @@ static void mount_enter_dead(Mount *m, MountResult f) {
 static void mount_enter_mounted(Mount *m, MountResult f) {
         assert(m);
 
-        if (f != MOUNT_SUCCESS)
+        if (m->result == MOUNT_SUCCESS)
                 m->result = f;
 
         mount_set_state(m, MOUNT_MOUNTED);
@@ -786,7 +786,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) {
 
         assert(m);
 
-        if (f != MOUNT_SUCCESS)
+        if (m->result == MOUNT_SUCCESS)
                 m->result = f;
 
         r = unit_kill_context(
@@ -1158,7 +1158,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) {
         else
                 assert_not_reached("Unknown code");
 
-        if (f != MOUNT_SUCCESS)
+        if (m->result == MOUNT_SUCCESS)
                 m->result = f;
 
         if (m->control_command) {

diff --git a/src/core/path.c b/src/core/path.c
index 0dd0d375d8a6294b464eeb5d79ddd96a54781758..10f9b06974d5dcd59f7d3faeb46a8fdc881d43e8 100644 (file)
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -454,7 +454,7 @@ static int path_coldplug(Unit *u) {
 static void path_enter_dead(Path *p, PathResult f) {
         assert(p);
 
-        if (f != PATH_SUCCESS)
+        if (p->result == PATH_SUCCESS)
                 p->result = f;
 
         path_set_state(p, p->result != PATH_SUCCESS ? PATH_FAILED : PATH_DEAD);

diff --git a/src/core/scope.c b/src/core/scope.c
index b45e238974abab5ab378d35caffa6015c8e0666f..b278aed3d661675e4f95b9f947295e84a8dac511 100644 (file)
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -221,7 +221,7 @@ static void scope_dump(Unit *u, FILE *f, const char *prefix) {
 static void scope_enter_dead(Scope *s, ScopeResult f) {
         assert(s);
 
-        if (f != SCOPE_SUCCESS)
+        if (s->result == SCOPE_SUCCESS)
                 s->result = f;
 
         scope_set_state(s, s->result != SCOPE_SUCCESS ? SCOPE_FAILED : SCOPE_DEAD);
@@ -233,7 +233,7 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) {
 
         assert(s);
 
-        if (f != SCOPE_SUCCESS)
+        if (s->result == SCOPE_SUCCESS)
                 s->result = f;
 
         unit_watch_all_pids(UNIT(s));

diff --git a/src/core/service.c b/src/core/service.c
index 0cbea522769a6f823c090ee0d5485b7dd37537bf..a6793b813b99ec7e8d4d75cfb4f49f5c4f4b688c 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1423,7 +1423,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
         int r;
         assert(s);
 
-        if (f != SERVICE_SUCCESS)
+        if (s->result == SERVICE_SUCCESS)
                 s->result = f;
 
         service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED : SERVICE_DEAD);
@@ -1472,7 +1472,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) {
         int r;
         assert(s);
 
-        if (f != SERVICE_SUCCESS)
+        if (s->result == SERVICE_SUCCESS)
                 s->result = f;
 
         service_unwatch_control_pid(s);
@@ -1525,7 +1525,7 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f
 
         assert(s);
 
-        if (f != SERVICE_SUCCESS)
+        if (s->result == SERVICE_SUCCESS)
                 s->result = f;
 
         unit_watch_all_pids(UNIT(s));
@@ -1583,7 +1583,7 @@ static void service_enter_stop(Service *s, ServiceResult f) {
 
         assert(s);
 
-        if (f != SERVICE_SUCCESS)
+        if (s->result == SERVICE_SUCCESS)
                 s->result = f;
 
         service_unwatch_control_pid(s);
@@ -1635,7 +1635,7 @@ static bool service_good(Service *s) {
 static void service_enter_running(Service *s, ServiceResult f) {
         assert(s);
 
-        if (f != SERVICE_SUCCESS)
+        if (s->result == SERVICE_SUCCESS)
                 s->result = f;
 
         service_unwatch_control_pid(s);
@@ -2609,7 +2609,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                            "EXIT_STATUS=%i", status,
                            NULL);
 
-                if (f != SERVICE_SUCCESS)
+                if (s->result == SERVICE_SUCCESS)
                         s->result = f;
 
                 if (s->main_command &&
@@ -2690,7 +2690,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                               "Control process exited, code=%s status=%i",
                               sigchld_code_to_string(code), status);
 
-                if (f != SERVICE_SUCCESS)
+                if (s->result == SERVICE_SUCCESS)
                         s->result = f;
 
                 /* Immediately get rid of the cgroup, so that the

diff --git a/src/core/socket.c b/src/core/socket.c
index 82363e21576a6442589c71a355c1410ac1d25f7d..c919e89b7df41b28d0d2759197c9490e434de3c8 100644 (file)
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1810,7 +1810,7 @@ fail:
 static void socket_enter_dead(Socket *s, SocketResult f) {
         assert(s);
 
-        if (f != SOCKET_SUCCESS)
+        if (s->result == SOCKET_SUCCESS)
                 s->result = f;
 
         socket_set_state(s, s->result != SOCKET_SUCCESS ? SOCKET_FAILED : SOCKET_DEAD);
@@ -1829,7 +1829,7 @@ static void socket_enter_stop_post(Socket *s, SocketResult f) {
         int r;
         assert(s);
 
-        if (f != SOCKET_SUCCESS)
+        if (s->result == SOCKET_SUCCESS)
                 s->result = f;
 
         socket_unwatch_control_pid(s);
@@ -1857,7 +1857,7 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) {
 
         assert(s);
 
-        if (f != SOCKET_SUCCESS)
+        if (s->result == SOCKET_SUCCESS)
                 s->result = f;
 
         r = unit_kill_context(
@@ -1901,7 +1901,7 @@ static void socket_enter_stop_pre(Socket *s, SocketResult f) {
         int r;
         assert(s);
 
-        if (f != SOCKET_SUCCESS)
+        if (s->result == SOCKET_SUCCESS)
                 s->result = f;
 
         socket_unwatch_control_pid(s);
@@ -2822,7 +2822,7 @@ static void socket_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                       "Control process exited, code=%s status=%i",
                       sigchld_code_to_string(code), status);
 
-        if (f != SOCKET_SUCCESS)
+        if (s->result == SOCKET_SUCCESS)
                 s->result = f;
 
         if (s->control_command &&

diff --git a/src/core/swap.c b/src/core/swap.c
index 0ba4c4d881bc2b8dd166b17b2f471abc3a78b346..2c802da3b5357af0da55165ab7e99fff54744018 100644 (file)
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -673,7 +673,7 @@ fail:
 static void swap_enter_dead(Swap *s, SwapResult f) {
         assert(s);
 
-        if (f != SWAP_SUCCESS)
+        if (s->result == SWAP_SUCCESS)
                 s->result = f;
 
         swap_set_state(s, s->result != SWAP_SUCCESS ? SWAP_FAILED : SWAP_DEAD);
@@ -689,7 +689,7 @@ static void swap_enter_dead(Swap *s, SwapResult f) {
 static void swap_enter_active(Swap *s, SwapResult f) {
         assert(s);
 
-        if (f != SWAP_SUCCESS)
+        if (s->result == SWAP_SUCCESS)
                 s->result = f;
 
         swap_set_state(s, SWAP_ACTIVE);
@@ -700,7 +700,7 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) {
 
         assert(s);
 
-        if (f != SWAP_SUCCESS)
+        if (s->result == SWAP_SUCCESS)
                 s->result = f;
 
         r = unit_kill_context(
@@ -997,7 +997,7 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) {
         else
                 assert_not_reached("Unknown code");
 
-        if (f != SWAP_SUCCESS)
+        if (s->result == SWAP_SUCCESS)
                 s->result = f;
 
         if (s->control_command) {

diff --git a/src/core/timer.c b/src/core/timer.c
index 3206296f09d3aa11faebaf73cd8940d85d84e2aa..e2b43f02f8b12a0d709f52668e902a7c7cb8e3f9 100644 (file)
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -291,7 +291,7 @@ static int timer_coldplug(Unit *u) {
 static void timer_enter_dead(Timer *t, TimerResult f) {
         assert(t);
 
-        if (f != TIMER_SUCCESS)
+        if (t->result == TIMER_SUCCESS)
                 t->result = f;
 
         timer_set_state(t, t->result != TIMER_SUCCESS ? TIMER_FAILED : TIMER_DEAD);