From: Michal Koutný <mkoutny@suse.com>
Date: Thu, 6 Jun 2019 21:27:20 +0000 (+0200)
Subject: core: Check transaction against execution cycles
X-Git-Tag: v243-rc1~152^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dfd79eca55e3d4bd61813ca4cf8887544d952c28;p=thirdparty%2Fsystemd.git

core: Check transaction against execution cycles

When we are validating a transaction, we take into account declared
ordering between job units. However, since JOB_STOP goes always first
regardless of the ordering constraint between respective units, we may
detect some false cycles in the transaction which would not prevent the
execution though.

Use the same logic in transaction checking as we use for job execution.
---

diff --git a/src/core/transaction.c b/src/core/transaction.c
index 3b6b240d361..d1bf2e64c91 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -353,6 +353,11 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi
         Unit *u;
         void *v;
         int r;
+        static const UnitDependency directions[] = {
+                UNIT_BEFORE,
+                UNIT_AFTER,
+        };
+        size_t d;
 
         assert(tr);
         assert(j);
@@ -441,25 +446,33 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi
         j->marker = from ? from : j;
         j->generation = generation;
 
-        /* We assume that the dependencies are bidirectional, and
-         * hence can ignore UNIT_AFTER */
-        HASHMAP_FOREACH_KEY(v, u, j->unit->dependencies[UNIT_BEFORE], i) {
-                Job *o;
-
-                /* Is there a job for this unit? */
-                o = hashmap_get(tr->jobs, u);
-                if (!o) {
-                        /* Ok, there is no job for this in the
-                         * transaction, but maybe there is already one
-                         * running? */
-                        o = u->job;
-                        if (!o)
+        /* Actual ordering of jobs depends on the unit ordering dependency and job types. We need to traverse
+         * the graph over 'before' edges in the actual job execution order. We traverse over both unit
+         * ordering dependencies and we test with job_compare() whether it is the 'before' edge in the job
+         * execution ordering. */
+        for (d = 0; d < ELEMENTSOF(directions); d++) {
+                HASHMAP_FOREACH_KEY(v, u, j->unit->dependencies[directions[d]], i) {
+                        Job *o;
+
+                        /* Is there a job for this unit? */
+                        o = hashmap_get(tr->jobs, u);
+                        if (!o) {
+                                /* Ok, there is no job for this in the
+                                 * transaction, but maybe there is already one
+                                 * running? */
+                                o = u->job;
+                                if (!o)
+                                        continue;
+                        }
+
+                        /* Cut traversing if the job j is not really *before* o. */
+                        if (job_compare(j, o, directions[d]) >= 0)
                                 continue;
-                }
 
-                r = transaction_verify_order_one(tr, o, j, generation, e);
-                if (r < 0)
-                        return r;
+                        r = transaction_verify_order_one(tr, o, j, generation, e);
+                        if (r < 0)
+                                return r;
+                }
         }
 
         /* Ok, let's backtrack, and remember that this entry is not on