1abbee
From a7ec486dede56ab2ec28132133becf11e5685884 Mon Sep 17 00:00:00 2001
1abbee
From: Michal Sekletar <msekletar@users.noreply.github.com>
1abbee
Date: Mon, 16 May 2016 17:24:51 +0200
1abbee
Subject: [PATCH] core: don't log job status message in case job was
1abbee
 effectively NOP (#3199)
1abbee
1abbee
We currently generate log message about unit being started even when
1abbee
unit was started already and job didn't do anything. This is because job
1abbee
was requested explicitly and hence became anchor job of the transaction
1abbee
thus we could not eliminate it. That is fine but, let's not pollute
1abbee
journal with useless log messages.
1abbee
1abbee
$ systemctl start systemd-resolved
1abbee
$ systemctl start systemd-resolved
1abbee
$ systemctl start systemd-resolved
1abbee
1abbee
Current state:
1abbee
$ journalctl -u systemd-resolved | grep Started
1abbee
1abbee
May 05 15:31:42 rawhide systemd[1]: Started Network Name Resolution.
1abbee
May 05 15:31:59 rawhide systemd[1]: Started Network Name Resolution.
1abbee
May 05 15:32:01 rawhide systemd[1]: Started Network Name Resolution.
1abbee
1abbee
After patch applied:
1abbee
$ journalctl -u systemd-resolved | grep Started
1abbee
1abbee
May 05 16:42:12 rawhide systemd[1]: Started Network Name Resolution.
1abbee
1abbee
Fixes #1723
1abbee
1abbee
Cherry-picked from: 833f92ad39beca0e954e91e5764ffc83f8d0c1cf
1abbee
Resolves: #1280014
1abbee
---
1abbee
 src/core/dbus-job.c    |  2 +-
1abbee
 src/core/job.c         | 33 ++++++++++++++++++---------------
1abbee
 src/core/job.h         |  2 +-
1abbee
 src/core/manager.c     |  2 +-
1abbee
 src/core/transaction.c |  2 +-
1abbee
 src/core/unit.c        | 12 ++++++------
1abbee
 6 files changed, 28 insertions(+), 25 deletions(-)
1abbee
1abbee
diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
Pablo Greco 48fc63
index 8b5ea2566d..7ce5d57726 100644
1abbee
--- a/src/core/dbus-job.c
1abbee
+++ b/src/core/dbus-job.c
1abbee
@@ -84,7 +84,7 @@ int bus_job_method_cancel(sd_bus *bus, sd_bus_message *message, void *userdata,
1abbee
         if (r < 0)
1abbee
                 return r;
1abbee
 
1abbee
-        job_finish_and_invalidate(j, JOB_CANCELED, true);
1abbee
+        job_finish_and_invalidate(j, JOB_CANCELED, true, false);
1abbee
 
1abbee
         return sd_bus_reply_method_return(message, NULL);
1abbee
 }
1abbee
diff --git a/src/core/job.c b/src/core/job.c
Pablo Greco 48fc63
index 7416386a18..c2876dec18 100644
1abbee
--- a/src/core/job.c
1abbee
+++ b/src/core/job.c
1abbee
@@ -190,7 +190,7 @@ Job* job_install(Job *j) {
1abbee
 
1abbee
         if (uj) {
1abbee
                 if (job_type_is_conflicting(uj->type, j->type))
1abbee
-                        job_finish_and_invalidate(uj, JOB_CANCELED, false);
1abbee
+                        job_finish_and_invalidate(uj, JOB_CANCELED, false, false);
1abbee
                 else {
1abbee
                         /* not conflicting, i.e. mergeable */
1abbee
 
1abbee
@@ -571,19 +571,19 @@ int job_run_and_invalidate(Job *j) {
1abbee
         j = manager_get_job(m, id);
1abbee
         if (j) {
1abbee
                 if (r == -EALREADY)
1abbee
-                        r = job_finish_and_invalidate(j, JOB_DONE, true);
1abbee
+                        r = job_finish_and_invalidate(j, JOB_DONE, true, true);
1abbee
                 else if (r == -EBADR)
1abbee
-                        r = job_finish_and_invalidate(j, JOB_SKIPPED, true);
1abbee
+                        r = job_finish_and_invalidate(j, JOB_SKIPPED, true, false);
1abbee
                 else if (r == -ENOEXEC)
1abbee
-                        r = job_finish_and_invalidate(j, JOB_INVALID, true);
1abbee
+                        r = job_finish_and_invalidate(j, JOB_INVALID, true, false);
1abbee
                 else if (r == -EPROTO)
1abbee
-                        r = job_finish_and_invalidate(j, JOB_ASSERT, true);
1abbee
+                        r = job_finish_and_invalidate(j, JOB_ASSERT, true, false);
1abbee
                 else if (r == -ENOTSUP)
1abbee
-                        r = job_finish_and_invalidate(j, JOB_UNSUPPORTED, true);
1abbee
+                        r = job_finish_and_invalidate(j, JOB_UNSUPPORTED, true, false);
1abbee
                 else if (r == -EAGAIN)
1abbee
                         job_set_state(j, JOB_WAITING);
1abbee
                 else if (r < 0)
1abbee
-                        r = job_finish_and_invalidate(j, JOB_FAILED, true);
1abbee
+                        r = job_finish_and_invalidate(j, JOB_FAILED, true, false);
1abbee
         }
1abbee
 
1abbee
         return r;
1abbee
@@ -792,7 +792,7 @@ static void job_log_status_message(Unit *u, JobType t, JobResult result) {
1abbee
                                 NULL);
1abbee
 }
1abbee
 
1abbee
-int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
1abbee
+int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool already) {
1abbee
         Unit *u;
1abbee
         Unit *other;
1abbee
         JobType t;
1abbee
@@ -810,8 +810,11 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
1abbee
         log_unit_debug(u->id, "Job %s/%s finished, result=%s",
1abbee
                        u->id, job_type_to_string(t), job_result_to_string(result));
1abbee
 
1abbee
-        job_print_status_message(u, t, result);
1abbee
-        job_log_status_message(u, t, result);
1abbee
+        /* If this job did nothing to respective unit we don't log the status message */
1abbee
+        if (!already) {
1abbee
+                job_print_status_message(u, t, result);
1abbee
+                job_log_status_message(u, t, result);
1abbee
+        }
1abbee
 
1abbee
         job_add_to_dbus_queue(j);
1abbee
 
1abbee
@@ -842,20 +845,20 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
1abbee
                                 if (other->job &&
1abbee
                                     (other->job->type == JOB_START ||
1abbee
                                      other->job->type == JOB_VERIFY_ACTIVE))
1abbee
-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
1abbee
+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);
1abbee
 
1abbee
                         SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i)
1abbee
                                 if (other->job &&
1abbee
                                     (other->job->type == JOB_START ||
1abbee
                                      other->job->type == JOB_VERIFY_ACTIVE))
1abbee
-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
1abbee
+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);
1abbee
 
1abbee
                         SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i)
1abbee
                                 if (other->job &&
1abbee
                                     !other->job->override &&
1abbee
                                     (other->job->type == JOB_START ||
1abbee
                                      other->job->type == JOB_VERIFY_ACTIVE))
1abbee
-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
1abbee
+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);
1abbee
 
1abbee
                 } else if (t == JOB_STOP) {
1abbee
 
1abbee
@@ -863,7 +866,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
1abbee
                                 if (other->job &&
1abbee
                                     (other->job->type == JOB_START ||
1abbee
                                      other->job->type == JOB_VERIFY_ACTIVE))
1abbee
-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);
1abbee
+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);
1abbee
                 }
1abbee
         }
1abbee
 
1abbee
@@ -911,7 +914,7 @@ static int job_dispatch_timer(sd_event_source *s, uint64_t monotonic, void *user
1abbee
         log_unit_warning(j->unit->id, "Job %s/%s timed out.", j->unit->id, job_type_to_string(j->type));
1abbee
 
1abbee
         u = j->unit;
1abbee
-        job_finish_and_invalidate(j, JOB_TIMEOUT, true);
1abbee
+        job_finish_and_invalidate(j, JOB_TIMEOUT, true, false);
1abbee
 
1abbee
         failure_action(u->manager, u->job_timeout_action, u->job_timeout_reboot_arg);
1abbee
 
1abbee
diff --git a/src/core/job.h b/src/core/job.h
Pablo Greco 48fc63
index d967b68a3f..e4191ee775 100644
1abbee
--- a/src/core/job.h
1abbee
+++ b/src/core/job.h
1abbee
@@ -220,7 +220,7 @@ void job_add_to_dbus_queue(Job *j);
1abbee
 int job_start_timer(Job *j);
1abbee
 
1abbee
 int job_run_and_invalidate(Job *j);
1abbee
-int job_finish_and_invalidate(Job *j, JobResult result, bool recursive);
1abbee
+int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool already);
1abbee
 
1abbee
 char *job_dbus_path(Job *j);
1abbee
 
1abbee
diff --git a/src/core/manager.c b/src/core/manager.c
Pablo Greco 48fc63
index a1504bf17b..ee456fb790 100644
1abbee
--- a/src/core/manager.c
1abbee
+++ b/src/core/manager.c
1abbee
@@ -1426,7 +1426,7 @@ void manager_clear_jobs(Manager *m) {
1abbee
 
1abbee
         while ((j = hashmap_first(m->jobs)))
1abbee
                 /* No need to recurse. We're cancelling all jobs. */
1abbee
-                job_finish_and_invalidate(j, JOB_CANCELED, false);
1abbee
+                job_finish_and_invalidate(j, JOB_CANCELED, false, false);
1abbee
 }
1abbee
 
1abbee
 static int manager_dispatch_run_queue(sd_event_source *source, void *userdata) {
1abbee
diff --git a/src/core/transaction.c b/src/core/transaction.c
Pablo Greco 48fc63
index b0b3d6bd60..aed64fa419 100644
1abbee
--- a/src/core/transaction.c
1abbee
+++ b/src/core/transaction.c
1abbee
@@ -592,7 +592,7 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) {
1abbee
                         /* Not invalidating recursively. Avoids triggering
1abbee
                          * OnFailure= actions of dependent jobs. Also avoids
1abbee
                          * invalidating our iterator. */
1abbee
-                        job_finish_and_invalidate(j, JOB_CANCELED, false);
1abbee
+                        job_finish_and_invalidate(j, JOB_CANCELED, false, false);
1abbee
                 }
1abbee
         }
1abbee
 
1abbee
diff --git a/src/core/unit.c b/src/core/unit.c
Pablo Greco 48fc63
index db5aa987ec..d6ead7d672 100644
1abbee
--- a/src/core/unit.c
1abbee
+++ b/src/core/unit.c
1abbee
@@ -1851,12 +1851,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
1abbee
                 case JOB_VERIFY_ACTIVE:
1abbee
 
1abbee
                         if (UNIT_IS_ACTIVE_OR_RELOADING(ns))
1abbee
-                                job_finish_and_invalidate(u->job, JOB_DONE, true);
1abbee
+                                job_finish_and_invalidate(u->job, JOB_DONE, true, false);
1abbee
                         else if (u->job->state == JOB_RUNNING && ns != UNIT_ACTIVATING) {
1abbee
                                 unexpected = true;
1abbee
 
1abbee
                                 if (UNIT_IS_INACTIVE_OR_FAILED(ns))
1abbee
-                                        job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true);
1abbee
+                                        job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true, false);
1abbee
                         }
1abbee
 
1abbee
                         break;
1abbee
@@ -1866,12 +1866,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
1abbee
 
1abbee
                         if (u->job->state == JOB_RUNNING) {
1abbee
                                 if (ns == UNIT_ACTIVE)
1abbee
-                                        job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true);
1abbee
+                                        job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true, false);
1abbee
                                 else if (ns != UNIT_ACTIVATING && ns != UNIT_RELOADING) {
1abbee
                                         unexpected = true;
1abbee
 
1abbee
                                         if (UNIT_IS_INACTIVE_OR_FAILED(ns))
1abbee
-                                                job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true);
1abbee
+                                                job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true, false);
1abbee
                                 }
1abbee
                         }
1abbee
 
1abbee
@@ -1882,10 +1882,10 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
1abbee
                 case JOB_TRY_RESTART:
1abbee
 
1abbee
                         if (UNIT_IS_INACTIVE_OR_FAILED(ns))
1abbee
-                                job_finish_and_invalidate(u->job, JOB_DONE, true);
1abbee
+                                job_finish_and_invalidate(u->job, JOB_DONE, true, false);
1abbee
                         else if (u->job->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) {
1abbee
                                 unexpected = true;
1abbee
-                                job_finish_and_invalidate(u->job, JOB_FAILED, true);
1abbee
+                                job_finish_and_invalidate(u->job, JOB_FAILED, true, false);
1abbee
                         }
1abbee
 
1abbee
                         break;