From a7ec486dede56ab2ec28132133becf11e5685884 Mon Sep 17 00:00:00 2001

From: Michal Sekletar <msekletar@users.noreply.github.com>

Date: Mon, 16 May 2016 17:24:51 +0200

Subject: [PATCH] core: don't log job status message in case job was

 effectively NOP (#3199)

We currently generate log message about unit being started even when

unit was started already and job didn't do anything. This is because job

was requested explicitly and hence became anchor job of the transaction

thus we could not eliminate it. That is fine but, let's not pollute

journal with useless log messages.

$ systemctl start systemd-resolved

$ systemctl start systemd-resolved

$ systemctl start systemd-resolved

Current state:

$ journalctl -u systemd-resolved | grep Started

May 05 15:31:42 rawhide systemd[1]: Started Network Name Resolution.

May 05 15:31:59 rawhide systemd[1]: Started Network Name Resolution.

May 05 15:32:01 rawhide systemd[1]: Started Network Name Resolution.

After patch applied:

$ journalctl -u systemd-resolved | grep Started

May 05 16:42:12 rawhide systemd[1]: Started Network Name Resolution.

Fixes #1723

Cherry-picked from: 833f92ad39beca0e954e91e5764ffc83f8d0c1cf

Resolves: #1280014

---

 src/core/dbus-job.c    |  2 +-

 src/core/job.c         | 33 ++++++++++++++++++---------------

 src/core/job.h         |  2 +-

 src/core/manager.c     |  2 +-

 src/core/transaction.c |  2 +-

 src/core/unit.c        | 12 ++++++------

 6 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c

index 8b5ea25..7ce5d57 100644

--- a/src/core/dbus-job.c

+++ b/src/core/dbus-job.c

@@ -84,7 +84,7 @@ int bus_job_method_cancel(sd_bus *bus, sd_bus_message *message, void *userdata,

         if (r < 0)

                 return r;

-        job_finish_and_invalidate(j, JOB_CANCELED, true);

+        job_finish_and_invalidate(j, JOB_CANCELED, true, false);

         return sd_bus_reply_method_return(message, NULL);

 }

diff --git a/src/core/job.c b/src/core/job.c

index 7416386..c2876de 100644

--- a/src/core/job.c

+++ b/src/core/job.c

@@ -190,7 +190,7 @@ Job* job_install(Job *j) {

         if (uj) {

                 if (job_type_is_conflicting(uj->type, j->type))

-                        job_finish_and_invalidate(uj, JOB_CANCELED, false);

+                        job_finish_and_invalidate(uj, JOB_CANCELED, false, false);

                 else {

                         /* not conflicting, i.e. mergeable */

@@ -571,19 +571,19 @@ int job_run_and_invalidate(Job *j) {

         j = manager_get_job(m, id);

         if (j) {

                 if (r == -EALREADY)

-                        r = job_finish_and_invalidate(j, JOB_DONE, true);

+                        r = job_finish_and_invalidate(j, JOB_DONE, true, true);

                 else if (r == -EBADR)

-                        r = job_finish_and_invalidate(j, JOB_SKIPPED, true);

+                        r = job_finish_and_invalidate(j, JOB_SKIPPED, true, false);

                 else if (r == -ENOEXEC)

-                        r = job_finish_and_invalidate(j, JOB_INVALID, true);

+                        r = job_finish_and_invalidate(j, JOB_INVALID, true, false);

                 else if (r == -EPROTO)

-                        r = job_finish_and_invalidate(j, JOB_ASSERT, true);

+                        r = job_finish_and_invalidate(j, JOB_ASSERT, true, false);

                 else if (r == -ENOTSUP)

-                        r = job_finish_and_invalidate(j, JOB_UNSUPPORTED, true);

+                        r = job_finish_and_invalidate(j, JOB_UNSUPPORTED, true, false);

                 else if (r == -EAGAIN)

                         job_set_state(j, JOB_WAITING);

                 else if (r < 0)

-                        r = job_finish_and_invalidate(j, JOB_FAILED, true);

+                        r = job_finish_and_invalidate(j, JOB_FAILED, true, false);

         }

         return r;

@@ -792,7 +792,7 @@ static void job_log_status_message(Unit *u, JobType t, JobResult result) {

                                 NULL);

 }

-int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {

+int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool already) {

         Unit *u;

         Unit *other;

         JobType t;

@@ -810,8 +810,11 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {

         log_unit_debug(u->id, "Job %s/%s finished, result=%s",

                        u->id, job_type_to_string(t), job_result_to_string(result));

-        job_print_status_message(u, t, result);

-        job_log_status_message(u, t, result);

+        /* If this job did nothing to respective unit we don't log the status message */

+        if (!already) {

+                job_print_status_message(u, t, result);

+                job_log_status_message(u, t, result);

+        }

         job_add_to_dbus_queue(j);

@@ -842,20 +845,20 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {

                                 if (other->job &&

                                     (other->job->type == JOB_START ||

                                      other->job->type == JOB_VERIFY_ACTIVE))

-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);

+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);

                         SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i)

                                 if (other->job &&

                                     (other->job->type == JOB_START ||

                                      other->job->type == JOB_VERIFY_ACTIVE))

-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);

+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);

                         SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i)

                                 if (other->job &&

                                     !other->job->override &&

                                     (other->job->type == JOB_START ||

                                      other->job->type == JOB_VERIFY_ACTIVE))

-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);

+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);

                 } else if (t == JOB_STOP) {

@@ -863,7 +866,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {

                                 if (other->job &&

                                     (other->job->type == JOB_START ||

                                      other->job->type == JOB_VERIFY_ACTIVE))

-                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true);

+                                        job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true, false);

                 }

         }

@@ -911,7 +914,7 @@ static int job_dispatch_timer(sd_event_source *s, uint64_t monotonic, void *user

         log_unit_warning(j->unit->id, "Job %s/%s timed out.", j->unit->id, job_type_to_string(j->type));

         u = j->unit;

-        job_finish_and_invalidate(j, JOB_TIMEOUT, true);

+        job_finish_and_invalidate(j, JOB_TIMEOUT, true, false);

         failure_action(u->manager, u->job_timeout_action, u->job_timeout_reboot_arg);

diff --git a/src/core/job.h b/src/core/job.h

index d967b68..e4191ee 100644

--- a/src/core/job.h

+++ b/src/core/job.h

@@ -220,7 +220,7 @@ void job_add_to_dbus_queue(Job *j);

 int job_start_timer(Job *j);

 int job_run_and_invalidate(Job *j);

-int job_finish_and_invalidate(Job *j, JobResult result, bool recursive);

+int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool already);

 char *job_dbus_path(Job *j);

diff --git a/src/core/manager.c b/src/core/manager.c

index a1504bf..ee456fb 100644

--- a/src/core/manager.c

+++ b/src/core/manager.c

@@ -1426,7 +1426,7 @@ void manager_clear_jobs(Manager *m) {

         while ((j = hashmap_first(m->jobs)))

                 /* No need to recurse. We're cancelling all jobs. */

-                job_finish_and_invalidate(j, JOB_CANCELED, false);

+                job_finish_and_invalidate(j, JOB_CANCELED, false, false);

 }

 static int manager_dispatch_run_queue(sd_event_source *source, void *userdata) {

diff --git a/src/core/transaction.c b/src/core/transaction.c

index b0b3d6b..aed64fa 100644

--- a/src/core/transaction.c

+++ b/src/core/transaction.c

@@ -592,7 +592,7 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) {

                         /* Not invalidating recursively. Avoids triggering

                          * OnFailure= actions of dependent jobs. Also avoids

                          * invalidating our iterator. */

-                        job_finish_and_invalidate(j, JOB_CANCELED, false);

+                        job_finish_and_invalidate(j, JOB_CANCELED, false, false);

                 }

         }

diff --git a/src/core/unit.c b/src/core/unit.c

index db5aa98..d6ead7d 100644

--- a/src/core/unit.c

+++ b/src/core/unit.c

@@ -1851,12 +1851,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su

                 case JOB_VERIFY_ACTIVE:

                         if (UNIT_IS_ACTIVE_OR_RELOADING(ns))

-                                job_finish_and_invalidate(u->job, JOB_DONE, true);

+                                job_finish_and_invalidate(u->job, JOB_DONE, true, false);

                         else if (u->job->state == JOB_RUNNING && ns != UNIT_ACTIVATING) {

                                 unexpected = true;

                                 if (UNIT_IS_INACTIVE_OR_FAILED(ns))

-                                        job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true);

+                                        job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true, false);

                         }

                         break;

@@ -1866,12 +1866,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su

                         if (u->job->state == JOB_RUNNING) {

                                 if (ns == UNIT_ACTIVE)

-                                        job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true);

+                                        job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true, false);

                                 else if (ns != UNIT_ACTIVATING && ns != UNIT_RELOADING) {

                                         unexpected = true;

                                         if (UNIT_IS_INACTIVE_OR_FAILED(ns))

-                                                job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true);

+                                                job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true, false);

                                 }

                         }

@@ -1882,10 +1882,10 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su

                 case JOB_TRY_RESTART:

                         if (UNIT_IS_INACTIVE_OR_FAILED(ns))

-                                job_finish_and_invalidate(u->job, JOB_DONE, true);

+                                job_finish_and_invalidate(u->job, JOB_DONE, true, false);

                         else if (u->job->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) {

                                 unexpected = true;

-                                job_finish_and_invalidate(u->job, JOB_FAILED, true);

+                                job_finish_and_invalidate(u->job, JOB_FAILED, true, false);

                         }

                         break;