daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
aed857
From 7bc07eb6c9a31f2c26d0fe3e6d7a26a13cbb2369 Mon Sep 17 00:00:00 2001
aed857
From: Michal Schmidt <mschmidt@redhat.com>
aed857
Date: Thu, 16 Jul 2015 20:08:30 +0200
aed857
Subject: [PATCH] core: fix confusing logging of instantaneous jobs
aed857
aed857
For instantaneous jobs (e.g. starting of targets, sockets, slices, or
aed857
Type=simple services) the log shows the job completion
aed857
before starting:
aed857
aed857
        systemd[1]: Created slice -.slice.
aed857
        systemd[1]: Starting -.slice.
aed857
        systemd[1]: Created slice System Slice.
aed857
        systemd[1]: Starting System Slice.
aed857
        systemd[1]: Listening on Journal Audit Socket.
aed857
        systemd[1]: Starting Journal Audit Socket.
aed857
        systemd[1]: Reached target Timers.
aed857
        systemd[1]: Starting Timers.
aed857
        ...
aed857
aed857
The reason is that the job completes before the ->start() method returns
aed857
and only then does unit_start() print the "Starting ..." message.
aed857
The same thing happens when stopping units.
aed857
aed857
Rather than fixing the order of the messages, let's just not emit the
aed857
Starting/Stopping message at all when the job completes instantaneously.
aed857
The job completion message is sufficient in this case.
aed857
aed857
(cherry picked from commit d1a34ae9c20f1c02aab17884919eccef572b1d21)
aed857
aed857
Resolves: #1506256
aed857
---
23b3cf
 src/core/job.c  | 65 ++++++++++++++++++++++++++++++++++---------------
23b3cf
 src/core/unit.c | 36 +++++++++------------------
aed857
 src/core/unit.h |  1 +
aed857
 3 files changed, 58 insertions(+), 44 deletions(-)
aed857
aed857
diff --git a/src/core/job.c b/src/core/job.c
aed857
index c9a43a4cb..612caa604 100644
aed857
--- a/src/core/job.c
aed857
+++ b/src/core/job.c
aed857
@@ -504,10 +504,48 @@ static void job_change_type(Job *j, JobType newtype) {
aed857
         j->type = newtype;
aed857
 }
aed857
 
aed857
+static int job_perform_on_unit(Job **j) {
aed857
+        /* While we execute this operation the job might go away (for
aed857
+         * example: because it finishes immediately or is replaced by a new,
aed857
+         * conflicting job.) To make sure we don't access a freed job later on
aed857
+         * we store the id here, so that we can verify the job is still
aed857
+         * valid. */
aed857
+        Manager *m  = (*j)->manager;
aed857
+        Unit *u     = (*j)->unit;
aed857
+        JobType t   = (*j)->type;
aed857
+        uint32_t id = (*j)->id;
aed857
+        int r;
aed857
+
aed857
+        switch (t) {
aed857
+                case JOB_START:
aed857
+                        r = unit_start(u);
aed857
+                        break;
aed857
+
aed857
+                case JOB_RESTART:
aed857
+                        t = JOB_STOP;
aed857
+                case JOB_STOP:
aed857
+                        r = unit_stop(u);
aed857
+                        break;
aed857
+
aed857
+                case JOB_RELOAD:
aed857
+                        r = unit_reload(u);
aed857
+                        break;
aed857
+
aed857
+                default:
aed857
+                        assert_not_reached("Invalid job type");
aed857
+        }
aed857
+
aed857
+        /* Log if the job still exists and the start/stop/reload function
aed857
+         * actually did something. */
aed857
+        *j = manager_get_job(m, id);
aed857
+        if (*j && r > 0)
aed857
+                unit_status_emit_starting_stopping_reloading(u, t);
aed857
+
aed857
+        return r;
aed857
+}
aed857
+
aed857
 int job_run_and_invalidate(Job *j) {
aed857
         int r;
aed857
-        uint32_t id;
aed857
-        Manager *m = j->manager;
aed857
 
aed857
         assert(j);
aed857
         assert(j->installed);
aed857
@@ -526,23 +564,9 @@ int job_run_and_invalidate(Job *j) {
aed857
         job_set_state(j, JOB_RUNNING);
aed857
         job_add_to_dbus_queue(j);
aed857
 
aed857
-        /* While we execute this operation the job might go away (for
aed857
-         * example: because it is replaced by a new, conflicting
aed857
-         * job.) To make sure we don't access a freed job later on we
aed857
-         * store the id here, so that we can verify the job is still
aed857
-         * valid. */
aed857
-        id = j->id;
aed857
 
aed857
         switch (j->type) {
aed857
 
aed857
-                case JOB_START:
aed857
-                        r = unit_start(j->unit);
aed857
-
aed857
-                        /* If this unit cannot be started, then simply wait */
aed857
-                        if (r == -EBADR)
aed857
-                                r = 0;
aed857
-                        break;
aed857
-
aed857
                 case JOB_VERIFY_ACTIVE: {
aed857
                         UnitActiveState t = unit_active_state(j->unit);
aed857
                         if (UNIT_IS_ACTIVE_OR_RELOADING(t))
aed857
@@ -554,17 +578,19 @@ int job_run_and_invalidate(Job *j) {
aed857
                         break;
aed857
                 }
aed857
 
aed857
+                case JOB_START:
aed857
                 case JOB_STOP:
aed857
                 case JOB_RESTART:
aed857
-                        r = unit_stop(j->unit);
aed857
+                        r = job_perform_on_unit(&j);
aed857
 
aed857
-                        /* If this unit cannot stopped, then simply wait. */
aed857
+                        /* If the unit type does not support starting/stopping,
aed857
+                         * then simply wait. */
aed857
                         if (r == -EBADR)
aed857
                                 r = 0;
aed857
                         break;
aed857
 
aed857
                 case JOB_RELOAD:
aed857
-                        r = unit_reload(j->unit);
aed857
+                        r = job_perform_on_unit(&j);
aed857
                         break;
aed857
 
aed857
                 case JOB_NOP:
aed857
@@ -575,7 +601,6 @@ int job_run_and_invalidate(Job *j) {
aed857
                         assert_not_reached("Unknown job type");
aed857
         }
aed857
 
aed857
-        j = manager_get_job(m, id);
aed857
         if (j) {
aed857
                 if (r == -EALREADY)
aed857
                         r = job_finish_and_invalidate(j, JOB_DONE, true, true);
aed857
diff --git a/src/core/unit.c b/src/core/unit.c
aed857
index 6d535ae12..907a4bf7f 100644
aed857
--- a/src/core/unit.c
aed857
+++ b/src/core/unit.c
aed857
@@ -1417,6 +1417,15 @@ static void unit_status_log_starting_stopping_reloading(Unit *u, JobType t) {
aed857
                         NULL);
aed857
 }
aed857
 
aed857
+void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t) {
aed857
+
aed857
+        unit_status_log_starting_stopping_reloading(u, t);
aed857
+
aed857
+        /* Reload status messages have traditionally not been printed to console. */
aed857
+        if (t != JOB_RELOAD)
aed857
+                unit_status_print_starting_stopping(u, t);
aed857
+}
aed857
+
aed857
 /* Errors:
aed857
  *         -EBADR:     This unit type does not support starting.
aed857
  *         -EALREADY:  Unit is already started.
aed857
@@ -1427,7 +1436,6 @@ static void unit_status_log_starting_stopping_reloading(Unit *u, JobType t) {
aed857
 int unit_start(Unit *u) {
aed857
         UnitActiveState state;
aed857
         Unit *following;
aed857
-        int r;
aed857
 
aed857
         assert(u);
aed857
 
aed857
@@ -1481,14 +1489,7 @@ int unit_start(Unit *u) {
aed857
 
aed857
         unit_add_to_dbus_queue(u);
aed857
 
aed857
-        r = UNIT_VTABLE(u)->start(u);
aed857
-        if (r <= 0)
aed857
-                return r;
aed857
-
aed857
-        /* Log if the start function actually did something */
aed857
-        unit_status_log_starting_stopping_reloading(u, JOB_START);
aed857
-        unit_status_print_starting_stopping(u, JOB_START);
aed857
-        return r;
aed857
+        return UNIT_VTABLE(u)->start(u);
aed857
 }
aed857
 
aed857
 bool unit_can_start(Unit *u) {
aed857
@@ -1512,7 +1513,6 @@ bool unit_can_isolate(Unit *u) {
aed857
 int unit_stop(Unit *u) {
aed857
         UnitActiveState state;
aed857
         Unit *following;
aed857
-        int r;
aed857
 
aed857
         assert(u);
aed857
 
aed857
@@ -1531,13 +1531,7 @@ int unit_stop(Unit *u) {
aed857
 
aed857
         unit_add_to_dbus_queue(u);
aed857
 
aed857
-        r = UNIT_VTABLE(u)->stop(u);
aed857
-        if (r <= 0)
aed857
-                return r;
aed857
-
aed857
-        unit_status_log_starting_stopping_reloading(u, JOB_STOP);
aed857
-        unit_status_print_starting_stopping(u, JOB_STOP);
aed857
-        return r;
aed857
+        return UNIT_VTABLE(u)->stop(u);
aed857
 }
aed857
 
aed857
 /* Errors:
aed857
@@ -1548,7 +1542,6 @@ int unit_stop(Unit *u) {
aed857
 int unit_reload(Unit *u) {
aed857
         UnitActiveState state;
aed857
         Unit *following;
aed857
-        int r;
aed857
 
aed857
         assert(u);
aed857
 
aed857
@@ -1575,12 +1568,7 @@ int unit_reload(Unit *u) {
aed857
 
aed857
         unit_add_to_dbus_queue(u);
aed857
 
aed857
-        r = UNIT_VTABLE(u)->reload(u);
aed857
-        if (r <= 0)
aed857
-                return r;
aed857
-
aed857
-        unit_status_log_starting_stopping_reloading(u, JOB_RELOAD);
aed857
-        return r;
aed857
+        return UNIT_VTABLE(u)->reload(u);
aed857
 }
aed857
 
aed857
 bool unit_can_reload(Unit *u) {
aed857
diff --git a/src/core/unit.h b/src/core/unit.h
aed857
index 85f52df18..480e2e95f 100644
aed857
--- a/src/core/unit.h
aed857
+++ b/src/core/unit.h
aed857
@@ -562,6 +562,7 @@ int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency d);
aed857
 int unit_coldplug(Unit *u, Hashmap *deferred_work);
aed857
 
aed857
 void unit_status_printf(Unit *u, const char *status, const char *unit_status_msg_format) _printf_(3, 0);
aed857
+void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t);
aed857
 
aed857
 bool unit_need_daemon_reload(Unit *u);
aed857