make sure impact of transactions is minimized

6 files changed, 173 insertions, 17 deletions

diff --git a/job.c b/job.c
index 6241f203f9..d22ce19ee1 100644
--- a/job.c
+++ b/job.c
@@ -264,3 +264,12 @@ bool job_type_is_superset(JobType a, JobType b) {
 
         }
 }
+
+bool job_type_is_conflicting(JobType a, JobType b) {
+
+        /* Checks whether two types are "conflicting" */
+
+        return
+                (a == JOB_STOP && b != JOB_STOP) ||
+                (b == JOB_STOP && a != JOB_STOP);
+}
diff --git a/job.h b/job.h
index 33c599eaba..d839db5a40 100644
--- a/job.h
+++ b/job.h
@@ -94,5 +94,6 @@ const char* job_type_to_string(JobType t);
 int job_type_merge(JobType *a, JobType b);
 bool job_type_mergeable(JobType a, JobType b);
 bool job_type_is_superset(JobType a, JobType b);
+bool job_type_is_conflicting(JobType a, JobType b);
 
 #endif
diff --git a/manager.c b/manager.c
index 94ad680eb4..b1193b0ce3 100644
--- a/manager.c
+++ b/manager.c
@@ -244,9 +244,14 @@ static int transaction_merge_jobs(Manager *m) {
                 JobType t = j->type;
                 Job *k;
 
+                /* Merge all transactions */
                 for (k = j->transaction_next; k; k = k->transaction_next)
                         assert_se(job_type_merge(&t, k->type) == 0);
 
+                /* If an active job is mergable, merge it too */
+                if (j->name->meta.job)
+                        job_type_merge(&t, j->name->meta.job->type); /* Might fail. Which is OK */
+
                 while ((k = j->transaction_next)) {
                         if (j->linked) {
                                 transaction_merge_and_delete_job(m, k, j, t);
@@ -399,15 +404,61 @@ static int transaction_is_destructive(Manager *m, JobMode mode) {
         /* Checks whether applying this transaction means that
          * existing jobs would be replaced */
 
-        HASHMAP_FOREACH(j, m->transaction_jobs, state)
+        HASHMAP_FOREACH(j, m->transaction_jobs, state) {
+
+                /* Assume merged */
+                assert(!j->transaction_prev);
+                assert(!j->transaction_next);
+
                 if (j->name->meta.job &&
                     j->name->meta.job != j &&
                     !job_type_is_superset(j->type, j->name->meta.job->type))
                         return -EEXIST;
+        }
 
         return 0;
 }
 
+static void transaction_minimize_impact(Manager *m) {
+        bool again;
+        assert(m);
+
+        /* Drops all unnecessary jobs that reverse already active jobs
+         * or that stop a running service. */
+
+        do {
+                Job *j;
+                void *state;
+
+                again = false;
+
+                HASHMAP_FOREACH(j, m->transaction_jobs, state) {
+                        for (; j; j = j->transaction_next) {
+
+                                /* If it matters, we shouldn't drop it */
+                                if (j->matters_to_anchor)
+                                        continue;
+
+                                /* Would this stop a running service?
+                                 * Would this change an existing job?
+                                 * If so, let's drop this entry */
+                                if ((j->type != JOB_STOP || name_is_dead(j->name)) &&
+                                    (!j->name->meta.job  || job_type_is_conflicting(j->type, j->name->meta.job->state)))
+                                        continue;
+
+                                /* Ok, let's get rid of this */
+                                transaction_delete_job(m, j);
+                                again = true;
+                                break;
+                        }
+
+                        if (again)
+                                break;
+                }
+
+        } while (again);
+}
+
 static int transaction_apply(Manager *m, JobMode mode) {
         void *state;
         Job *j;
@@ -416,6 +467,10 @@ static int transaction_apply(Manager *m, JobMode mode) {
         /* Moves the transaction jobs to the set of active jobs */
 
         HASHMAP_FOREACH(j, m->transaction_jobs, state) {
+                /* Assume merged */
+                assert(!j->transaction_prev);
+                assert(!j->transaction_next);
+
                 if (j->linked)
                         continue;
 
@@ -461,7 +516,6 @@ rollback:
         return r;
 }
 
-
 static int transaction_activate(Manager *m, JobMode mode) {
         int r;
         unsigned generation = 1;
@@ -474,12 +528,17 @@ static int transaction_activate(Manager *m, JobMode mode) {
         /* First step: figure out which jobs matter */
         transaction_find_jobs_that_matter_to_anchor(m, NULL, generation++);
 
+        /* Second step: Try not to stop any running services if
+         * we don't have to. Don't try to reverse running
+         * jobs if we don't have to. */
+        transaction_minimize_impact(m);
+
         for (;;) {
-                /* Second step: Let's remove unneeded jobs that might
+                /* Third step: Let's remove unneeded jobs that might
                  * be lurking. */
                 transaction_collect_garbage(m);
 
-                /* Third step: verify order makes sense and correct
+                /* Fourth step: verify order makes sense and correct
                  * cycles if necessary and possible */
                 if ((r = transaction_verify_order(m, &generation)) >= 0)
                         break;
@@ -492,7 +551,7 @@ static int transaction_activate(Manager *m, JobMode mode) {
         }
 
         for (;;) {
-                /* Fourth step: let's drop unmergable entries if
+                /* Fifth step: let's drop unmergable entries if
                  * necessary and possible, merge entries we can
                  * merge */
                 if ((r = transaction_merge_jobs(m)) >= 0)
@@ -501,7 +560,7 @@ static int transaction_activate(Manager *m, JobMode mode) {
                 if (r != -EAGAIN)
                         goto rollback;
 
-                /* Fifth step: an entry got dropped, let's garbage
+                /* Sixth step: an entry got dropped, let's garbage
                  * collect its dependencies. */
                 transaction_collect_garbage(m);
 
@@ -509,12 +568,12 @@ static int transaction_activate(Manager *m, JobMode mode) {
                  * unmergable entries ... */
         }
 
-        /* Sith step: check whether we can actually apply this */
+        /* Seventh step: check whether we can actually apply this */
         if (mode == JOB_FAIL)
                 if ((r = transaction_is_destructive(m, mode)) < 0)
                         goto rollback;
 
-        /* Seventh step: apply changes */
+        /* Eights step: apply changes */
         if ((r = transaction_apply(m, mode)) < 0)
                 goto rollback;
 
diff --git a/name.c b/name.c
index 340e8e424a..08194d05f4 100644
--- a/name.c
+++ b/name.c
@@ -226,7 +226,6 @@ void name_free(Name *name) {
 }
 
 bool name_is_ready(Name *name) {
-
         assert(name);
 
         if (name->meta.state != NAME_LOADED)
@@ -291,6 +290,69 @@ bool name_is_ready(Name *name) {
         return false;
 }
 
+bool name_is_dead(Name *name) {
+        assert(name);
+
+        if (name->meta.state != NAME_LOADED)
+                return true;
+        assert(name->meta.type < _NAME_TYPE_MAX);
+
+        switch (name->meta.type) {
+                case NAME_SERVICE: {
+                        Service *s = SERVICE(name);
+
+                        return
+                                s->state == SERVICE_DEAD ||
+                                s->state == SERVICE_MAINTAINANCE;
+                }
+
+                case NAME_TIMER:
+                        return TIMER(name)->state == TIMER_DEAD;
+
+                case NAME_SOCKET: {
+                        Socket *s = SOCKET(name);
+
+                        return
+                                s->state == SOCKET_DEAD ||
+                                s->state == SOCKET_MAINTAINANCE;
+                }
+
+                case NAME_MILESTONE:
+                        return MILESTONE(name)->state == MILESTONE_DEAD;
+
+                case NAME_DEVICE:
+                        return DEVICE(name)->state == DEVICE_DEAD;
+
+                case NAME_MOUNT: {
+                        Mount *a = MOUNT(name);
+
+                        return
+                                a->state == AUTOMOUNT_DEAD ||
+                                a->state == AUTOMOUNT_MAINTAINANCE;
+                }
+
+                case NAME_AUTOMOUNT: {
+                        Automount *a = AUTOMOUNT(name);
+
+                        return
+                                a->state == AUTOMOUNT_DEAD ||
+                                a->state == AUTOMOUNT_MAINTAINANCE;
+                }
+
+                case NAME_SNAPSHOT:
+                        return SNAPSHOT(name)->state == SNAPSHOT_DEAD;
+
+
+                case _NAME_TYPE_MAX:
+                case _NAME_TYPE_INVALID:
+                        ;
+        }
+
+        assert_not_reached("Unknown name type.");
+        return false;
+}
+
+
 static int ensure_in_set(Set **s, void *data) {
         int r;
 
diff --git a/name.h b/name.h
index 52d92776f3..869e71bb09 100644
--- a/name.h
+++ b/name.h
@@ -124,8 +124,7 @@ typedef enum TimerState {
         TIMER_RUNNING,
         TIMER_STOP_PRE,
         TIMER_STOP,
-        TIMER_STOP_POST,
-        TIMER_MAINTAINANCE
+        TIMER_STOP_POST
 } TimerState;
 
 struct Timer {
@@ -193,7 +192,12 @@ struct Device {
 typedef enum MountState {
         MOUNT_DEAD,
         MOUNT_BEFORE,
-        MOUNT_MOUNTED
+        MOUNT_MOUNTING,
+        MOUNT_MOUNTED,
+        MOUNT_UNMOUNTING,
+        MOUNT_SIGTERM,  /* if the mount command hangs */
+        MOUNT_SIGKILL,
+        MOUNT_MAINTAINANCE
 } MountState;
 
 struct Mount {
@@ -272,7 +276,9 @@ DEFINE_CAST(SNAPSHOT, Snapshot, snapshot);
 /* For casting the various name types into a name */
 #define NAME(o) ((Name*) (o))
 
-bool name_is_ready(Name *name);
+bool name_is_running(Name *name);
+bool name_is_dead(Name *name);
+
 NameType name_type_from_string(const char *n);
 bool name_is_valid(const char *n);
 
diff --git a/test-engine.c b/test-engine.c
index f92b05a7dd..53cac92e72 100644
--- a/test-engine.c
+++ b/test-engine.c
@@ -9,14 +9,14 @@
 
 int main(int argc, char *argv[]) {
         Manager *m = NULL;
-        Name *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL;
+        Name *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL, *h = NULL;
         Job *j;
 
         assert_se(chdir("test2") == 0);
 
         assert_se(m = manager_new());
 
-        printf("Loaded1:\n");
+        printf("Load1:\n");
         assert_se(manager_load_name(m, "a.service", &a) == 0);
         assert_se(manager_load_name(m, "b.service", &b) == 0);
         assert_se(manager_load_name(m, "c.service", &c) == 0);
@@ -26,7 +26,7 @@ int main(int argc, char *argv[]) {
         assert_se(manager_add_job(m, JOB_START, c, JOB_REPLACE, false, &j) == 0);
         manager_dump_jobs(m, stdout, "\t");
 
-        printf("Loaded2:\n");
+        printf("Load2:\n");
         manager_clear_jobs(m);
         assert_se(manager_load_name(m, "d.service", &d) == 0);
         assert_se(manager_load_name(m, "e.service", &e) == 0);
@@ -44,7 +44,7 @@ int main(int argc, char *argv[]) {
         assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, false, &j) == 0);
         manager_dump_jobs(m, stdout, "\t");
 
-        printf("Loaded3:\n");
+        printf("Load3:\n");
         assert_se(manager_load_name(m, "g.service", &g) == 0);
         manager_dump_names(m, stdout, "\t");
 
@@ -55,6 +55,25 @@ int main(int argc, char *argv[]) {
         assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, false, &j) == 0);
         manager_dump_jobs(m, stdout, "\t");
 
+        printf("Test7: (Unmeargable job type, fail)\n");
+        assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, false, &j) == -EEXIST);
+
+        printf("Test8: (Mergeable job type, fail)\n");
+        assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, false, &j) == 0);
+        manager_dump_jobs(m, stdout, "\t");
+
+        printf("Test9: (Unmeargable job type, replace)\n");
+        assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, false, &j) == 0);
+        manager_dump_jobs(m, stdout, "\t");
+
+        printf("Load4:\n");
+        assert_se(manager_load_name(m, "h.service", &h) == 0);
+        manager_dump_names(m, stdout, "\t");
+
+        printf("Test10: (Unmeargable job type of auxiliary job, fail)\n");
+        assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, false, &j) == 0);
+        manager_dump_jobs(m, stdout, "\t");
+
         manager_free(m);
 
         return 0;