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

diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
index 97a93fb2f1573cbb83cf53ebafde5084f49b9542..ccf7453d47a1fbeeafb6cb6436ddb24d5d7f36f9 100644 (file)
--- a/src/core/dbus-job.c
+++ b/src/core/dbus-job.c
@@ -75,7 +75,7 @@ int bus_job_method_cancel(sd_bus_message *message, void *userdata, sd_bus_error
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
         }
 
-        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 d9c5669c9fbb65f813dcdaeff5f8a2f238b05784..7557874d4d6358cb5aa47f9e238f96d8d310d9aa 100644 (file)
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -191,7 +191,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 */
 
@@ -614,19 +614,19 @@ int job_run_and_invalidate(Job *j) {
 
         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 == -EOPNOTSUPP)
-                        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;
@@ -827,11 +827,11 @@ static void job_fail_dependencies(Unit *u, UnitDependency d) {
                 if (!IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE))
                         continue;
 
-                job_finish_and_invalidate(j, JOB_DEPENDENCY, true);
+                job_finish_and_invalidate(j, JOB_DEPENDENCY, true, false);
         }
 }
 
-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;
@@ -848,7 +848,9 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
 
         log_unit_debug(u, "Job %s/%s finished, result=%s", u->id, job_type_to_string(t), job_result_to_string(result));
 
-        job_emit_status_message(u, t, result);
+        /* If this job did nothing to respective unit we don't log the status message */
+        if (!already)
+                job_emit_status_message(u, t, result);
 
         job_add_to_dbus_queue(j);
 
@@ -923,7 +925,7 @@ static int job_dispatch_timer(sd_event_source *s, uint64_t monotonic, void *user
         log_unit_warning(j->unit, "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 856b0ce82935a43376ee55dcc1f55afaf1a353e8..d359e8bb3ed8bca02df4f7922a048bcf985b9964 100644 (file)
--- a/src/core/job.h
+++ b/src/core/job.h
@@ -219,7 +219,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 e192cd475dc7a68d3e8aaf00faad80d0ee0802ba..7838f56fd209c0197b698944f98b497ac2dedd3d 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1494,7 +1494,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 d5370b2a1451018894f5d412ead31752e8482ec9..e06a48a2f1e440bbda5687ea349ba53a75cb7733 100644 (file)
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -597,7 +597,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 dc8325515c74575a0b683ac1e872d7750eb0fce0..acbd3d0bdfe3926a7cea35beb0ce990de8c2b1ff 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1916,12 +1916,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;
@@ -1932,12 +1932,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);
                                 }
                         }
 
@@ -1948,10 +1948,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;