manager: when two pending jobs conflict, keep the one that "conflicts", remove the one that is "conflicted"

This gives the writer of units control which unit is kept and which is
stopped when two units conflict.

13 files changed, 89 insertions, 21 deletions

diff --git a/src/automount.c b/src/automount.c
index 57d1065049..9843481a61 100644
--- a/src/automount.c
+++ b/src/automount.c
@@ -156,7 +156,7 @@ static int automount_add_default_dependencies(Automount *a) {
                 if ((r = unit_add_dependency_by_name(UNIT(a), UNIT_AFTER, SPECIAL_FSCK_TARGET, NULL, true)) < 0)
                         return r;
 
-                if ((r = unit_add_two_dependencies_by_name(UNIT(a), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true)) < 0)
+                if ((r = unit_add_two_dependencies_by_name(UNIT(a), UNIT_BEFORE, UNIT_CONFLICTED_BY, SPECIAL_UMOUNT_TARGET, NULL, true)) < 0)
                         return r;
         }
 
diff --git a/src/dbus-unit.h b/src/dbus-unit.h
index 0d17322709..0744377ed1 100644
--- a/src/dbus-unit.h
+++ b/src/dbus-unit.h
@@ -70,6 +70,7 @@
         "  <property name=\"RequiredByOverridable\" type=\"as\" access=\"read\"/>\n" \
         "  <property name=\"WantedBy\" type=\"as\" access=\"read\"/>\n" \
         "  <property name=\"Conflicts\" type=\"as\" access=\"read\"/>\n" \
+        "  <property name=\"ConflictedBy\" type=\"as\" access=\"read\"/>\n" \
         "  <property name=\"Before\" type=\"as\" access=\"read\"/>\n"   \
         "  <property name=\"After\" type=\"as\" access=\"read\"/>\n"    \
         "  <property name=\"OnFailure\" type=\"as\" access=\"read\"/>\n"    \
@@ -108,6 +109,7 @@
         { "org.freedesktop.systemd1.Unit", "RequiredByOverridable",bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_REQUIRED_BY_OVERRIDABLE] }, \
         { "org.freedesktop.systemd1.Unit", "WantedBy",             bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_WANTED_BY] }, \
         { "org.freedesktop.systemd1.Unit", "Conflicts",            bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_CONFLICTS] }, \
+        { "org.freedesktop.systemd1.Unit", "ConflictedBy",         bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_CONFLICTED_BY] }, \
         { "org.freedesktop.systemd1.Unit", "Before",               bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_BEFORE] }, \
         { "org.freedesktop.systemd1.Unit", "After",                bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_AFTER]  }, \
         { "org.freedesktop.systemd1.Unit", "OnFailure",            bus_unit_append_dependencies,   "as",   u->meta.dependencies[UNIT_ON_FAILURE] }, \
diff --git a/src/job.c b/src/job.c
index d08bfe9a77..c3b529e920 100644
--- a/src/job.c
+++ b/src/job.c
@@ -93,7 +93,7 @@ void job_free(Job *j) {
         free(j);
 }
 
-JobDependency* job_dependency_new(Job *subject, Job *object, bool matters) {
+JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) {
         JobDependency *l;
 
         assert(object);
@@ -109,6 +109,7 @@ JobDependency* job_dependency_new(Job *subject, Job *object, bool matters) {
         l->subject = subject;
         l->object = object;
         l->matters = matters;
+        l->conflicts = conflicts;
 
         if (subject)
                 LIST_PREPEND(JobDependency, subject, subject->subject_list, l);
@@ -533,6 +534,14 @@ int job_finish_and_invalidate(Job *j, bool success) {
                                      other->meta.job->type == JOB_VERIFY_ACTIVE ||
                                      other->meta.job->type == JOB_RELOAD_OR_START))
                                         job_finish_and_invalidate(other->meta.job, false);
+
+                        SET_FOREACH(other, u->meta.dependencies[UNIT_CONFLICTED_BY], i)
+                                if (!other->meta.ignore_dependency_failure &&
+                                    other->meta.job &&
+                                    (other->meta.job->type == JOB_START ||
+                                     other->meta.job->type == JOB_VERIFY_ACTIVE ||
+                                     other->meta.job->type == JOB_RELOAD_OR_START))
+                                        job_finish_and_invalidate(other->meta.job, false);
                 }
         }
 
diff --git a/src/job.h b/src/job.h
index 41d697e842..a69c4aaf1d 100644
--- a/src/job.h
+++ b/src/job.h
@@ -80,6 +80,7 @@ struct JobDependency {
         LIST_FIELDS(JobDependency, object);
 
         bool matters;
+        bool conflicts;
 };
 
 struct Job {
@@ -121,7 +122,7 @@ Job* job_new(Manager *m, JobType type, Unit *unit);
 void job_free(Job *job);
 void job_dump(Job *j, FILE*f, const char *prefix);
 
-JobDependency* job_dependency_new(Job *subject, Job *object, bool matters);
+JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts);
 void job_dependency_free(JobDependency *l);
 void job_dependency_delete(Job *subject, Job *object, bool *matters);
 
diff --git a/src/manager.c b/src/manager.c
index 9b6cbc94e0..ddb253ae4e 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -715,6 +715,20 @@ static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, Job
         other->object_list = NULL;
         transaction_delete_job(m, other, true);
 }
+static bool job_is_conflicted_by(Job *j) {
+        JobDependency *l;
+
+        assert(j);
+
+        /* Returns true if this job is pulled in by a least one
+         * ConflictedBy dependency. */
+
+        LIST_FOREACH(object, l, j->object_list)
+                if (l->conflicts)
+                        return true;
+
+        return false;
+}
 
 static int delete_one_unmergeable_job(Manager *m, Job *j) {
         Job *k;
@@ -738,7 +752,36 @@ static int delete_one_unmergeable_job(Manager *m, Job *j) {
 
                         /* Ok, we found two that conflict, let's see if we can
                          * drop one of them */
-                        if (!j->matters_to_anchor)
+                        if (!j->matters_to_anchor && !k->matters_to_anchor) {
+
+                                /* Both jobs don't matter, so let's
+                                 * find the one that is smarter to
+                                 * remove. Let's think positive and
+                                 * rather remove stops then starts --
+                                 * except if something is being
+                                 * stopped because it is conflicted by
+                                 * another unit in which case we
+                                 * rather remove the start. */
+
+                                log_debug("Looking at job %s/%s conflicted_by=%s", j->unit->meta.id, job_type_to_string(j->type), yes_no(j->type == JOB_STOP && job_is_conflicted_by(j)));
+                                log_debug("Looking at job %s/%s conflicted_by=%s", k->unit->meta.id, job_type_to_string(k->type), yes_no(k->type == JOB_STOP && job_is_conflicted_by(k)));
+
+                                if (j->type == JOB_STOP) {
+
+                                        if (job_is_conflicted_by(j))
+                                                d = k;
+                                        else
+                                                d = j;
+
+                                } else if (k->type == JOB_STOP) {
+
+                                        if (job_is_conflicted_by(k))
+                                                d = j;
+                                        else
+                                                d = k;
+                                }
+
+                        } else if (!j->matters_to_anchor)
                                 d = j;
                         else if (!k->matters_to_anchor)
                                 d = k;
@@ -1324,6 +1367,7 @@ static int transaction_add_job_and_dependencies(
                 Job *by,
                 bool matters,
                 bool override,
+                bool conflicts,
                 DBusError *e,
                 Job **_ret) {
         Job *ret;
@@ -1352,46 +1396,50 @@ static int transaction_add_job_and_dependencies(
                 return -ENOMEM;
 
         /* Then, add a link to the job. */
-        if (!job_dependency_new(by, ret, matters))
+        if (!job_dependency_new(by, ret, matters, conflicts))
                 return -ENOMEM;
 
         if (is_new) {
                 /* Finally, recursively add in all dependencies. */
                 if (type == JOB_START || type == JOB_RELOAD_OR_START) {
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_REQUIRES], i)
-                                if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, true, override, e, NULL)) < 0 && r != -EBADR)
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, true, override, false, e, NULL)) < 0 && r != -EBADR)
                                         goto fail;
 
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_REQUIRES_OVERRIDABLE], i)
-                                if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, !override, override, e, NULL)) < 0 && r != -EBADR) {
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, !override, override, false, e, NULL)) < 0 && r != -EBADR) {
                                         log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->meta.id, bus_error(e, r));
                                         dbus_error_free(e);
                                 }
 
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_WANTS], i)
-                                if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, false, false, e, NULL)) < 0) {
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, false, false, false, e, NULL)) < 0) {
                                         log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->meta.id, bus_error(e, r));
                                         dbus_error_free(e);
                                 }
 
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_REQUISITE], i)
-                                if ((r = transaction_add_job_and_dependencies(m, JOB_VERIFY_ACTIVE, dep, ret, true, override, e, NULL)) < 0 && r != -EBADR)
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, e, NULL)) < 0 && r != -EBADR)
                                         goto fail;
 
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_REQUISITE_OVERRIDABLE], i)
-                                if ((r = transaction_add_job_and_dependencies(m, JOB_VERIFY_ACTIVE, dep, ret, !override, override, e, NULL)) < 0 && r != -EBADR) {
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_VERIFY_ACTIVE, dep, ret, !override, override, false, e, NULL)) < 0 && r != -EBADR) {
                                         log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->meta.id, bus_error(e, r));
                                         dbus_error_free(e);
                                 }
 
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_CONFLICTS], i)
-                                if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, dep, ret, true, override, e, NULL)) < 0 && r != -EBADR)
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, dep, ret, true, override, true, e, NULL)) < 0 && r != -EBADR)
+                                        goto fail;
+
+                        SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_CONFLICTED_BY], i)
+                                if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, dep, ret, true, override, false, e, NULL)) < 0 && r != -EBADR)
                                         goto fail;
 
                 } else if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) {
 
                         SET_FOREACH(dep, ret->unit->meta.dependencies[UNIT_REQUIRED_BY], i)
-                                if ((r = transaction_add_job_and_dependencies(m, type, dep, ret, true, override, e, NULL)) < 0 && r != -EBADR)
+                                if ((r = transaction_add_job_and_dependencies(m, type, dep, ret, true, override, false, e, NULL)) < 0 && r != -EBADR)
                                         goto fail;
                 }
 
@@ -1432,7 +1480,7 @@ static int transaction_add_isolate_jobs(Manager *m) {
                 if (hashmap_get(m->transaction_jobs, u))
                         continue;
 
-                if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, u, NULL, true, false, NULL, NULL)) < 0)
+                if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, u, NULL, true, false, false, NULL, NULL)) < 0)
                         log_warning("Cannot add isolate job for unit %s, ignoring: %s", u->meta.id, strerror(-r));
         }
 
@@ -1455,7 +1503,7 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove
 
         log_debug("Trying to enqueue job %s/%s", unit->meta.id, job_type_to_string(type));
 
-        if ((r = transaction_add_job_and_dependencies(m, type, unit, NULL, true, override, e, &ret)) < 0) {
+        if ((r = transaction_add_job_and_dependencies(m, type, unit, NULL, true, override, false, e, &ret)) < 0) {
                 transaction_abort(m);
                 return r;
         }
diff --git a/src/mount.c b/src/mount.c
index ba85d8c0a4..cf2355a333 100644
--- a/src/mount.c
+++ b/src/mount.c
@@ -285,7 +285,7 @@ static int mount_add_default_dependencies(Mount *m) {
                 if ((r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_FSCK_TARGET, NULL, true)) < 0)
                         return r;
 
-                if ((r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true)) < 0)
+                if ((r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTED_BY, SPECIAL_UMOUNT_TARGET, NULL, true)) < 0)
                         return r;
         }
 
diff --git a/src/path.c b/src/path.c
index f4c20940d0..91de56f34c 100644
--- a/src/path.c
+++ b/src/path.c
@@ -110,7 +110,7 @@ static int path_add_default_dependencies(Path *p) {
                 if ((r = unit_add_two_dependencies_by_name(UNIT(p), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true)) < 0)
                         return r;
 
-        return unit_add_two_dependencies_by_name(UNIT(p), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true);
+        return unit_add_two_dependencies_by_name(UNIT(p), UNIT_BEFORE, UNIT_CONFLICTED_BY, SPECIAL_SHUTDOWN_TARGET, NULL, true);
 }
 
 static int path_load(Unit *u) {
diff --git a/src/service.c b/src/service.c
index c052d7cdcb..d6086ca410 100644
--- a/src/service.c
+++ b/src/service.c
@@ -873,7 +873,7 @@ static int service_add_default_dependencies(Service *s) {
         }
 
         /* Second, activate normal shutdown */
-        return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true);
+        return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTED_BY, SPECIAL_SHUTDOWN_TARGET, NULL, true);
 }
 
 static int service_load(Unit *u) {
diff --git a/src/socket.c b/src/socket.c
index 2a5270f336..7c280e0067 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -296,7 +296,7 @@ static int socket_add_default_dependencies(Socket *s) {
                 if ((r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true)) < 0)
                         return r;
 
-        return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true);
+        return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTED_BY, SPECIAL_SHUTDOWN_TARGET, NULL, true);
 }
 
 static int socket_load(Unit *u) {
diff --git a/src/systemctl.c b/src/systemctl.c
index 27e5e3708e..e49c5b8671 100644
--- a/src/systemctl.c
+++ b/src/systemctl.c
@@ -311,6 +311,7 @@ static int dot_one_property(const char *name, const char *prop, DBusMessageIter
                 "RequisiteOverridable",  "[color=\"darkblue\"]",
                 "Wants",                 "[color=\"darkgrey\"]",
                 "Conflicts",             "[color=\"red\"]",
+                "ConflictedBy",          "[color=\"red\"]",
                 "After",                 "[color=\"green\"]"
         };
 
diff --git a/src/timer.c b/src/timer.c
index cd6728a186..2c21b49197 100644
--- a/src/timer.c
+++ b/src/timer.c
@@ -82,7 +82,7 @@ static int timer_add_default_dependencies(Timer *t) {
                 if ((r = unit_add_two_dependencies_by_name(UNIT(t), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true)) < 0)
                         return r;
 
-        return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true);
+        return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTED_BY, SPECIAL_SHUTDOWN_TARGET, NULL, true);
 }
 
 static int timer_load(Unit *u) {
diff --git a/src/unit.c b/src/unit.c
index dd665e2d5a..d205ca4aae 100644
--- a/src/unit.c
+++ b/src/unit.c
@@ -934,6 +934,10 @@ static void retroactively_start_dependencies(Unit *u) {
         SET_FOREACH(other, u->meta.dependencies[UNIT_CONFLICTS], i)
                 if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
                         manager_add_job(u->meta.manager, JOB_STOP, other, JOB_REPLACE, true, NULL, NULL);
+
+        SET_FOREACH(other, u->meta.dependencies[UNIT_CONFLICTED_BY], i)
+                if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other)))
+                        manager_add_job(u->meta.manager, JOB_STOP, other, JOB_REPLACE, true, NULL, NULL);
 }
 
 static void retroactively_stop_dependencies(Unit *u) {
@@ -1312,7 +1316,8 @@ int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_referen
                 [UNIT_REQUIRED_BY] = _UNIT_DEPENDENCY_INVALID,
                 [UNIT_REQUIRED_BY_OVERRIDABLE] = _UNIT_DEPENDENCY_INVALID,
                 [UNIT_WANTED_BY] = _UNIT_DEPENDENCY_INVALID,
-                [UNIT_CONFLICTS] = UNIT_CONFLICTS,
+                [UNIT_CONFLICTS] = UNIT_CONFLICTED_BY,
+                [UNIT_CONFLICTED_BY] = UNIT_CONFLICTS,
                 [UNIT_BEFORE] = UNIT_AFTER,
                 [UNIT_AFTER] = UNIT_BEFORE,
                 [UNIT_ON_FAILURE] = _UNIT_DEPENDENCY_INVALID,
@@ -2138,6 +2143,7 @@ static const char* const unit_dependency_table[_UNIT_DEPENDENCY_MAX] = {
         [UNIT_REQUIRED_BY_OVERRIDABLE] = "RequiredByOverridable",
         [UNIT_WANTED_BY] = "WantedBy",
         [UNIT_CONFLICTS] = "Conflicts",
+        [UNIT_CONFLICTED_BY] = "ConflictedBy",
         [UNIT_BEFORE] = "Before",
         [UNIT_AFTER] = "After",
         [UNIT_REFERENCES] = "References",
diff --git a/src/unit.h b/src/unit.h
index 34e86d1086..f657aea3e2 100644
--- a/src/unit.h
+++ b/src/unit.h
@@ -108,7 +108,8 @@ enum UnitDependency {
         UNIT_WANTED_BY,               /* inverse of 'wants' */
 
         /* Negative dependencies */
-        UNIT_CONFLICTS,               /* inverse of 'conflicts' is 'conflicts' */
+        UNIT_CONFLICTS,               /* inverse of 'conflicts' is 'conflicted_by' */
+        UNIT_CONFLICTED_BY,
 
         /* Order */
         UNIT_BEFORE,                  /* inverse of 'before' is 'after' and vice versa */