chiark / gitweb /
transaction: fix detection of cycles involving installed jobs
[elogind.git] / src / core / transaction.c
index c3e1e1329698efa4dfc0ff74d5b290259b8e7a88..394c18135232663b93bd025d17f9d96360308cd7 100644 (file)
@@ -1,3 +1,24 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2010 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
 #include "transaction.h"
 #include "bus-errors.h"
 
@@ -258,44 +279,32 @@ static int transaction_merge_jobs(Transaction *tr, DBusError *e) {
 }
 
 static void transaction_drop_redundant(Transaction *tr) {
-        bool again;
-
-        assert(tr);
-
-        /* Goes through the transaction and removes all jobs that are
-         * a noop */
-
-        do {
-                Job *j;
-                Iterator i;
-
-                again = false;
-
-                HASHMAP_FOREACH(j, tr->jobs, i) {
-                        bool changes_something = false;
-                        Job *k;
+        Job *j;
+        Iterator i;
 
-                        LIST_FOREACH(transaction, k, j) {
+        /* Goes through the transaction and removes all jobs of the units
+         * whose jobs are all noops. If not all of a unit's jobs are
+         * redundant, they are kept. */
 
-                                if (tr->anchor_job != k &&
-                                    job_type_is_redundant(k->type, unit_active_state(k->unit)) &&
-                                    (!k->unit->job || !job_type_is_conflicting(k->type, k->unit->job->type)))
-                                        continue;
+        assert(tr);
 
-                                changes_something = true;
-                                break;
-                        }
+rescan:
+        HASHMAP_FOREACH(j, tr->jobs, i) {
+                Job *k;
 
-                        if (changes_something)
-                                continue;
+                LIST_FOREACH(transaction, k, j) {
 
-                        /* log_debug("Found redundant job %s/%s, dropping.", j->unit->id, job_type_to_string(j->type)); */
-                        transaction_delete_job(tr, j, false);
-                        again = true;
-                        break;
+                        if (tr->anchor_job == k ||
+                            !job_type_is_redundant(k->type, unit_active_state(k->unit)) ||
+                            (k->unit->job && job_type_is_conflicting(k->type, k->unit->job->type)))
+                                goto next_unit;
                 }
 
-        } while (again);
+                /* log_debug("Found redundant job %s/%s, dropping.", j->unit->id, job_type_to_string(j->type)); */
+                transaction_delete_job(tr, j, false);
+                goto rescan;
+        next_unit:;
+        }
 }
 
 static bool unit_matters_to_anchor(Unit *u, Job *j) {
@@ -430,34 +439,27 @@ static int transaction_verify_order(Transaction *tr, unsigned *generation, DBusE
 }
 
 static void transaction_collect_garbage(Transaction *tr) {
-        bool again;
+        Iterator i;
+        Job *j;
 
         assert(tr);
 
         /* Drop jobs that are not required by any other job */
 
-        do {
-                Iterator i;
-                Job *j;
-
-                again = false;
-
-                HASHMAP_FOREACH(j, tr->jobs, i) {
-                        if (tr->anchor_job == j || j->object_list) {
-                                /* log_debug("Keeping job %s/%s because of %s/%s", */
-                                /*           j->unit->id, job_type_to_string(j->type), */
-                                /*           j->object_list->subject ? j->object_list->subject->unit->id : "root", */
-                                /*           j->object_list->subject ? job_type_to_string(j->object_list->subject->type) : "root"); */
-                                continue;
-                        }
-
-                        /* log_debug("Garbage collecting job %s/%s", j->unit->id, job_type_to_string(j->type)); */
-                        transaction_delete_job(tr, j, true);
-                        again = true;
-                        break;
+rescan:
+        HASHMAP_FOREACH(j, tr->jobs, i) {
+                if (tr->anchor_job == j || j->object_list) {
+                        /* log_debug("Keeping job %s/%s because of %s/%s", */
+                        /*           j->unit->id, job_type_to_string(j->type), */
+                        /*           j->object_list->subject ? j->object_list->subject->unit->id : "root", */
+                        /*           j->object_list->subject ? job_type_to_string(j->object_list->subject->type) : "root"); */
+                        continue;
                 }
 
-        } while (again);
+                /* log_debug("Garbage collecting job %s/%s", j->unit->id, job_type_to_string(j->type)); */
+                transaction_delete_job(tr, j, true);
+                goto rescan;
+        }
 }
 
 static int transaction_is_destructive(Transaction *tr, DBusError *e) {
@@ -487,59 +489,50 @@ static int transaction_is_destructive(Transaction *tr, DBusError *e) {
 }
 
 static void transaction_minimize_impact(Transaction *tr) {
-        bool again;
+        Job *j;
+        Iterator i;
+
         assert(tr);
 
         /* Drops all unnecessary jobs that reverse already active jobs
          * or that stop a running service. */
 
-        do {
-                Job *j;
-                Iterator i;
-
-                again = false;
-
-                HASHMAP_FOREACH(j, tr->jobs, i) {
-                        LIST_FOREACH(transaction, j, j) {
-                                bool stops_running_service, changes_existing_job;
-
-                                /* If it matters, we shouldn't drop it */
-                                if (j->matters_to_anchor)
-                                        continue;
+rescan:
+        HASHMAP_FOREACH(j, tr->jobs, i) {
+                LIST_FOREACH(transaction, j, j) {
+                        bool stops_running_service, changes_existing_job;
 
-                                /* Would this stop a running service?
-                                 * Would this change an existing job?
-                                 * If so, let's drop this entry */
+                        /* If it matters, we shouldn't drop it */
+                        if (j->matters_to_anchor)
+                                continue;
 
-                                stops_running_service =
-                                        j->type == JOB_STOP && UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(j->unit));
+                        /* Would this stop a running service?
+                         * Would this change an existing job?
+                         * If so, let's drop this entry */
 
-                                changes_existing_job =
-                                        j->unit->job &&
-                                        job_type_is_conflicting(j->type, j->unit->job->type);
+                        stops_running_service =
+                                j->type == JOB_STOP && UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(j->unit));
 
-                                if (!stops_running_service && !changes_existing_job)
-                                        continue;
+                        changes_existing_job =
+                                j->unit->job &&
+                                job_type_is_conflicting(j->type, j->unit->job->type);
 
-                                if (stops_running_service)
-                                        log_debug("%s/%s would stop a running service.", j->unit->id, job_type_to_string(j->type));
+                        if (!stops_running_service && !changes_existing_job)
+                                continue;
 
-                                if (changes_existing_job)
-                                        log_debug("%s/%s would change existing job.", j->unit->id, job_type_to_string(j->type));
+                        if (stops_running_service)
+                                log_debug("%s/%s would stop a running service.", j->unit->id, job_type_to_string(j->type));
 
-                                /* Ok, let's get rid of this */
-                                log_debug("Deleting %s/%s to minimize impact.", j->unit->id, job_type_to_string(j->type));
+                        if (changes_existing_job)
+                                log_debug("%s/%s would change existing job.", j->unit->id, job_type_to_string(j->type));
 
-                                transaction_delete_job(tr, j, true);
-                                again = true;
-                                break;
-                        }
+                        /* Ok, let's get rid of this */
+                        log_debug("Deleting %s/%s to minimize impact.", j->unit->id, job_type_to_string(j->type));
 
-                        if (again)
-                                break;
+                        transaction_delete_job(tr, j, true);
+                        goto rescan;
                 }
-
-        } while (again);
+        }
 }
 
 static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) {
@@ -610,6 +603,8 @@ rollback:
 }
 
 int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e) {
+        Iterator i;
+        Job *j;
         int r;
         unsigned generation = 1;
 
@@ -618,6 +613,12 @@ int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e
         /* This applies the changes recorded in tr->jobs to
          * the actual list of jobs, if possible. */
 
+        /* Reset the generation counter of all installed jobs. The detection of cycles
+         * looks at installed jobs. If they had a non-zero generation from some previous
+         * walk of the graph, the algorithm would break. */
+        HASHMAP_FOREACH(j, m->jobs, i)
+                j->generation = 0;
+
         /* First step: figure out which jobs matter */
         transaction_find_jobs_that_matter_to_anchor(tr->anchor_job, generation++);
 
@@ -822,7 +823,7 @@ int transaction_add_job_and_dependencies(
 
         if (type != JOB_STOP && unit->load_state == UNIT_MASKED) {
                 dbus_set_error(e, BUS_ERROR_MASKED, "Unit %s is masked.", unit->id);
-                return -EINVAL;
+                return -EADDRNOTAVAIL;
         }
 
         if (!unit_job_is_applicable(unit, type)) {
@@ -866,7 +867,7 @@ int transaction_add_job_and_dependencies(
                 }
 
                 /* Finally, recursively add in all dependencies. */
-                if (type == JOB_START || type == JOB_RELOAD_OR_START) {
+                if (type == JOB_START || type == JOB_RELOAD_OR_START || type == JOB_RESTART) {
                         SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRES], i) {
                                 r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e);
                                 if (r < 0) {
@@ -892,7 +893,8 @@ int transaction_add_job_and_dependencies(
                         SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRES_OVERRIDABLE], i) {
                                 r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, !override, override, false, false, ignore_order, e);
                                 if (r < 0) {
-                                        log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r));
+                                        log_full(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_WARNING,
+                                                 "Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r));
 
                                         if (e)
                                                 dbus_error_free(e);
@@ -902,7 +904,8 @@ int transaction_add_job_and_dependencies(
                         SET_FOREACH(dep, ret->unit->dependencies[UNIT_WANTS], i) {
                                 r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, false, false, false, false, ignore_order, e);
                                 if (r < 0) {
-                                        log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r));
+                                        log_full(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_WARNING,
+                                                 "Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r));
 
                                         if (e)
                                                 dbus_error_free(e);
@@ -923,7 +926,8 @@ int transaction_add_job_and_dependencies(
                         SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE_OVERRIDABLE], i) {
                                 r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, !override, override, false, false, ignore_order, e);
                                 if (r < 0) {
-                                        log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r));
+                                        log_full(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_WARNING,
+                                                 "Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r));
 
                                         if (e)
                                                 dbus_error_free(e);