From bd156cfcf68075f43344870cf7072f7afd4167a338a939c11e7a9108907f009e Mon Sep 17 00:00:00 2001 From: Andreas Jaeger Date: Tue, 3 Jul 2012 16:31:03 +0000 Subject: [PATCH] Accepting request 127017 from home:fcrozat:branches:Base:System - Add fix-dir-noatime-tmpfiles.patch: do not modify directory atime, which was preventing removing empty directories (bnc#751253, rh#810257). - Add improve-restart-behaviour.patch: prevent deadlock during try-restart (bnc#743218). - Add journal-bugfixes.patch: don't crash when rotating journal (bnc#768953) and prevent memleak at rotation time too. - Add ulimit-support.patch: add support for system wide ulimit (bnc#744818). - Add change-terminal.patch: use vt102 instead of vt100 as terminal for non-vc tty. - Package various .wants directories, which were no longer packaged due to plymouth units being removed from systemd package. - Fix buildrequires for manpages build. OBS-URL: https://build.opensuse.org/request/show/127017 OBS-URL: https://build.opensuse.org/package/show/Base:System/systemd?expand=0&rev=284 --- change-terminal.patch | 37 + fix-dir-noatime-tmpfiles.patch | 41 + improve-restart-behaviour.patch | 7661 +++++++++++++++++++++++++++++++ journal-bugfixes.patch | 186 + systemd.changes | 18 + systemd.spec | 19 +- ulimit-support.patch | 253 + 7 files changed, 8213 insertions(+), 2 deletions(-) create mode 100644 change-terminal.patch create mode 100644 fix-dir-noatime-tmpfiles.patch create mode 100644 improve-restart-behaviour.patch create mode 100644 journal-bugfixes.patch create mode 100644 ulimit-support.patch diff --git a/change-terminal.patch b/change-terminal.patch new file mode 100644 index 00000000..93d877c0 --- /dev/null +++ b/change-terminal.patch @@ -0,0 +1,37 @@ +From 1505a61772a6e697f2aabdbb0e827a88b0d7ee6b Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Sun, 22 Apr 2012 02:45:39 +0200 +Subject: [PATCH] default to v102 everywhere, instead of vt100, to synchronize + with agetty + +--- + src/util.c | 2 +- + units/serial-getty@.service.m4 | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +Index: systemd-44/src/util.c +=================================================================== +--- systemd-44.orig/src/util.c ++++ systemd-44/src/util.c +@@ -4425,7 +4425,7 @@ bool tty_is_vc_resolve(const char *tty) + const char *default_term_for_tty(const char *tty) { + assert(tty); + +- return tty_is_vc_resolve(tty) ? "TERM=linux" : "TERM=vt100"; ++ return tty_is_vc_resolve(tty) ? "TERM=linux" : "TERM=vt102"; + } + + bool dirent_is_file(const struct dirent *de) { +Index: systemd-44/units/serial-getty@.service.m4 +=================================================================== +--- systemd-44.orig/units/serial-getty@.service.m4 ++++ systemd-44/units/serial-getty@.service.m4 +@@ -35,7 +35,7 @@ Before=getty.target + IgnoreOnIsolate=yes + + [Service] +-Environment=TERM=vt100 ++Environment=TERM=vt102 + ExecStart=-/sbin/agetty -s %I 115200,38400,9600 + Restart=always + RestartSec=0 diff --git a/fix-dir-noatime-tmpfiles.patch b/fix-dir-noatime-tmpfiles.patch new file mode 100644 index 00000000..714ada2f --- /dev/null +++ b/fix-dir-noatime-tmpfiles.patch @@ -0,0 +1,41 @@ +From de49f6dd99aca059da24c9afc672782f1768abd2 Mon Sep 17 00:00:00 2001 +From: Kay Sievers +Date: Wed, 11 Apr 2012 21:33:12 +0200 +Subject: [PATCH] tmpfiles: open directories with O_NOATIME to preserve + timestamp + +Before: + # stat /tmp/pulse-Du5ectm60QYM | grep 'Access: 20' + Access: 2012-04-11 21:32:48.444920237 +0200 + # systemd-tmpfiles --clean + # stat /tmp/pulse-Du5ectm60QYM | grep 'Access: 20' + Access: 2012-04-11 21:36:27.628925459 +0200 + +After: + # stat /tmp/pulse-Du5ectm60QYM | grep 'Access: 20' + Access: 2012-04-11 21:32:48.444920237 +0200 + # ./systemd-tmpfiles --clean + # stat /tmp/pulse-Du5ectm60QYM | grep 'Access: 20' + Access: 2012-04-11 21:32:48.444920237 +0200 + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=810257 +--- + src/tmpfiles.c | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/src/tmpfiles.c b/src/tmpfiles.c +index 21bf44d..09eefcf 100644 +--- a/src/tmpfiles.c ++++ b/src/tmpfiles.c +@@ -250,7 +250,7 @@ static int dir_cleanup( + DIR *sub_dir; + int q; + +- sub_dir = xopendirat(dirfd(d), dent->d_name, O_NOFOLLOW); ++ sub_dir = xopendirat(dirfd(d), dent->d_name, O_NOFOLLOW|O_NOATIME); + if (sub_dir == NULL) { + if (errno != ENOENT) { + log_error("opendir(%s/%s) failed: %m", p, dent->d_name); +-- +1.7.7 + diff --git a/improve-restart-behaviour.patch b/improve-restart-behaviour.patch new file mode 100644 index 00000000..28521824 --- /dev/null +++ b/improve-restart-behaviour.patch @@ -0,0 +1,7661 @@ +From 4cbb803c2197b6be6be709a11911bde00f6853f6 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Wed, 28 Mar 2012 00:42:27 +0200 +Subject: [PATCH 01/30] job: fix loss of ordering with restart jobs + +Suppose that foo.service/start is a job waiting on other job bar.service/start +to finish. And then foo.service/restart is enqueued (not using +--ignore-dependencies). + +Currently this makes foo.service start immediately, forgetting about the +ordering to bar.service. + +The runnability check for JOB_RESTART jobs looks only at dependencies for +stopping. That's actually correct, because restart jobs should be treated the +same as stop jobs at first. The bug is that job_run_and_invalidate() does not +treat them exactly the same as stop jobs. unit_start() gets called without +checking for the runnability of the converted JOB_START job. + +The fix is to simplify the switch in job_run_and_invalidate(). Handle +JOB_RESTART identically to JOB_STOP. +Also simplify the handling of JOB_TRY_RESTART - just convert it to JOB_RESTART +if the unit is active and let it fall through to the JOB_RESTART case. +Similarly for JOB_RELOAD_OR_START - have a fall through to JOB_START. + +In job_finish_and_invalidate() it's not necessary to check for JOB_TRY_RESTART +with JOB_DONE, because JOB_TRY_RESTART jobs will have been converted to +JOB_RESTART already. + +Speeding up the restart of services in "auto-restart" state still works as +before. + +Improves: https://bugzilla.redhat.com/show_bug.cgi?id=753586 +but it's still not perfect. With this fix the try-restart action will wait for +the restart to complete in the right order, but the optimal behaviour would be +to finish quickly (without disturbing the start job). +--- + src/job.c | 64 ++++++++++++++++++++----------------------------------------- + 1 files changed, 21 insertions(+), 43 deletions(-) + +diff --git a/src/job.c b/src/job.c +index e57286f..d43ce8e 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -387,14 +387,21 @@ int job_run_and_invalidate(Job *j) { + + switch (j->type) { + ++ case JOB_RELOAD_OR_START: ++ if (unit_active_state(j->unit) == UNIT_ACTIVE) { ++ j->type = JOB_RELOAD; ++ r = unit_reload(j->unit); ++ break; ++ } ++ j->type = JOB_START; ++ /* fall through */ ++ + case JOB_START: + r = unit_start(j->unit); + +- /* If this unit cannot be started, then simply +- * wait */ ++ /* If this unit cannot be started, then simply wait */ + if (r == -EBADR) + r = 0; +- + break; + + case JOB_VERIFY_ACTIVE: { +@@ -408,11 +415,19 @@ int job_run_and_invalidate(Job *j) { + break; + } + ++ case JOB_TRY_RESTART: ++ if (UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(j->unit))) { ++ r = -ENOEXEC; ++ break; ++ } ++ j->type = JOB_RESTART; ++ /* fall through */ ++ + case JOB_STOP: ++ case JOB_RESTART: + r = unit_stop(j->unit); + +- /* If this unit cannot stopped, then simply +- * wait. */ ++ /* If this unit cannot stopped, then simply wait. */ + if (r == -EBADR) + r = 0; + break; +@@ -421,43 +436,6 @@ int job_run_and_invalidate(Job *j) { + r = unit_reload(j->unit); + break; + +- case JOB_RELOAD_OR_START: +- if (unit_active_state(j->unit) == UNIT_ACTIVE) { +- j->type = JOB_RELOAD; +- r = unit_reload(j->unit); +- } else { +- j->type = JOB_START; +- r = unit_start(j->unit); +- +- if (r == -EBADR) +- r = 0; +- } +- break; +- +- case JOB_RESTART: { +- UnitActiveState t = unit_active_state(j->unit); +- if (t == UNIT_INACTIVE || t == UNIT_FAILED || t == UNIT_ACTIVATING) { +- j->type = JOB_START; +- r = unit_start(j->unit); +- } else +- r = unit_stop(j->unit); +- break; +- } +- +- case JOB_TRY_RESTART: { +- UnitActiveState t = unit_active_state(j->unit); +- if (t == UNIT_INACTIVE || t == UNIT_FAILED || t == UNIT_DEACTIVATING) +- r = -ENOEXEC; +- else if (t == UNIT_ACTIVATING) { +- j->type = JOB_START; +- r = unit_start(j->unit); +- } else { +- j->type = JOB_RESTART; +- r = unit_stop(j->unit); +- } +- break; +- } +- + default: + assert_not_reached("Unknown job type"); + } +@@ -536,7 +514,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) { + job_add_to_dbus_queue(j); + + /* Patch restart jobs so that they become normal start jobs */ +- if (result == JOB_DONE && (j->type == JOB_RESTART || j->type == JOB_TRY_RESTART)) { ++ if (result == JOB_DONE && j->type == JOB_RESTART) { + + log_debug("Converting job %s/%s -> %s/%s", + j->unit->id, job_type_to_string(j->type), +-- +1.7.7 + + +From 8166db595346260a3e36206249f7e82bc9ad7406 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Wed, 28 Mar 2012 01:26:04 +0200 +Subject: [PATCH 02/30] job: add debug prints where job type gets changed + +Conflicts: + + src/job.c +--- + src/job.c | 14 +++++++++----- + 1 files changed, 9 insertions(+), 5 deletions(-) + +diff --git a/src/job.c b/src/job.c +index d43ce8e..6a4d8a7 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -355,6 +355,14 @@ bool job_is_runnable(Job *j) { + return true; + } + ++static void job_change_type(Job *j, JobType newtype) { ++ log_debug("Converting job %s/%s -> %s/%s", ++ j->unit->id, job_type_to_string(j->type), ++ j->unit->id, job_type_to_string(newtype)); ++ ++ j->type = newtype; ++} ++ + int job_run_and_invalidate(Job *j) { + int r; + uint32_t id; +@@ -516,12 +524,8 @@ int job_finish_and_invalidate(Job *j, JobResult result) { + /* Patch restart jobs so that they become normal start jobs */ + if (result == JOB_DONE && j->type == JOB_RESTART) { + +- log_debug("Converting job %s/%s -> %s/%s", +- j->unit->id, job_type_to_string(j->type), +- j->unit->id, job_type_to_string(JOB_START)); +- ++ job_change_type(j, JOB_START); + j->state = JOB_WAITING; +- j->type = JOB_START; + + job_add_to_run_queue(j); + +-- +1.7.7 + + +From b0055c992aefe7512ad0f53d7c6e202d4db2bf53 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Thu, 5 Apr 2012 08:34:05 +0200 +Subject: [PATCH 03/30] job: use a lookup table for merging of job types + +It is easier to see what job_type_merge() is doing when the merging +rules are written in the form of a table. + +job_type_is_superset() contained redundant information. It can be +simplified to a simple rule: Type A is a superset of B iff merging A +with B gives A. + +Two job types are conflicting iff they are not mergeable. + +Make job_type_lookup_merge() the core function to decide the type +merging. All other job_type_*() are just short wrappers around it. +They can be inline. + +test-job-type gives the same results as before. +btw, the systemd binary is smaller by almost 1 KB. +(cherry picked from commit 348e27fedfd4cdd2238ff31a46785a70b9dc6fc0) +--- + src/job.c | 123 +++++++++++++++++------------------------------------------- + src/job.h | 29 ++++++++++++-- + 2 files changed, 60 insertions(+), 92 deletions(-) + +diff --git a/src/job.c b/src/job.c +index 6a4d8a7..da939fb 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -164,100 +164,47 @@ bool job_is_anchor(Job *j) { + return false; + } + +-static bool types_match(JobType a, JobType b, JobType c, JobType d) { +- return +- (a == c && b == d) || +- (a == d && b == c); +-} +- +-int job_type_merge(JobType *a, JobType b) { +- if (*a == b) +- return 0; +- +- /* Merging is associative! a merged with b merged with c is +- * the same as a merged with c merged with b. */ +- +- /* Mergeability is transitive! if a can be merged with b and b +- * with c then a also with c */ +- +- /* Also, if a merged with b cannot be merged with c, then +- * either a or b cannot be merged with c either */ +- +- if (types_match(*a, b, JOB_START, JOB_VERIFY_ACTIVE)) +- *a = JOB_START; +- else if (types_match(*a, b, JOB_START, JOB_RELOAD) || +- types_match(*a, b, JOB_START, JOB_RELOAD_OR_START) || +- types_match(*a, b, JOB_VERIFY_ACTIVE, JOB_RELOAD_OR_START) || +- types_match(*a, b, JOB_RELOAD, JOB_RELOAD_OR_START)) +- *a = JOB_RELOAD_OR_START; +- else if (types_match(*a, b, JOB_START, JOB_RESTART) || +- types_match(*a, b, JOB_START, JOB_TRY_RESTART) || +- types_match(*a, b, JOB_VERIFY_ACTIVE, JOB_RESTART) || +- types_match(*a, b, JOB_RELOAD, JOB_RESTART) || +- types_match(*a, b, JOB_RELOAD_OR_START, JOB_RESTART) || +- types_match(*a, b, JOB_RELOAD_OR_START, JOB_TRY_RESTART) || +- types_match(*a, b, JOB_RESTART, JOB_TRY_RESTART)) +- *a = JOB_RESTART; +- else if (types_match(*a, b, JOB_VERIFY_ACTIVE, JOB_RELOAD)) +- *a = JOB_RELOAD; +- else if (types_match(*a, b, JOB_VERIFY_ACTIVE, JOB_TRY_RESTART) || +- types_match(*a, b, JOB_RELOAD, JOB_TRY_RESTART)) +- *a = JOB_TRY_RESTART; +- else +- return -EEXIST; +- +- return 0; +-} +- +-bool job_type_is_mergeable(JobType a, JobType b) { +- return job_type_merge(&a, b) >= 0; +-} +- +-bool job_type_is_superset(JobType a, JobType b) { ++/* ++ * Merging is commutative, so imagine the matrix as symmetric. We store only ++ * its lower triangle to avoid duplication. We don't store the main diagonal, ++ * because A merged with A is simply A. ++ * ++ * Merging is associative! A merged with B merged with C is the same as ++ * A merged with C merged with B. ++ * ++ * Mergeability is transitive! If A can be merged with B and B with C then ++ * A also with C. ++ * ++ * Also, if A merged with B cannot be merged with C, then either A or B cannot ++ * be merged with C either. ++ */ ++static const JobType job_merging_table[] = { ++/* What \ With * JOB_START JOB_VERIFY_ACTIVE JOB_STOP JOB_RELOAD JOB_RELOAD_OR_START JOB_RESTART JOB_TRY_RESTART */ ++/************************************************************************************************************************************/ ++/*JOB_START */ ++/*JOB_VERIFY_ACTIVE */ JOB_START, ++/*JOB_STOP */ -1, -1, ++/*JOB_RELOAD */ JOB_RELOAD_OR_START, JOB_RELOAD, -1, ++/*JOB_RELOAD_OR_START*/ JOB_RELOAD_OR_START, JOB_RELOAD_OR_START, -1, JOB_RELOAD_OR_START, ++/*JOB_RESTART */ JOB_RESTART, JOB_RESTART, -1, JOB_RESTART, JOB_RESTART, ++/*JOB_TRY_RESTART */ JOB_RESTART, JOB_TRY_RESTART, -1, JOB_TRY_RESTART, JOB_RESTART, JOB_RESTART, ++}; + +- /* Checks whether operation a is a "superset" of b in its +- * actions */ ++JobType job_type_lookup_merge(JobType a, JobType b) { ++ assert_cc(ELEMENTSOF(job_merging_table) == _JOB_TYPE_MAX * (_JOB_TYPE_MAX - 1) / 2); ++ assert(a >= 0 && a < _JOB_TYPE_MAX); ++ assert(b >= 0 && b < _JOB_TYPE_MAX); + + if (a == b) +- return true; +- +- switch (a) { +- case JOB_START: +- return b == JOB_VERIFY_ACTIVE; +- +- case JOB_RELOAD: +- return +- b == JOB_VERIFY_ACTIVE; +- +- case JOB_RELOAD_OR_START: +- return +- b == JOB_RELOAD || +- b == JOB_START || +- b == JOB_VERIFY_ACTIVE; +- +- case JOB_RESTART: +- return +- b == JOB_START || +- b == JOB_VERIFY_ACTIVE || +- b == JOB_RELOAD || +- b == JOB_RELOAD_OR_START || +- b == JOB_TRY_RESTART; +- +- case JOB_TRY_RESTART: +- return +- b == JOB_VERIFY_ACTIVE || +- b == JOB_RELOAD; +- default: +- return false; ++ return a; + ++ if (a < b) { ++ JobType tmp = a; ++ a = b; ++ b = tmp; + } +-} +- +-bool job_type_is_conflicting(JobType a, JobType b) { +- assert(a >= 0 && a < _JOB_TYPE_MAX); +- assert(b >= 0 && b < _JOB_TYPE_MAX); + +- return (a == JOB_STOP) != (b == JOB_STOP); ++ return job_merging_table[(a - 1) * a / 2 + b]; + } + + bool job_type_is_redundant(JobType a, UnitActiveState b) { +diff --git a/src/job.h b/src/job.h +index 2121426..60a43e0 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -24,6 +24,7 @@ + + #include + #include ++#include + + typedef struct Job Job; + typedef struct JobDependency JobDependency; +@@ -37,6 +38,7 @@ typedef enum JobResult JobResult; + #include "hashmap.h" + #include "list.h" + ++/* Be careful when changing the job types! Adjust job_merging_table[] accordingly! */ + enum JobType { + JOB_START, /* if a unit does not support being started, we'll just wait until it becomes active */ + JOB_VERIFY_ACTIVE, +@@ -145,10 +147,29 @@ bool job_is_anchor(Job *j); + + int job_merge(Job *j, Job *other); + +-int job_type_merge(JobType *a, JobType b); +-bool job_type_is_mergeable(JobType a, JobType b); +-bool job_type_is_superset(JobType a, JobType b); +-bool job_type_is_conflicting(JobType a, JobType b); ++JobType job_type_lookup_merge(JobType a, JobType b); ++ ++static inline int job_type_merge(JobType *a, JobType b) { ++ JobType t = job_type_lookup_merge(*a, b); ++ if (t < 0) ++ return -EEXIST; ++ *a = t; ++ return 0; ++} ++ ++static inline bool job_type_is_mergeable(JobType a, JobType b) { ++ return job_type_lookup_merge(a, b) >= 0; ++} ++ ++static inline bool job_type_is_conflicting(JobType a, JobType b) { ++ return !job_type_is_mergeable(a, b); ++} ++ ++static inline bool job_type_is_superset(JobType a, JobType b) { ++ /* Checks whether operation a is a "superset" of b in its actions */ ++ return a == job_type_lookup_merge(a, b); ++} ++ + bool job_type_is_redundant(JobType a, UnitActiveState b); + + bool job_is_runnable(Job *j); +-- +1.7.7 + + +From 5503c3419fbcceebb6f8e94c291457adb260537b Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Thu, 19 Apr 2012 23:20:34 +0200 +Subject: [PATCH 04/30] job: allow job_free() only on already unlinked jobs + +job_free() is IMO too helpful when it unlinks the job from the transaction. +The callers should ensure the job is already unlinked before freeing. +The added assertions check if anyone gets it wrong. +(cherry picked from commit 02a3bcc6b4372ca50c0a62b193f9a75b988ffa69) +--- + src/job.c | 6 ++++-- + src/manager.c | 11 ++++++++--- + src/manager.h | 2 -- + 3 files changed, 12 insertions(+), 7 deletions(-) + +diff --git a/src/job.c b/src/job.c +index da939fb..8e4d93e 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -71,8 +71,10 @@ void job_free(Job *j) { + j->installed = false; + } + +- /* Detach from next 'smaller' objects */ +- manager_transaction_unlink_job(j->manager, j, true); ++ assert(!j->transaction_prev); ++ assert(!j->transaction_next); ++ assert(!j->subject_list); ++ assert(!j->object_list); + + if (j->in_run_queue) + LIST_REMOVE(Job, run_queue, j->manager->run_queue, j); +diff --git a/src/manager.c b/src/manager.c +index 74bd740..c77b5d2 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -637,13 +637,15 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { + return r; + } + ++static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies); ++ + static void transaction_delete_job(Manager *m, Job *j, bool delete_dependencies) { + assert(m); + assert(j); + + /* Deletes one job from the transaction */ + +- manager_transaction_unlink_job(m, j, delete_dependencies); ++ transaction_unlink_job(m, j, delete_dependencies); + + if (!j->installed) + job_free(j); +@@ -685,8 +687,10 @@ static void transaction_abort(Manager *m) { + while ((j = hashmap_first(m->transaction_jobs))) + if (j->installed) + transaction_delete_job(m, j, true); +- else ++ else { ++ transaction_unlink_job(m, j, true); + job_free(j); ++ } + + assert(hashmap_isempty(m->transaction_jobs)); + +@@ -1415,6 +1419,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o + LIST_PREPEND(Job, transaction, f, j); + + if (hashmap_replace(m->transaction_jobs, unit, f) < 0) { ++ LIST_REMOVE(Job, transaction, f, j); + job_free(j); + return NULL; + } +@@ -1427,7 +1432,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o + return j; + } + +-void manager_transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) { ++static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) { + assert(m); + assert(j); + +diff --git a/src/manager.h b/src/manager.h +index a9d08f0..64d8d0e 100644 +--- a/src/manager.h ++++ b/src/manager.h +@@ -254,8 +254,6 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode + void manager_dump_units(Manager *s, FILE *f, const char *prefix); + void manager_dump_jobs(Manager *s, FILE *f, const char *prefix); + +-void manager_transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies); +- + void manager_clear_jobs(Manager *m); + + unsigned manager_dispatch_load_queue(Manager *m); +-- +1.7.7 + + +From c2c46edd09f2d0eefc127d22fd38312bf2bed01a Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Thu, 19 Apr 2012 23:23:04 +0200 +Subject: [PATCH 05/30] manager: simplify transaction_abort() + +This is equivalent. +(cherry picked from commit 121b3b318042b7fd67ac96601971c1c2f9b77be5) +--- + src/manager.c | 7 +------ + 1 files changed, 1 insertions(+), 6 deletions(-) + +diff --git a/src/manager.c b/src/manager.c +index c77b5d2..cac1fe8 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -685,12 +685,7 @@ static void transaction_abort(Manager *m) { + assert(m); + + while ((j = hashmap_first(m->transaction_jobs))) +- if (j->installed) +- transaction_delete_job(m, j, true); +- else { +- transaction_unlink_job(m, j, true); +- job_free(j); +- } ++ transaction_delete_job(m, j, true); + + assert(hashmap_isempty(m->transaction_jobs)); + +-- +1.7.7 + + +From 4eb87c1148bd48b5d121297f72aa47eb39482ad3 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 10:21:37 +0200 +Subject: [PATCH 06/30] job: job_uninstall() + +Split the uninstallation of the job from job_free() into a separate function. +Adjust the callers. + +job_free() now only works on unlinked and uninstalled jobs. This enforces clear +thinking about job lifetimes. +(cherry picked from commit 97e7d748d1bf26790fc3b2607885f4ac8c4ecf3a) +--- + src/job.c | 25 ++++++++++++++----------- + src/job.h | 1 + + src/manager.c | 8 ++++++-- + src/unit.c | 7 +++++-- + 4 files changed, 26 insertions(+), 15 deletions(-) + +diff --git a/src/job.c b/src/job.c +index 8e4d93e..35e358d 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -55,22 +55,24 @@ Job* job_new(Manager *m, JobType type, Unit *unit) { + return j; + } + +-void job_free(Job *j) { +- assert(j); +- ++void job_uninstall(Job *j) { ++ assert(j->installed); + /* Detach from next 'bigger' objects */ +- if (j->installed) { +- bus_job_send_removed_signal(j); + +- if (j->unit->job == j) { +- j->unit->job = NULL; +- unit_add_to_gc_queue(j->unit); +- } ++ bus_job_send_removed_signal(j); + +- hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); +- j->installed = false; ++ if (j->unit->job == j) { ++ j->unit->job = NULL; ++ unit_add_to_gc_queue(j->unit); + } + ++ hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); ++ j->installed = false; ++} ++ ++void job_free(Job *j) { ++ assert(j); ++ assert(!j->installed); + assert(!j->transaction_prev); + assert(!j->transaction_next); + assert(!j->subject_list); +@@ -491,6 +493,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) { + + u = j->unit; + t = j->type; ++ job_uninstall(j); + job_free(j); + + job_print_status_message(u, t, result); +diff --git a/src/job.h b/src/job.h +index 60a43e0..ed375fa 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -137,6 +137,7 @@ struct Job { + }; + + Job* job_new(Manager *m, JobType type, Unit *unit); ++void job_uninstall(Job *j); + void job_free(Job *job); + void job_dump(Job *j, FILE*f, const char *prefix); + +diff --git a/src/manager.c b/src/manager.c +index cac1fe8..f8e5d64 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -1249,13 +1249,17 @@ static int transaction_apply(Manager *m, JobMode mode) { + } + + while ((j = hashmap_steal_first(m->transaction_jobs))) { ++ Job *uj; + if (j->installed) { + /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ + continue; + } + +- if (j->unit->job) +- job_free(j->unit->job); ++ uj = j->unit->job; ++ if (uj) { ++ job_uninstall(uj); ++ job_free(uj); ++ } + + j->unit->job = j; + j->installed = true; +diff --git a/src/unit.c b/src/unit.c +index 9e33701..1949995 100644 +--- a/src/unit.c ++++ b/src/unit.c +@@ -352,8 +352,11 @@ void unit_free(Unit *u) { + SET_FOREACH(t, u->names, i) + hashmap_remove_value(u->manager->units, t, u); + +- if (u->job) +- job_free(u->job); ++ if (u->job) { ++ Job *j = u->job; ++ job_uninstall(j); ++ job_free(j); ++ } + + for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) + bidi_set_free(u, u->dependencies[d]); +-- +1.7.7 + + +From b95d521e3816800bf43d5e11cef58b69b360233d Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 10:22:07 +0200 +Subject: [PATCH 07/30] manager: Transaction as an object + +This makes it obvious that transactions are short-lived. They are created in +manager_add_job() and destroyed after the application of jobs. +It also prepares for a split of the transaction code to a new source. +(cherry picked from commit 7527cb527598aaabf0ed9b38a352edb28536392a) +--- + src/job.c | 8 +- + src/job.h | 4 +- + src/manager.c | 390 +++++++++++++++++++++++++++++++-------------------------- + src/manager.h | 11 +- + 4 files changed, 225 insertions(+), 188 deletions(-) + +diff --git a/src/job.c b/src/job.c +index 35e358d..6d48748 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -97,7 +97,7 @@ void job_free(Job *j) { + free(j); + } + +-JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) { ++JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts, Transaction *tr) { + JobDependency *l; + + assert(object); +@@ -118,20 +118,20 @@ JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool + if (subject) + LIST_PREPEND(JobDependency, subject, subject->subject_list, l); + else +- LIST_PREPEND(JobDependency, subject, object->manager->transaction_anchor, l); ++ LIST_PREPEND(JobDependency, subject, tr->anchor, l); + + LIST_PREPEND(JobDependency, object, object->object_list, l); + + return l; + } + +-void job_dependency_free(JobDependency *l) { ++void job_dependency_free(JobDependency *l, Transaction *tr) { + assert(l); + + if (l->subject) + LIST_REMOVE(JobDependency, subject, l->subject->subject_list, l); + else +- LIST_REMOVE(JobDependency, subject, l->object->manager->transaction_anchor, l); ++ LIST_REMOVE(JobDependency, subject, tr->anchor, l); + + LIST_REMOVE(JobDependency, object, l->object->object_list, l); + +diff --git a/src/job.h b/src/job.h +index ed375fa..18ba64f 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -141,8 +141,8 @@ void job_uninstall(Job *j); + 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, bool conflicts); +-void job_dependency_free(JobDependency *l); ++JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts, Transaction *tr); ++void job_dependency_free(JobDependency *l, Transaction *tr); + + bool job_is_anchor(Job *j); + +diff --git a/src/manager.c b/src/manager.c +index f8e5d64..ff4249d 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -259,9 +259,6 @@ int manager_new(ManagerRunningAs running_as, Manager **_m) { + if (!(m->jobs = hashmap_new(trivial_hash_func, trivial_compare_func))) + goto fail; + +- if (!(m->transaction_jobs = hashmap_new(trivial_hash_func, trivial_compare_func))) +- goto fail; +- + if (!(m->watch_pids = hashmap_new(trivial_hash_func, trivial_compare_func))) + goto fail; + +@@ -429,14 +426,10 @@ static unsigned manager_dispatch_gc_queue(Manager *m) { + } + + static void manager_clear_jobs_and_units(Manager *m) { +- Job *j; + Unit *u; + + assert(m); + +- while ((j = hashmap_first(m->transaction_jobs))) +- job_free(j); +- + while ((u = hashmap_first(m->units))) + unit_free(u); + +@@ -449,7 +442,6 @@ static void manager_clear_jobs_and_units(Manager *m) { + assert(!m->cleanup_queue); + assert(!m->gc_queue); + +- assert(hashmap_isempty(m->transaction_jobs)); + assert(hashmap_isempty(m->jobs)); + assert(hashmap_isempty(m->units)); + } +@@ -475,7 +467,6 @@ void manager_free(Manager *m) { + + hashmap_free(m->units); + hashmap_free(m->jobs); +- hashmap_free(m->transaction_jobs); + hashmap_free(m->watch_pids); + hashmap_free(m->watch_bus); + +@@ -637,65 +628,47 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { + return r; + } + +-static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies); ++static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies); + +-static void transaction_delete_job(Manager *m, Job *j, bool delete_dependencies) { +- assert(m); ++static void transaction_delete_job(Transaction *tr, Job *j, bool delete_dependencies) { ++ assert(tr); + assert(j); + + /* Deletes one job from the transaction */ + +- transaction_unlink_job(m, j, delete_dependencies); ++ transaction_unlink_job(tr, j, delete_dependencies); + + if (!j->installed) + job_free(j); + } + +-static void transaction_delete_unit(Manager *m, Unit *u) { ++static void transaction_delete_unit(Transaction *tr, Unit *u) { + Job *j; + + /* Deletes all jobs associated with a certain unit from the + * transaction */ + +- while ((j = hashmap_get(m->transaction_jobs, u))) +- transaction_delete_job(m, j, true); +-} +- +-static void transaction_clean_dependencies(Manager *m) { +- Iterator i; +- Job *j; +- +- assert(m); +- +- /* Drops all dependencies of all installed jobs */ +- +- HASHMAP_FOREACH(j, m->jobs, i) { +- while (j->subject_list) +- job_dependency_free(j->subject_list); +- while (j->object_list) +- job_dependency_free(j->object_list); +- } +- +- assert(!m->transaction_anchor); ++ while ((j = hashmap_get(tr->jobs, u))) ++ transaction_delete_job(tr, j, true); + } + +-static void transaction_abort(Manager *m) { ++static void transaction_abort(Transaction *tr) { + Job *j; + +- assert(m); ++ assert(tr); + +- while ((j = hashmap_first(m->transaction_jobs))) +- transaction_delete_job(m, j, true); ++ while ((j = hashmap_first(tr->jobs))) ++ transaction_delete_job(tr, j, true); + +- assert(hashmap_isempty(m->transaction_jobs)); ++ assert(hashmap_isempty(tr->jobs)); + +- transaction_clean_dependencies(m); ++ assert(!tr->anchor); + } + +-static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsigned generation) { ++static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, Job *j, unsigned generation) { + JobDependency *l; + +- assert(m); ++ assert(tr); + + /* A recursive sweep through the graph that marks all units + * that matter to the anchor job, i.e. are directly or +@@ -705,7 +678,7 @@ static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsi + if (j) + l = j->subject_list; + else +- l = m->transaction_anchor; ++ l = tr->anchor; + + LIST_FOREACH(subject, l, l) { + +@@ -720,11 +693,11 @@ static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsi + l->object->matters_to_anchor = true; + l->object->generation = generation; + +- transaction_find_jobs_that_matter_to_anchor(m, l->object, generation); ++ transaction_find_jobs_that_matter_to_anchor(tr, l->object, generation); + } + } + +-static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, JobType t) { ++static void transaction_merge_and_delete_job(Transaction *tr, Job *j, Job *other, JobType t) { + JobDependency *l, *last; + + assert(j); +@@ -775,7 +748,7 @@ static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, Job + /* Kill the other job */ + other->subject_list = NULL; + other->object_list = NULL; +- transaction_delete_job(m, other, true); ++ transaction_delete_job(tr, other, true); + } + static bool job_is_conflicted_by(Job *j) { + JobDependency *l; +@@ -792,7 +765,7 @@ static bool job_is_conflicted_by(Job *j) { + return false; + } + +-static int delete_one_unmergeable_job(Manager *m, Job *j) { ++static int delete_one_unmergeable_job(Transaction *tr, Job *j) { + Job *k; + + assert(j); +@@ -853,23 +826,23 @@ static int delete_one_unmergeable_job(Manager *m, Job *j) { + + /* Ok, we can drop one, so let's do so. */ + log_debug("Fixing conflicting jobs by deleting job %s/%s", d->unit->id, job_type_to_string(d->type)); +- transaction_delete_job(m, d, true); ++ transaction_delete_job(tr, d, true); + return 0; + } + + return -EINVAL; + } + +-static int transaction_merge_jobs(Manager *m, DBusError *e) { ++static int transaction_merge_jobs(Transaction *tr, DBusError *e) { + Job *j; + Iterator i; + int r; + +- assert(m); ++ assert(tr); + + /* First step, check whether any of the jobs for one specific + * task conflict. If so, try to drop one of them. */ +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + JobType t; + Job *k; + +@@ -882,7 +855,8 @@ static int transaction_merge_jobs(Manager *m, DBusError *e) { + * action. Let's see if we can get rid of one + * of them */ + +- if ((r = delete_one_unmergeable_job(m, j)) >= 0) ++ r = delete_one_unmergeable_job(tr, j); ++ if (r >= 0) + /* Ok, we managed to drop one, now + * let's ask our callers to call us + * again after garbage collecting */ +@@ -896,7 +870,7 @@ static int transaction_merge_jobs(Manager *m, DBusError *e) { + } + + /* Second step, merge the jobs. */ +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + JobType t = j->type; + Job *k; + +@@ -910,14 +884,14 @@ static int transaction_merge_jobs(Manager *m, DBusError *e) { + + while ((k = j->transaction_next)) { + if (j->installed) { +- transaction_merge_and_delete_job(m, k, j, t); ++ transaction_merge_and_delete_job(tr, k, j, t); + j = k; + } else +- transaction_merge_and_delete_job(m, j, k, t); ++ transaction_merge_and_delete_job(tr, j, k, t); + } + + if (j->unit->job && !j->installed) +- transaction_merge_and_delete_job(m, j, j->unit->job, t); ++ transaction_merge_and_delete_job(tr, j, j->unit->job, t); + + assert(!j->transaction_next); + assert(!j->transaction_prev); +@@ -926,10 +900,10 @@ static int transaction_merge_jobs(Manager *m, DBusError *e) { + return 0; + } + +-static void transaction_drop_redundant(Manager *m) { ++static void transaction_drop_redundant(Transaction *tr) { + bool again; + +- assert(m); ++ assert(tr); + + /* Goes through the transaction and removes all jobs that are + * a noop */ +@@ -940,7 +914,7 @@ static void transaction_drop_redundant(Manager *m) { + + again = false; + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + bool changes_something = false; + Job *k; + +@@ -959,7 +933,7 @@ static void transaction_drop_redundant(Manager *m) { + continue; + + /* log_debug("Found redundant job %s/%s, dropping.", j->unit->id, job_type_to_string(j->type)); */ +- transaction_delete_job(m, j, false); ++ transaction_delete_job(tr, j, false); + again = true; + break; + } +@@ -981,12 +955,12 @@ static bool unit_matters_to_anchor(Unit *u, Job *j) { + return false; + } + +-static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned generation, DBusError *e) { ++static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsigned generation, DBusError *e) { + Iterator i; + Unit *u; + int r; + +- assert(m); ++ assert(tr); + assert(j); + assert(!j->transaction_prev); + +@@ -1033,7 +1007,7 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned + + if (delete) { + log_warning("Breaking ordering cycle by deleting job %s/%s", delete->unit->id, job_type_to_string(delete->type)); +- transaction_delete_unit(m, delete->unit); ++ transaction_delete_unit(tr, delete->unit); + return -EAGAIN; + } + +@@ -1056,15 +1030,18 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned + Job *o; + + /* Is there a job for this unit? */ +- if (!(o = hashmap_get(m->transaction_jobs, u))) +- ++ 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? */ +- if (!(o = u->job)) ++ o = u->job; ++ if (!o) + continue; ++ } + +- if ((r = transaction_verify_order_one(m, o, j, generation, e)) < 0) ++ r = transaction_verify_order_one(tr, o, j, generation, e); ++ if (r < 0) + return r; + } + +@@ -1075,13 +1052,13 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned + return 0; + } + +-static int transaction_verify_order(Manager *m, unsigned *generation, DBusError *e) { ++static int transaction_verify_order(Transaction *tr, unsigned *generation, DBusError *e) { + Job *j; + int r; + Iterator i; + unsigned g; + +- assert(m); ++ assert(tr); + assert(generation); + + /* Check if the ordering graph is cyclic. If it is, try to fix +@@ -1089,17 +1066,17 @@ static int transaction_verify_order(Manager *m, unsigned *generation, DBusError + + g = (*generation)++; + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) +- if ((r = transaction_verify_order_one(m, j, NULL, g, e)) < 0) ++ HASHMAP_FOREACH(j, tr->jobs, i) ++ if ((r = transaction_verify_order_one(tr, j, NULL, g, e)) < 0) + return r; + + return 0; + } + +-static void transaction_collect_garbage(Manager *m) { ++static void transaction_collect_garbage(Transaction *tr) { + bool again; + +- assert(m); ++ assert(tr); + + /* Drop jobs that are not required by any other job */ + +@@ -1109,7 +1086,7 @@ static void transaction_collect_garbage(Manager *m) { + + again = false; + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + if (j->object_list) { + /* log_debug("Keeping job %s/%s because of %s/%s", */ + /* j->unit->id, job_type_to_string(j->type), */ +@@ -1119,7 +1096,7 @@ static void transaction_collect_garbage(Manager *m) { + } + + /* log_debug("Garbage collecting job %s/%s", j->unit->id, job_type_to_string(j->type)); */ +- transaction_delete_job(m, j, true); ++ transaction_delete_job(tr, j, true); + again = true; + break; + } +@@ -1127,16 +1104,16 @@ static void transaction_collect_garbage(Manager *m) { + } while (again); + } + +-static int transaction_is_destructive(Manager *m, DBusError *e) { ++static int transaction_is_destructive(Transaction *tr, DBusError *e) { + Iterator i; + Job *j; + +- assert(m); ++ assert(tr); + + /* Checks whether applying this transaction means that + * existing jobs would be replaced */ + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + + /* Assume merged */ + assert(!j->transaction_prev); +@@ -1154,9 +1131,9 @@ static int transaction_is_destructive(Manager *m, DBusError *e) { + return 0; + } + +-static void transaction_minimize_impact(Manager *m) { ++static void transaction_minimize_impact(Transaction *tr) { + bool again; +- assert(m); ++ assert(tr); + + /* Drops all unnecessary jobs that reverse already active jobs + * or that stop a running service. */ +@@ -1167,7 +1144,7 @@ static void transaction_minimize_impact(Manager *m) { + + again = false; + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + LIST_FOREACH(transaction, j, j) { + bool stops_running_service, changes_existing_job; + +@@ -1198,7 +1175,7 @@ static void transaction_minimize_impact(Manager *m) { + /* Ok, let's get rid of this */ + log_debug("Deleting %s/%s to minimize impact.", j->unit->id, job_type_to_string(j->type)); + +- transaction_delete_job(m, j, true); ++ transaction_delete_job(tr, j, true); + again = true; + break; + } +@@ -1210,7 +1187,7 @@ static void transaction_minimize_impact(Manager *m) { + } while (again); + } + +-static int transaction_apply(Manager *m, JobMode mode) { ++static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + Iterator i; + Job *j; + int r; +@@ -1225,7 +1202,7 @@ static int transaction_apply(Manager *m, JobMode mode) { + HASHMAP_FOREACH(j, m->jobs, i) { + assert(j->installed); + +- if (hashmap_get(m->transaction_jobs, j->unit)) ++ if (hashmap_get(tr->jobs, j->unit)) + continue; + + /* 'j' itself is safe to remove, but if other jobs +@@ -1236,7 +1213,7 @@ static int transaction_apply(Manager *m, JobMode mode) { + } + } + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + /* Assume merged */ + assert(!j->transaction_prev); + assert(!j->transaction_next); +@@ -1244,11 +1221,12 @@ static int transaction_apply(Manager *m, JobMode mode) { + if (j->installed) + continue; + +- if ((r = hashmap_put(m->jobs, UINT32_TO_PTR(j->id), j)) < 0) ++ r = hashmap_put(m->jobs, UINT32_TO_PTR(j->id), j); ++ if (r < 0) + goto rollback; + } + +- while ((j = hashmap_steal_first(m->transaction_jobs))) { ++ while ((j = hashmap_steal_first(tr->jobs))) { + Job *uj; + if (j->installed) { + /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ +@@ -1271,6 +1249,9 @@ static int transaction_apply(Manager *m, JobMode mode) { + assert(!j->transaction_next); + assert(!j->transaction_prev); + ++ /* Clean the job dependencies */ ++ transaction_unlink_job(tr, j, false); ++ + job_add_to_run_queue(j); + job_add_to_dbus_queue(j); + job_start_timer(j); +@@ -1278,14 +1259,13 @@ static int transaction_apply(Manager *m, JobMode mode) { + log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); + } + +- /* As last step, kill all remaining job dependencies. */ +- transaction_clean_dependencies(m); ++ assert(!tr->anchor); + + return 0; + + rollback: + +- HASHMAP_FOREACH(j, m->transaction_jobs, i) { ++ HASHMAP_FOREACH(j, tr->jobs, i) { + if (j->installed) + continue; + +@@ -1295,41 +1275,42 @@ rollback: + return r; + } + +-static int transaction_activate(Manager *m, JobMode mode, DBusError *e) { ++static int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e) { + int r; + unsigned generation = 1; + +- assert(m); ++ assert(tr); + +- /* This applies the changes recorded in transaction_jobs to ++ /* This applies the changes recorded in tr->jobs to + * the actual list of jobs, if possible. */ + + /* First step: figure out which jobs matter */ +- transaction_find_jobs_that_matter_to_anchor(m, NULL, generation++); ++ transaction_find_jobs_that_matter_to_anchor(tr, 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. */ + if (mode == JOB_FAIL) +- transaction_minimize_impact(m); ++ transaction_minimize_impact(tr); + + /* Third step: Drop redundant jobs */ +- transaction_drop_redundant(m); ++ transaction_drop_redundant(tr); + + for (;;) { + /* Fourth step: Let's remove unneeded jobs that might + * be lurking. */ + if (mode != JOB_ISOLATE) +- transaction_collect_garbage(m); ++ transaction_collect_garbage(tr); + + /* Fifth step: verify order makes sense and correct + * cycles if necessary and possible */ +- if ((r = transaction_verify_order(m, &generation, e)) >= 0) ++ r = transaction_verify_order(tr, &generation, e); ++ if (r >= 0) + break; + + if (r != -EAGAIN) { + log_warning("Requested transaction contains an unfixable cyclic ordering dependency: %s", bus_error(e, r)); +- goto rollback; ++ return r; + } + + /* Let's see if the resulting transaction ordering +@@ -1340,60 +1321,60 @@ static int transaction_activate(Manager *m, JobMode mode, DBusError *e) { + /* Sixth step: let's drop unmergeable entries if + * necessary and possible, merge entries we can + * merge */ +- if ((r = transaction_merge_jobs(m, e)) >= 0) ++ r = transaction_merge_jobs(tr, e); ++ if (r >= 0) + break; + + if (r != -EAGAIN) { + log_warning("Requested transaction contains unmergeable jobs: %s", bus_error(e, r)); +- goto rollback; ++ return r; + } + + /* Seventh step: an entry got dropped, let's garbage + * collect its dependencies. */ + if (mode != JOB_ISOLATE) +- transaction_collect_garbage(m); ++ transaction_collect_garbage(tr); + + /* Let's see if the resulting transaction still has + * unmergeable entries ... */ + } + + /* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */ +- transaction_drop_redundant(m); ++ transaction_drop_redundant(tr); + + /* Ninth step: check whether we can actually apply this */ +- if (mode == JOB_FAIL) +- if ((r = transaction_is_destructive(m, e)) < 0) { ++ if (mode == JOB_FAIL) { ++ r = transaction_is_destructive(tr, e); ++ if (r < 0) { + log_notice("Requested transaction contradicts existing jobs: %s", bus_error(e, r)); +- goto rollback; ++ return r; + } ++ } + + /* Tenth step: apply changes */ +- if ((r = transaction_apply(m, mode)) < 0) { ++ r = transaction_apply(tr, m, mode); ++ if (r < 0) { + log_warning("Failed to apply transaction: %s", strerror(-r)); +- goto rollback; ++ return r; + } + +- assert(hashmap_isempty(m->transaction_jobs)); +- assert(!m->transaction_anchor); ++ assert(hashmap_isempty(tr->jobs)); ++ assert(!tr->anchor); + + return 0; +- +-rollback: +- transaction_abort(m); +- return r; + } + +-static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool override, bool *is_new) { ++static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, bool override, bool *is_new) { + Job *j, *f; + +- assert(m); ++ assert(tr); + assert(unit); + + /* Looks for an existing prospective job and returns that. If + * it doesn't exist it is created and added to the prospective + * jobs list. */ + +- f = hashmap_get(m->transaction_jobs, unit); ++ f = hashmap_get(tr->jobs, unit); + + LIST_FOREACH(transaction, j, f) { + assert(j->unit == unit); +@@ -1407,8 +1388,11 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o + + if (unit->job && unit->job->type == type) + j = unit->job; +- else if (!(j = job_new(m, type, unit))) +- return NULL; ++ else { ++ j = job_new(unit->manager, type, unit); ++ if (!j) ++ return NULL; ++ } + + j->generation = 0; + j->marker = NULL; +@@ -1417,7 +1401,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o + + LIST_PREPEND(Job, transaction, f, j); + +- if (hashmap_replace(m->transaction_jobs, unit, f) < 0) { ++ if (hashmap_replace(tr->jobs, unit, f) < 0) { + LIST_REMOVE(Job, transaction, f, j); + job_free(j); + return NULL; +@@ -1431,16 +1415,16 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o + return j; + } + +-static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) { +- assert(m); ++static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies) { ++ assert(tr); + assert(j); + + if (j->transaction_prev) + j->transaction_prev->transaction_next = j->transaction_next; + else if (j->transaction_next) +- hashmap_replace(m->transaction_jobs, j->unit, j->transaction_next); ++ hashmap_replace(tr->jobs, j->unit, j->transaction_next); + else +- hashmap_remove_value(m->transaction_jobs, j->unit, j); ++ hashmap_remove_value(tr->jobs, j->unit, j); + + if (j->transaction_next) + j->transaction_next->transaction_prev = j->transaction_prev; +@@ -1448,24 +1432,24 @@ static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) + j->transaction_prev = j->transaction_next = NULL; + + while (j->subject_list) +- job_dependency_free(j->subject_list); ++ job_dependency_free(j->subject_list, tr); + + while (j->object_list) { + Job *other = j->object_list->matters ? j->object_list->subject : NULL; + +- job_dependency_free(j->object_list); ++ job_dependency_free(j->object_list, tr); + + if (other && delete_dependencies) { + log_debug("Deleting job %s/%s as dependency of job %s/%s", + other->unit->id, job_type_to_string(other->type), + j->unit->id, job_type_to_string(j->type)); +- transaction_delete_job(m, other, delete_dependencies); ++ transaction_delete_job(tr, other, delete_dependencies); + } + } + } + + static int transaction_add_job_and_dependencies( +- Manager *m, ++ Transaction *tr, + JobType type, + Unit *unit, + Job *by, +@@ -1482,7 +1466,7 @@ static int transaction_add_job_and_dependencies( + int r; + bool is_new; + +- assert(m); ++ assert(tr); + assert(type < _JOB_TYPE_MAX); + assert(unit); + +@@ -1519,13 +1503,14 @@ static int transaction_add_job_and_dependencies( + } + + /* First add the job. */ +- if (!(ret = transaction_add_one_job(m, type, unit, override, &is_new))) ++ ret = transaction_add_one_job(tr, type, unit, override, &is_new); ++ if (!ret) + return -ENOMEM; + + ret->ignore_order = ret->ignore_order || ignore_order; + + /* Then, add a link to the job. */ +- if (!job_dependency_new(by, ret, matters, conflicts)) ++ if (!job_dependency_new(by, ret, matters, conflicts, tr)) + return -ENOMEM; + + if (is_new && !ignore_requirements) { +@@ -1534,120 +1519,136 @@ static int transaction_add_job_and_dependencies( + /* If we are following some other unit, make sure we + * add all dependencies of everybody following. */ + if (unit_following_set(ret->unit, &following) > 0) { +- SET_FOREACH(dep, following, i) +- if ((r = transaction_add_job_and_dependencies(m, type, dep, ret, false, override, false, false, ignore_order, e, NULL)) < 0) { ++ SET_FOREACH(dep, following, i) { ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { + log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + + if (e) + dbus_error_free(e); + } ++ } + + set_free(following); + } + + /* Finally, recursively add in all dependencies. */ + if (type == JOB_START || type == JOB_RELOAD_OR_START) { +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRES], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, true, override, false, false, ignore_order, e, NULL)) < 0) { ++ 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, NULL); ++ if (r < 0) { + if (r != -EBADR) + goto fail; + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_BIND_TO], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, true, override, false, false, ignore_order, e, NULL)) < 0) { +- ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_BIND_TO], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { + if (r != -EBADR) + goto fail; + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRES_OVERRIDABLE], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, !override, override, false, false, ignore_order, e, NULL)) < 0) { ++ 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, NULL); ++ if (r < 0) { + log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_WANTS], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_START, dep, ret, false, false, false, false, ignore_order, e, NULL)) < 0) { ++ 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, NULL); ++ if (r < 0) { + log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, false, ignore_order, e, NULL)) < 0) { +- ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { + if (r != -EBADR) + goto fail; + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE_OVERRIDABLE], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_VERIFY_ACTIVE, dep, ret, !override, override, false, false, ignore_order, e, NULL)) < 0) { ++ 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, NULL); ++ if (r < 0) { + log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTS], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, dep, ret, true, override, true, false, ignore_order, e, NULL)) < 0) { +- ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTS], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, true, override, true, false, ignore_order, e, NULL); ++ if (r < 0) { + if (r != -EBADR) + goto fail; + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) +- if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, dep, ret, false, override, false, false, ignore_order, e, NULL)) < 0) { ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { + log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + + if (e) + dbus_error_free(e); + } ++ } + + } + + if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) { + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRED_BY], i) +- if ((r = transaction_add_job_and_dependencies(m, type, dep, ret, true, override, false, false, ignore_order, e, NULL)) < 0) { +- ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRED_BY], i) { ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { + if (r != -EBADR) + goto fail; + + if (e) + dbus_error_free(e); + } ++ } + +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_BOUND_BY], i) +- if ((r = transaction_add_job_and_dependencies(m, type, dep, ret, true, override, false, false, ignore_order, e, NULL)) < 0) { +- ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_BOUND_BY], i) { ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { + if (r != -EBADR) + goto fail; + + if (e) + dbus_error_free(e); + } ++ } + } + + if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START) { + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_PROPAGATE_RELOAD_TO], i) { +- r = transaction_add_job_and_dependencies(m, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e, NULL); +- ++ r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e, NULL); + if (r < 0) { + log_warning("Cannot add dependency reload job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + +@@ -1669,12 +1670,13 @@ fail: + return r; + } + +-static int transaction_add_isolate_jobs(Manager *m) { ++static int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { + Iterator i; + Unit *u; + char *k; + int r; + ++ assert(tr); + assert(m); + + HASHMAP_FOREACH_KEY(u, k, m->units, i) { +@@ -1691,19 +1693,43 @@ static int transaction_add_isolate_jobs(Manager *m) { + continue; + + /* Is there already something listed for this? */ +- if (hashmap_get(m->transaction_jobs, u)) ++ if (hashmap_get(tr->jobs, u)) + continue; + +- if ((r = transaction_add_job_and_dependencies(m, JOB_STOP, u, NULL, true, false, false, false, false, NULL, NULL)) < 0) ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, NULL, true, false, false, false, false, NULL, NULL); ++ if (r < 0) + log_warning("Cannot add isolate job for unit %s, ignoring: %s", u->id, strerror(-r)); + } + + return 0; + } + ++static Transaction *transaction_new(void) { ++ Transaction *tr; ++ ++ tr = new0(Transaction, 1); ++ if (!tr) ++ return NULL; ++ ++ tr->jobs = hashmap_new(trivial_hash_func, trivial_compare_func); ++ if (!tr->jobs) { ++ free(tr); ++ return NULL; ++ } ++ ++ return tr; ++} ++ ++static void transaction_free(Transaction *tr) { ++ assert(hashmap_isempty(tr->jobs)); ++ hashmap_free(tr->jobs); ++ free(tr); ++} ++ + int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool override, DBusError *e, Job **_ret) { + int r; + Job *ret; ++ Transaction *tr; + + assert(m); + assert(type < _JOB_TYPE_MAX); +@@ -1722,28 +1748,38 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove + + log_debug("Trying to enqueue job %s/%s/%s", unit->id, job_type_to_string(type), job_mode_to_string(mode)); + +- if ((r = transaction_add_job_and_dependencies(m, type, unit, NULL, true, override, false, +- mode == JOB_IGNORE_DEPENDENCIES || mode == JOB_IGNORE_REQUIREMENTS, +- mode == JOB_IGNORE_DEPENDENCIES, e, &ret)) < 0) { +- transaction_abort(m); +- return r; +- } ++ tr = transaction_new(); ++ if (!tr) ++ return -ENOMEM; + +- if (mode == JOB_ISOLATE) +- if ((r = transaction_add_isolate_jobs(m)) < 0) { +- transaction_abort(m); +- return r; +- } ++ r = transaction_add_job_and_dependencies(tr, type, unit, NULL, true, override, false, ++ mode == JOB_IGNORE_DEPENDENCIES || mode == JOB_IGNORE_REQUIREMENTS, ++ mode == JOB_IGNORE_DEPENDENCIES, e, &ret); ++ if (r < 0) ++ goto tr_abort; + +- if ((r = transaction_activate(m, mode, e)) < 0) +- return r; ++ if (mode == JOB_ISOLATE) { ++ r = transaction_add_isolate_jobs(tr, m); ++ if (r < 0) ++ goto tr_abort; ++ } ++ ++ r = transaction_activate(tr, m, mode, e); ++ if (r < 0) ++ goto tr_abort; + + log_debug("Enqueued job %s/%s as %u", unit->id, job_type_to_string(type), (unsigned) ret->id); + + if (_ret) + *_ret = ret; + ++ transaction_free(tr); + return 0; ++ ++tr_abort: ++ transaction_abort(tr); ++ transaction_free(tr); ++ return r; + } + + int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, bool override, DBusError *e, Job **_ret) { +@@ -1907,8 +1943,6 @@ void manager_clear_jobs(Manager *m) { + + assert(m); + +- transaction_abort(m); +- + while ((j = hashmap_first(m->jobs))) + job_finish_and_invalidate(j, JOB_CANCELED); + } +diff --git a/src/manager.h b/src/manager.h +index 64d8d0e..b982d7e 100644 +--- a/src/manager.h ++++ b/src/manager.h +@@ -33,6 +33,7 @@ + #define MANAGER_MAX_NAMES 131072 /* 128K */ + + typedef struct Manager Manager; ++typedef struct Transaction Transaction; + typedef enum WatchType WatchType; + typedef struct Watch Watch; + +@@ -91,6 +92,12 @@ struct Watch { + #include "dbus.h" + #include "path-lookup.h" + ++struct Transaction { ++ /* Jobs to be added */ ++ Hashmap *jobs; /* Unit object => Job object list 1:1 */ ++ JobDependency *anchor; ++}; ++ + struct Manager { + /* Note that the set of units we know of is allowed to be + * inconsistent. However the subset of it that is loaded may +@@ -123,10 +130,6 @@ struct Manager { + /* Units to check when doing GC */ + LIST_HEAD(Unit, gc_queue); + +- /* Jobs to be added */ +- Hashmap *transaction_jobs; /* Unit object => Job object list 1:1 */ +- JobDependency *transaction_anchor; +- + Hashmap *watch_pids; /* pid => Unit object n:1 */ + + char *notify_socket; +-- +1.7.7 + + +From e617c9a330b60d070e3db57c1f86f3c579e03f0c Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Thu, 19 Apr 2012 23:54:11 +0200 +Subject: [PATCH 08/30] manager: split transaction.[ch] + +manager.c takes care of the main loop, unit management, signal handling, ... +transaction.c computes transactions. + +After split: +manager.c: 65 KB +transaction.c: 40 KB +(cherry picked from commit 75778e21dfeee51036d24501e39ea7398fabe502) + +Conflicts: + + Makefile.am + src/manager.c +--- + Makefile.am | 2 + + src/job.h | 1 + + src/manager.c | 1099 +---------------------------------------------------- + src/manager.h | 7 - + src/transaction.c | 1101 +++++++++++++++++++++++++++++++++++++++++++++++++++++ + src/transaction.h | 36 ++ + 6 files changed, 1141 insertions(+), 1105 deletions(-) + create mode 100644 src/transaction.c + create mode 100644 src/transaction.h + +diff --git a/Makefile.am b/Makefile.am +index 079c118..45a0b10 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -483,6 +483,7 @@ libsystemd_core_la_SOURCES = \ + src/unit.c \ + src/job.c \ + src/manager.c \ ++ src/transaction.c \ + src/path-lookup.c \ + src/load-fragment.c \ + src/service.c \ +@@ -579,6 +580,7 @@ EXTRA_DIST += \ + src/unit.h \ + src/job.h \ + src/manager.h \ ++ src/transaction.h \ + src/path-lookup.h \ + src/load-fragment.h \ + src/service.h \ +diff --git a/src/job.h b/src/job.h +index 18ba64f..efb0af9 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -34,6 +34,7 @@ typedef enum JobMode JobMode; + typedef enum JobResult JobResult; + + #include "manager.h" ++#include "transaction.h" + #include "unit.h" + #include "hashmap.h" + #include "list.h" +diff --git a/src/manager.c b/src/manager.c +index ff4249d..a129b88 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -44,6 +44,7 @@ + #include + + #include "manager.h" ++#include "transaction.h" + #include "hashmap.h" + #include "macro.h" + #include "strv.h" +@@ -628,1104 +629,6 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { + return r; + } + +-static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies); +- +-static void transaction_delete_job(Transaction *tr, Job *j, bool delete_dependencies) { +- assert(tr); +- assert(j); +- +- /* Deletes one job from the transaction */ +- +- transaction_unlink_job(tr, j, delete_dependencies); +- +- if (!j->installed) +- job_free(j); +-} +- +-static void transaction_delete_unit(Transaction *tr, Unit *u) { +- Job *j; +- +- /* Deletes all jobs associated with a certain unit from the +- * transaction */ +- +- while ((j = hashmap_get(tr->jobs, u))) +- transaction_delete_job(tr, j, true); +-} +- +-static void transaction_abort(Transaction *tr) { +- Job *j; +- +- assert(tr); +- +- while ((j = hashmap_first(tr->jobs))) +- transaction_delete_job(tr, j, true); +- +- assert(hashmap_isempty(tr->jobs)); +- +- assert(!tr->anchor); +-} +- +-static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, Job *j, unsigned generation) { +- JobDependency *l; +- +- assert(tr); +- +- /* A recursive sweep through the graph that marks all units +- * that matter to the anchor job, i.e. are directly or +- * indirectly a dependency of the anchor job via paths that +- * are fully marked as mattering. */ +- +- if (j) +- l = j->subject_list; +- else +- l = tr->anchor; +- +- LIST_FOREACH(subject, l, l) { +- +- /* This link does not matter */ +- if (!l->matters) +- continue; +- +- /* This unit has already been marked */ +- if (l->object->generation == generation) +- continue; +- +- l->object->matters_to_anchor = true; +- l->object->generation = generation; +- +- transaction_find_jobs_that_matter_to_anchor(tr, l->object, generation); +- } +-} +- +-static void transaction_merge_and_delete_job(Transaction *tr, Job *j, Job *other, JobType t) { +- JobDependency *l, *last; +- +- assert(j); +- assert(other); +- assert(j->unit == other->unit); +- assert(!j->installed); +- +- /* Merges 'other' into 'j' and then deletes j. */ +- +- j->type = t; +- j->state = JOB_WAITING; +- j->override = j->override || other->override; +- +- j->matters_to_anchor = j->matters_to_anchor || other->matters_to_anchor; +- +- /* Patch us in as new owner of the JobDependency objects */ +- last = NULL; +- LIST_FOREACH(subject, l, other->subject_list) { +- assert(l->subject == other); +- l->subject = j; +- last = l; +- } +- +- /* Merge both lists */ +- if (last) { +- last->subject_next = j->subject_list; +- if (j->subject_list) +- j->subject_list->subject_prev = last; +- j->subject_list = other->subject_list; +- } +- +- /* Patch us in as new owner of the JobDependency objects */ +- last = NULL; +- LIST_FOREACH(object, l, other->object_list) { +- assert(l->object == other); +- l->object = j; +- last = l; +- } +- +- /* Merge both lists */ +- if (last) { +- last->object_next = j->object_list; +- if (j->object_list) +- j->object_list->object_prev = last; +- j->object_list = other->object_list; +- } +- +- /* Kill the other job */ +- other->subject_list = NULL; +- other->object_list = NULL; +- transaction_delete_job(tr, 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(Transaction *tr, Job *j) { +- Job *k; +- +- assert(j); +- +- /* Tries to delete one item in the linked list +- * j->transaction_next->transaction_next->... that conflicts +- * with another one, in an attempt to make an inconsistent +- * transaction work. */ +- +- /* We rely here on the fact that if a merged with b does not +- * merge with c, either a or b merge with c neither */ +- LIST_FOREACH(transaction, j, j) +- LIST_FOREACH(transaction, k, j->transaction_next) { +- Job *d; +- +- /* Is this one mergeable? Then skip it */ +- if (job_type_is_mergeable(j->type, k->type)) +- continue; +- +- /* Ok, we found two that conflict, let's see if we can +- * drop one of them */ +- 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->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->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 +- d = j; +- +- } else if (!j->matters_to_anchor) +- d = j; +- else if (!k->matters_to_anchor) +- d = k; +- else +- return -ENOEXEC; +- +- /* Ok, we can drop one, so let's do so. */ +- log_debug("Fixing conflicting jobs by deleting job %s/%s", d->unit->id, job_type_to_string(d->type)); +- transaction_delete_job(tr, d, true); +- return 0; +- } +- +- return -EINVAL; +-} +- +-static int transaction_merge_jobs(Transaction *tr, DBusError *e) { +- Job *j; +- Iterator i; +- int r; +- +- assert(tr); +- +- /* First step, check whether any of the jobs for one specific +- * task conflict. If so, try to drop one of them. */ +- HASHMAP_FOREACH(j, tr->jobs, i) { +- JobType t; +- Job *k; +- +- t = j->type; +- LIST_FOREACH(transaction, k, j->transaction_next) { +- if (job_type_merge(&t, k->type) >= 0) +- continue; +- +- /* OK, we could not merge all jobs for this +- * action. Let's see if we can get rid of one +- * of them */ +- +- r = delete_one_unmergeable_job(tr, j); +- if (r >= 0) +- /* Ok, we managed to drop one, now +- * let's ask our callers to call us +- * again after garbage collecting */ +- return -EAGAIN; +- +- /* We couldn't merge anything. Failure */ +- dbus_set_error(e, BUS_ERROR_TRANSACTION_JOBS_CONFLICTING, "Transaction contains conflicting jobs '%s' and '%s' for %s. Probably contradicting requirement dependencies configured.", +- job_type_to_string(t), job_type_to_string(k->type), k->unit->id); +- return r; +- } +- } +- +- /* Second step, merge the jobs. */ +- HASHMAP_FOREACH(j, tr->jobs, i) { +- JobType t = j->type; +- Job *k; +- +- /* Merge all transactions */ +- LIST_FOREACH(transaction, k, j->transaction_next) +- assert_se(job_type_merge(&t, k->type) == 0); +- +- /* If an active job is mergeable, merge it too */ +- if (j->unit->job) +- job_type_merge(&t, j->unit->job->type); /* Might fail. Which is OK */ +- +- while ((k = j->transaction_next)) { +- if (j->installed) { +- transaction_merge_and_delete_job(tr, k, j, t); +- j = k; +- } else +- transaction_merge_and_delete_job(tr, j, k, t); +- } +- +- if (j->unit->job && !j->installed) +- transaction_merge_and_delete_job(tr, j, j->unit->job, t); +- +- assert(!j->transaction_next); +- assert(!j->transaction_prev); +- } +- +- return 0; +-} +- +-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; +- +- LIST_FOREACH(transaction, k, j) { +- +- if (!job_is_anchor(k) && +- (k->installed || 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; +- +- changes_something = true; +- break; +- } +- +- if (changes_something) +- continue; +- +- /* 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; +- } +- +- } while (again); +-} +- +-static bool unit_matters_to_anchor(Unit *u, Job *j) { +- assert(u); +- assert(!j->transaction_prev); +- +- /* Checks whether at least one of the jobs for this unit +- * matters to the anchor. */ +- +- LIST_FOREACH(transaction, j, j) +- if (j->matters_to_anchor) +- return true; +- +- return false; +-} +- +-static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsigned generation, DBusError *e) { +- Iterator i; +- Unit *u; +- int r; +- +- assert(tr); +- assert(j); +- assert(!j->transaction_prev); +- +- /* Does a recursive sweep through the ordering graph, looking +- * for a cycle. If we find cycle we try to break it. */ +- +- /* Have we seen this before? */ +- if (j->generation == generation) { +- Job *k, *delete; +- +- /* If the marker is NULL we have been here already and +- * decided the job was loop-free from here. Hence +- * shortcut things and return right-away. */ +- if (!j->marker) +- return 0; +- +- /* So, the marker is not NULL and we already have been +- * here. We have a cycle. Let's try to break it. We go +- * backwards in our path and try to find a suitable +- * job to remove. We use the marker to find our way +- * back, since smart how we are we stored our way back +- * in there. */ +- log_warning("Found ordering cycle on %s/%s", j->unit->id, job_type_to_string(j->type)); +- +- delete = NULL; +- for (k = from; k; k = ((k->generation == generation && k->marker != k) ? k->marker : NULL)) { +- +- log_info("Walked on cycle path to %s/%s", k->unit->id, job_type_to_string(k->type)); +- +- if (!delete && +- !k->installed && +- !unit_matters_to_anchor(k->unit, k)) { +- /* Ok, we can drop this one, so let's +- * do so. */ +- delete = k; +- } +- +- /* Check if this in fact was the beginning of +- * the cycle */ +- if (k == j) +- break; +- } +- +- +- if (delete) { +- log_warning("Breaking ordering cycle by deleting job %s/%s", delete->unit->id, job_type_to_string(delete->type)); +- transaction_delete_unit(tr, delete->unit); +- return -EAGAIN; +- } +- +- log_error("Unable to break cycle"); +- +- dbus_set_error(e, BUS_ERROR_TRANSACTION_ORDER_IS_CYCLIC, "Transaction order is cyclic. See system logs for details."); +- return -ENOEXEC; +- } +- +- /* Make the marker point to where we come from, so that we can +- * find our way backwards if we want to break a cycle. We use +- * a special marker for the beginning: we point to +- * ourselves. */ +- j->marker = from ? from : j; +- j->generation = generation; +- +- /* We assume that the the dependencies are bidirectional, and +- * hence can ignore UNIT_AFTER */ +- SET_FOREACH(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) +- continue; +- } +- +- 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 +- * our path anymore. */ +- j->marker = NULL; +- +- return 0; +-} +- +-static int transaction_verify_order(Transaction *tr, unsigned *generation, DBusError *e) { +- Job *j; +- int r; +- Iterator i; +- unsigned g; +- +- assert(tr); +- assert(generation); +- +- /* Check if the ordering graph is cyclic. If it is, try to fix +- * that up by dropping one of the jobs. */ +- +- g = (*generation)++; +- +- HASHMAP_FOREACH(j, tr->jobs, i) +- if ((r = transaction_verify_order_one(tr, j, NULL, g, e)) < 0) +- return r; +- +- return 0; +-} +- +-static void transaction_collect_garbage(Transaction *tr) { +- bool again; +- +- 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 (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; +- } +- +- } while (again); +-} +- +-static int transaction_is_destructive(Transaction *tr, DBusError *e) { +- Iterator i; +- Job *j; +- +- assert(tr); +- +- /* Checks whether applying this transaction means that +- * existing jobs would be replaced */ +- +- HASHMAP_FOREACH(j, tr->jobs, i) { +- +- /* Assume merged */ +- assert(!j->transaction_prev); +- assert(!j->transaction_next); +- +- if (j->unit->job && +- j->unit->job != j && +- !job_type_is_superset(j->type, j->unit->job->type)) { +- +- dbus_set_error(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE, "Transaction is destructive."); +- return -EEXIST; +- } +- } +- +- return 0; +-} +- +-static void transaction_minimize_impact(Transaction *tr) { +- bool again; +- 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; +- +- /* Would this stop a running service? +- * Would this change an existing job? +- * If so, let's drop this entry */ +- +- stops_running_service = +- j->type == JOB_STOP && UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(j->unit)); +- +- changes_existing_job = +- j->unit->job && +- job_type_is_conflicting(j->type, j->unit->job->type); +- +- if (!stops_running_service && !changes_existing_job) +- continue; +- +- if (stops_running_service) +- log_debug("%s/%s would stop a running service.", 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)); +- +- /* Ok, let's get rid of this */ +- log_debug("Deleting %s/%s to minimize impact.", j->unit->id, job_type_to_string(j->type)); +- +- transaction_delete_job(tr, j, true); +- again = true; +- break; +- } +- +- if (again) +- break; +- } +- +- } while (again); +-} +- +-static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { +- Iterator i; +- Job *j; +- int r; +- +- /* Moves the transaction jobs to the set of active jobs */ +- +- if (mode == JOB_ISOLATE) { +- +- /* When isolating first kill all installed jobs which +- * aren't part of the new transaction */ +- rescan: +- HASHMAP_FOREACH(j, m->jobs, i) { +- assert(j->installed); +- +- if (hashmap_get(tr->jobs, j->unit)) +- continue; +- +- /* 'j' itself is safe to remove, but if other jobs +- are invalidated recursively, our iterator may become +- invalid and we need to start over. */ +- if (job_finish_and_invalidate(j, JOB_CANCELED) > 0) +- goto rescan; +- } +- } +- +- HASHMAP_FOREACH(j, tr->jobs, i) { +- /* Assume merged */ +- assert(!j->transaction_prev); +- assert(!j->transaction_next); +- +- if (j->installed) +- continue; +- +- r = hashmap_put(m->jobs, UINT32_TO_PTR(j->id), j); +- if (r < 0) +- goto rollback; +- } +- +- while ((j = hashmap_steal_first(tr->jobs))) { +- Job *uj; +- if (j->installed) { +- /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ +- continue; +- } +- +- uj = j->unit->job; +- if (uj) { +- job_uninstall(uj); +- job_free(uj); +- } +- +- j->unit->job = j; +- j->installed = true; +- m->n_installed_jobs ++; +- +- /* We're fully installed. Now let's free data we don't +- * need anymore. */ +- +- assert(!j->transaction_next); +- assert(!j->transaction_prev); +- +- /* Clean the job dependencies */ +- transaction_unlink_job(tr, j, false); +- +- job_add_to_run_queue(j); +- job_add_to_dbus_queue(j); +- job_start_timer(j); +- +- log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); +- } +- +- assert(!tr->anchor); +- +- return 0; +- +-rollback: +- +- HASHMAP_FOREACH(j, tr->jobs, i) { +- if (j->installed) +- continue; +- +- hashmap_remove(m->jobs, UINT32_TO_PTR(j->id)); +- } +- +- return r; +-} +- +-static int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e) { +- int r; +- unsigned generation = 1; +- +- assert(tr); +- +- /* This applies the changes recorded in tr->jobs to +- * the actual list of jobs, if possible. */ +- +- /* First step: figure out which jobs matter */ +- transaction_find_jobs_that_matter_to_anchor(tr, 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. */ +- if (mode == JOB_FAIL) +- transaction_minimize_impact(tr); +- +- /* Third step: Drop redundant jobs */ +- transaction_drop_redundant(tr); +- +- for (;;) { +- /* Fourth step: Let's remove unneeded jobs that might +- * be lurking. */ +- if (mode != JOB_ISOLATE) +- transaction_collect_garbage(tr); +- +- /* Fifth step: verify order makes sense and correct +- * cycles if necessary and possible */ +- r = transaction_verify_order(tr, &generation, e); +- if (r >= 0) +- break; +- +- if (r != -EAGAIN) { +- log_warning("Requested transaction contains an unfixable cyclic ordering dependency: %s", bus_error(e, r)); +- return r; +- } +- +- /* Let's see if the resulting transaction ordering +- * graph is still cyclic... */ +- } +- +- for (;;) { +- /* Sixth step: let's drop unmergeable entries if +- * necessary and possible, merge entries we can +- * merge */ +- r = transaction_merge_jobs(tr, e); +- if (r >= 0) +- break; +- +- if (r != -EAGAIN) { +- log_warning("Requested transaction contains unmergeable jobs: %s", bus_error(e, r)); +- return r; +- } +- +- /* Seventh step: an entry got dropped, let's garbage +- * collect its dependencies. */ +- if (mode != JOB_ISOLATE) +- transaction_collect_garbage(tr); +- +- /* Let's see if the resulting transaction still has +- * unmergeable entries ... */ +- } +- +- /* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */ +- transaction_drop_redundant(tr); +- +- /* Ninth step: check whether we can actually apply this */ +- if (mode == JOB_FAIL) { +- r = transaction_is_destructive(tr, e); +- if (r < 0) { +- log_notice("Requested transaction contradicts existing jobs: %s", bus_error(e, r)); +- return r; +- } +- } +- +- /* Tenth step: apply changes */ +- r = transaction_apply(tr, m, mode); +- if (r < 0) { +- log_warning("Failed to apply transaction: %s", strerror(-r)); +- return r; +- } +- +- assert(hashmap_isempty(tr->jobs)); +- assert(!tr->anchor); +- +- return 0; +-} +- +-static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, bool override, bool *is_new) { +- Job *j, *f; +- +- assert(tr); +- assert(unit); +- +- /* Looks for an existing prospective job and returns that. If +- * it doesn't exist it is created and added to the prospective +- * jobs list. */ +- +- f = hashmap_get(tr->jobs, unit); +- +- LIST_FOREACH(transaction, j, f) { +- assert(j->unit == unit); +- +- if (j->type == type) { +- if (is_new) +- *is_new = false; +- return j; +- } +- } +- +- if (unit->job && unit->job->type == type) +- j = unit->job; +- else { +- j = job_new(unit->manager, type, unit); +- if (!j) +- return NULL; +- } +- +- j->generation = 0; +- j->marker = NULL; +- j->matters_to_anchor = false; +- j->override = override; +- +- LIST_PREPEND(Job, transaction, f, j); +- +- if (hashmap_replace(tr->jobs, unit, f) < 0) { +- LIST_REMOVE(Job, transaction, f, j); +- job_free(j); +- return NULL; +- } +- +- if (is_new) +- *is_new = true; +- +- /* log_debug("Added job %s/%s to transaction.", unit->id, job_type_to_string(type)); */ +- +- return j; +-} +- +-static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies) { +- assert(tr); +- assert(j); +- +- if (j->transaction_prev) +- j->transaction_prev->transaction_next = j->transaction_next; +- else if (j->transaction_next) +- hashmap_replace(tr->jobs, j->unit, j->transaction_next); +- else +- hashmap_remove_value(tr->jobs, j->unit, j); +- +- if (j->transaction_next) +- j->transaction_next->transaction_prev = j->transaction_prev; +- +- j->transaction_prev = j->transaction_next = NULL; +- +- while (j->subject_list) +- job_dependency_free(j->subject_list, tr); +- +- while (j->object_list) { +- Job *other = j->object_list->matters ? j->object_list->subject : NULL; +- +- job_dependency_free(j->object_list, tr); +- +- if (other && delete_dependencies) { +- log_debug("Deleting job %s/%s as dependency of job %s/%s", +- other->unit->id, job_type_to_string(other->type), +- j->unit->id, job_type_to_string(j->type)); +- transaction_delete_job(tr, other, delete_dependencies); +- } +- } +-} +- +-static int transaction_add_job_and_dependencies( +- Transaction *tr, +- JobType type, +- Unit *unit, +- Job *by, +- bool matters, +- bool override, +- bool conflicts, +- bool ignore_requirements, +- bool ignore_order, +- DBusError *e, +- Job **_ret) { +- Job *ret; +- Iterator i; +- Unit *dep; +- int r; +- bool is_new; +- +- assert(tr); +- assert(type < _JOB_TYPE_MAX); +- assert(unit); +- +- /* log_debug("Pulling in %s/%s from %s/%s", */ +- /* unit->id, job_type_to_string(type), */ +- /* by ? by->unit->id : "NA", */ +- /* by ? job_type_to_string(by->type) : "NA"); */ +- +- if (unit->load_state != UNIT_LOADED && +- unit->load_state != UNIT_ERROR && +- unit->load_state != UNIT_MASKED) { +- dbus_set_error(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id); +- return -EINVAL; +- } +- +- if (type != JOB_STOP && unit->load_state == UNIT_ERROR) { +- dbus_set_error(e, BUS_ERROR_LOAD_FAILED, +- "Unit %s failed to load: %s. " +- "See system logs and 'systemctl status %s' for details.", +- unit->id, +- strerror(-unit->load_error), +- unit->id); +- return -EINVAL; +- } +- +- if (type != JOB_STOP && unit->load_state == UNIT_MASKED) { +- dbus_set_error(e, BUS_ERROR_MASKED, "Unit %s is masked.", unit->id); +- return -EINVAL; +- } +- +- if (!unit_job_is_applicable(unit, type)) { +- dbus_set_error(e, BUS_ERROR_JOB_TYPE_NOT_APPLICABLE, "Job type %s is not applicable for unit %s.", job_type_to_string(type), unit->id); +- return -EBADR; +- } +- +- /* First add the job. */ +- ret = transaction_add_one_job(tr, type, unit, override, &is_new); +- if (!ret) +- return -ENOMEM; +- +- ret->ignore_order = ret->ignore_order || ignore_order; +- +- /* Then, add a link to the job. */ +- if (!job_dependency_new(by, ret, matters, conflicts, tr)) +- return -ENOMEM; +- +- if (is_new && !ignore_requirements) { +- Set *following; +- +- /* If we are following some other unit, make sure we +- * add all dependencies of everybody following. */ +- if (unit_following_set(ret->unit, &following) > 0) { +- SET_FOREACH(dep, following, i) { +- r = transaction_add_job_and_dependencies(tr, type, dep, ret, false, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- set_free(following); +- } +- +- /* Finally, recursively add in all dependencies. */ +- if (type == JOB_START || type == JOB_RELOAD_OR_START) { +- 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, NULL); +- if (r < 0) { +- if (r != -EBADR) +- goto fail; +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_BIND_TO], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- if (r != -EBADR) +- goto fail; +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- 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, NULL); +- if (r < 0) { +- log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- 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, NULL); +- if (r < 0) { +- log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- if (r != -EBADR) +- goto fail; +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- 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, NULL); +- if (r < 0) { +- log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTS], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, true, override, true, false, ignore_order, e, NULL); +- if (r < 0) { +- if (r != -EBADR) +- goto fail; +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, false, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- } +- +- if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) { +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRED_BY], i) { +- r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- if (r != -EBADR) +- goto fail; +- +- if (e) +- dbus_error_free(e); +- } +- } +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_BOUND_BY], i) { +- r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- if (r != -EBADR) +- goto fail; +- +- if (e) +- dbus_error_free(e); +- } +- } +- } +- +- if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START) { +- +- SET_FOREACH(dep, ret->unit->dependencies[UNIT_PROPAGATE_RELOAD_TO], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e, NULL); +- if (r < 0) { +- log_warning("Cannot add dependency reload job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); +- +- if (e) +- dbus_error_free(e); +- } +- } +- } +- +- /* JOB_VERIFY_STARTED, JOB_RELOAD require no dependency handling */ +- } +- +- if (_ret) +- *_ret = ret; +- +- return 0; +- +-fail: +- return r; +-} +- +-static int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { +- Iterator i; +- Unit *u; +- char *k; +- int r; +- +- assert(tr); +- assert(m); +- +- HASHMAP_FOREACH_KEY(u, k, m->units, i) { +- +- /* ignore aliases */ +- if (u->id != k) +- continue; +- +- if (u->ignore_on_isolate) +- continue; +- +- /* No need to stop inactive jobs */ +- if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(u)) && !u->job) +- continue; +- +- /* Is there already something listed for this? */ +- if (hashmap_get(tr->jobs, u)) +- continue; +- +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, NULL, true, false, false, false, false, NULL, NULL); +- if (r < 0) +- log_warning("Cannot add isolate job for unit %s, ignoring: %s", u->id, strerror(-r)); +- } +- +- return 0; +-} +- +-static Transaction *transaction_new(void) { +- Transaction *tr; +- +- tr = new0(Transaction, 1); +- if (!tr) +- return NULL; +- +- tr->jobs = hashmap_new(trivial_hash_func, trivial_compare_func); +- if (!tr->jobs) { +- free(tr); +- return NULL; +- } +- +- return tr; +-} +- +-static void transaction_free(Transaction *tr) { +- assert(hashmap_isempty(tr->jobs)); +- hashmap_free(tr->jobs); +- free(tr); +-} +- + int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool override, DBusError *e, Job **_ret) { + int r; + Job *ret; +diff --git a/src/manager.h b/src/manager.h +index b982d7e..915f6fc 100644 +--- a/src/manager.h ++++ b/src/manager.h +@@ -33,7 +33,6 @@ + #define MANAGER_MAX_NAMES 131072 /* 128K */ + + typedef struct Manager Manager; +-typedef struct Transaction Transaction; + typedef enum WatchType WatchType; + typedef struct Watch Watch; + +@@ -92,12 +91,6 @@ struct Watch { + #include "dbus.h" + #include "path-lookup.h" + +-struct Transaction { +- /* Jobs to be added */ +- Hashmap *jobs; /* Unit object => Job object list 1:1 */ +- JobDependency *anchor; +-}; +- + struct Manager { + /* Note that the set of units we know of is allowed to be + * inconsistent. However the subset of it that is loaded may +diff --git a/src/transaction.c b/src/transaction.c +new file mode 100644 +index 0000000..8fa89a7 +--- /dev/null ++++ b/src/transaction.c +@@ -0,0 +1,1101 @@ ++#include "transaction.h" ++#include "bus-errors.h" ++ ++static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies); ++ ++static void transaction_delete_job(Transaction *tr, Job *j, bool delete_dependencies) { ++ assert(tr); ++ assert(j); ++ ++ /* Deletes one job from the transaction */ ++ ++ transaction_unlink_job(tr, j, delete_dependencies); ++ ++ if (!j->installed) ++ job_free(j); ++} ++ ++static void transaction_delete_unit(Transaction *tr, Unit *u) { ++ Job *j; ++ ++ /* Deletes all jobs associated with a certain unit from the ++ * transaction */ ++ ++ while ((j = hashmap_get(tr->jobs, u))) ++ transaction_delete_job(tr, j, true); ++} ++ ++void transaction_abort(Transaction *tr) { ++ Job *j; ++ ++ assert(tr); ++ ++ while ((j = hashmap_first(tr->jobs))) ++ transaction_delete_job(tr, j, true); ++ ++ assert(hashmap_isempty(tr->jobs)); ++ ++ assert(!tr->anchor); ++} ++ ++static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, Job *j, unsigned generation) { ++ JobDependency *l; ++ ++ assert(tr); ++ ++ /* A recursive sweep through the graph that marks all units ++ * that matter to the anchor job, i.e. are directly or ++ * indirectly a dependency of the anchor job via paths that ++ * are fully marked as mattering. */ ++ ++ if (j) ++ l = j->subject_list; ++ else ++ l = tr->anchor; ++ ++ LIST_FOREACH(subject, l, l) { ++ ++ /* This link does not matter */ ++ if (!l->matters) ++ continue; ++ ++ /* This unit has already been marked */ ++ if (l->object->generation == generation) ++ continue; ++ ++ l->object->matters_to_anchor = true; ++ l->object->generation = generation; ++ ++ transaction_find_jobs_that_matter_to_anchor(tr, l->object, generation); ++ } ++} ++ ++static void transaction_merge_and_delete_job(Transaction *tr, Job *j, Job *other, JobType t) { ++ JobDependency *l, *last; ++ ++ assert(j); ++ assert(other); ++ assert(j->unit == other->unit); ++ assert(!j->installed); ++ ++ /* Merges 'other' into 'j' and then deletes 'other'. */ ++ ++ j->type = t; ++ j->state = JOB_WAITING; ++ j->override = j->override || other->override; ++ ++ j->matters_to_anchor = j->matters_to_anchor || other->matters_to_anchor; ++ ++ /* Patch us in as new owner of the JobDependency objects */ ++ last = NULL; ++ LIST_FOREACH(subject, l, other->subject_list) { ++ assert(l->subject == other); ++ l->subject = j; ++ last = l; ++ } ++ ++ /* Merge both lists */ ++ if (last) { ++ last->subject_next = j->subject_list; ++ if (j->subject_list) ++ j->subject_list->subject_prev = last; ++ j->subject_list = other->subject_list; ++ } ++ ++ /* Patch us in as new owner of the JobDependency objects */ ++ last = NULL; ++ LIST_FOREACH(object, l, other->object_list) { ++ assert(l->object == other); ++ l->object = j; ++ last = l; ++ } ++ ++ /* Merge both lists */ ++ if (last) { ++ last->object_next = j->object_list; ++ if (j->object_list) ++ j->object_list->object_prev = last; ++ j->object_list = other->object_list; ++ } ++ ++ /* Kill the other job */ ++ other->subject_list = NULL; ++ other->object_list = NULL; ++ transaction_delete_job(tr, 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(Transaction *tr, Job *j) { ++ Job *k; ++ ++ assert(j); ++ ++ /* Tries to delete one item in the linked list ++ * j->transaction_next->transaction_next->... that conflicts ++ * with another one, in an attempt to make an inconsistent ++ * transaction work. */ ++ ++ /* We rely here on the fact that if a merged with b does not ++ * merge with c, either a or b merge with c neither */ ++ LIST_FOREACH(transaction, j, j) ++ LIST_FOREACH(transaction, k, j->transaction_next) { ++ Job *d; ++ ++ /* Is this one mergeable? Then skip it */ ++ if (job_type_is_mergeable(j->type, k->type)) ++ continue; ++ ++ /* Ok, we found two that conflict, let's see if we can ++ * drop one of them */ ++ 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->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->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 ++ d = j; ++ ++ } else if (!j->matters_to_anchor) ++ d = j; ++ else if (!k->matters_to_anchor) ++ d = k; ++ else ++ return -ENOEXEC; ++ ++ /* Ok, we can drop one, so let's do so. */ ++ log_debug("Fixing conflicting jobs by deleting job %s/%s", d->unit->id, job_type_to_string(d->type)); ++ transaction_delete_job(tr, d, true); ++ return 0; ++ } ++ ++ return -EINVAL; ++} ++ ++static int transaction_merge_jobs(Transaction *tr, DBusError *e) { ++ Job *j; ++ Iterator i; ++ int r; ++ ++ assert(tr); ++ ++ /* First step, check whether any of the jobs for one specific ++ * task conflict. If so, try to drop one of them. */ ++ HASHMAP_FOREACH(j, tr->jobs, i) { ++ JobType t; ++ Job *k; ++ ++ t = j->type; ++ LIST_FOREACH(transaction, k, j->transaction_next) { ++ if (job_type_merge(&t, k->type) >= 0) ++ continue; ++ ++ /* OK, we could not merge all jobs for this ++ * action. Let's see if we can get rid of one ++ * of them */ ++ ++ r = delete_one_unmergeable_job(tr, j); ++ if (r >= 0) ++ /* Ok, we managed to drop one, now ++ * let's ask our callers to call us ++ * again after garbage collecting */ ++ return -EAGAIN; ++ ++ /* We couldn't merge anything. Failure */ ++ dbus_set_error(e, BUS_ERROR_TRANSACTION_JOBS_CONFLICTING, "Transaction contains conflicting jobs '%s' and '%s' for %s. Probably contradicting requirement dependencies configured.", ++ job_type_to_string(t), job_type_to_string(k->type), k->unit->id); ++ return r; ++ } ++ } ++ ++ /* Second step, merge the jobs. */ ++ HASHMAP_FOREACH(j, tr->jobs, i) { ++ JobType t = j->type; ++ Job *k; ++ ++ /* Merge all transactions */ ++ LIST_FOREACH(transaction, k, j->transaction_next) ++ assert_se(job_type_merge(&t, k->type) == 0); ++ ++ /* If an active job is mergeable, merge it too */ ++ if (j->unit->job) ++ job_type_merge(&t, j->unit->job->type); /* Might fail. Which is OK */ ++ ++ while ((k = j->transaction_next)) { ++ if (j->installed) { ++ transaction_merge_and_delete_job(tr, k, j, t); ++ j = k; ++ } else ++ transaction_merge_and_delete_job(tr, j, k, t); ++ } ++ ++ if (j->unit->job && !j->installed) ++ transaction_merge_and_delete_job(tr, j, j->unit->job, t); ++ ++ assert(!j->transaction_next); ++ assert(!j->transaction_prev); ++ } ++ ++ return 0; ++} ++ ++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; ++ ++ LIST_FOREACH(transaction, k, j) { ++ ++ if (!job_is_anchor(k) && ++ (k->installed || 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; ++ ++ changes_something = true; ++ break; ++ } ++ ++ if (changes_something) ++ continue; ++ ++ /* 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; ++ } ++ ++ } while (again); ++} ++ ++static bool unit_matters_to_anchor(Unit *u, Job *j) { ++ assert(u); ++ assert(!j->transaction_prev); ++ ++ /* Checks whether at least one of the jobs for this unit ++ * matters to the anchor. */ ++ ++ LIST_FOREACH(transaction, j, j) ++ if (j->matters_to_anchor) ++ return true; ++ ++ return false; ++} ++ ++static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsigned generation, DBusError *e) { ++ Iterator i; ++ Unit *u; ++ int r; ++ ++ assert(tr); ++ assert(j); ++ assert(!j->transaction_prev); ++ ++ /* Does a recursive sweep through the ordering graph, looking ++ * for a cycle. If we find cycle we try to break it. */ ++ ++ /* Have we seen this before? */ ++ if (j->generation == generation) { ++ Job *k, *delete; ++ ++ /* If the marker is NULL we have been here already and ++ * decided the job was loop-free from here. Hence ++ * shortcut things and return right-away. */ ++ if (!j->marker) ++ return 0; ++ ++ /* So, the marker is not NULL and we already have been ++ * here. We have a cycle. Let's try to break it. We go ++ * backwards in our path and try to find a suitable ++ * job to remove. We use the marker to find our way ++ * back, since smart how we are we stored our way back ++ * in there. */ ++ log_warning("Found ordering cycle on %s/%s", j->unit->id, job_type_to_string(j->type)); ++ ++ delete = NULL; ++ for (k = from; k; k = ((k->generation == generation && k->marker != k) ? k->marker : NULL)) { ++ ++ log_info("Walked on cycle path to %s/%s", k->unit->id, job_type_to_string(k->type)); ++ ++ if (!delete && ++ !k->installed && ++ !unit_matters_to_anchor(k->unit, k)) { ++ /* Ok, we can drop this one, so let's ++ * do so. */ ++ delete = k; ++ } ++ ++ /* Check if this in fact was the beginning of ++ * the cycle */ ++ if (k == j) ++ break; ++ } ++ ++ ++ if (delete) { ++ log_warning("Breaking ordering cycle by deleting job %s/%s", delete->unit->id, job_type_to_string(delete->type)); ++ transaction_delete_unit(tr, delete->unit); ++ return -EAGAIN; ++ } ++ ++ log_error("Unable to break cycle"); ++ ++ dbus_set_error(e, BUS_ERROR_TRANSACTION_ORDER_IS_CYCLIC, "Transaction order is cyclic. See system logs for details."); ++ return -ENOEXEC; ++ } ++ ++ /* Make the marker point to where we come from, so that we can ++ * find our way backwards if we want to break a cycle. We use ++ * a special marker for the beginning: we point to ++ * ourselves. */ ++ j->marker = from ? from : j; ++ j->generation = generation; ++ ++ /* We assume that the the dependencies are bidirectional, and ++ * hence can ignore UNIT_AFTER */ ++ SET_FOREACH(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) ++ continue; ++ } ++ ++ 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 ++ * our path anymore. */ ++ j->marker = NULL; ++ ++ return 0; ++} ++ ++static int transaction_verify_order(Transaction *tr, unsigned *generation, DBusError *e) { ++ Job *j; ++ int r; ++ Iterator i; ++ unsigned g; ++ ++ assert(tr); ++ assert(generation); ++ ++ /* Check if the ordering graph is cyclic. If it is, try to fix ++ * that up by dropping one of the jobs. */ ++ ++ g = (*generation)++; ++ ++ HASHMAP_FOREACH(j, tr->jobs, i) ++ if ((r = transaction_verify_order_one(tr, j, NULL, g, e)) < 0) ++ return r; ++ ++ return 0; ++} ++ ++static void transaction_collect_garbage(Transaction *tr) { ++ bool again; ++ ++ 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 (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; ++ } ++ ++ } while (again); ++} ++ ++static int transaction_is_destructive(Transaction *tr, DBusError *e) { ++ Iterator i; ++ Job *j; ++ ++ assert(tr); ++ ++ /* Checks whether applying this transaction means that ++ * existing jobs would be replaced */ ++ ++ HASHMAP_FOREACH(j, tr->jobs, i) { ++ ++ /* Assume merged */ ++ assert(!j->transaction_prev); ++ assert(!j->transaction_next); ++ ++ if (j->unit->job && ++ j->unit->job != j && ++ !job_type_is_superset(j->type, j->unit->job->type)) { ++ ++ dbus_set_error(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE, "Transaction is destructive."); ++ return -EEXIST; ++ } ++ } ++ ++ return 0; ++} ++ ++static void transaction_minimize_impact(Transaction *tr) { ++ bool again; ++ 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; ++ ++ /* Would this stop a running service? ++ * Would this change an existing job? ++ * If so, let's drop this entry */ ++ ++ stops_running_service = ++ j->type == JOB_STOP && UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(j->unit)); ++ ++ changes_existing_job = ++ j->unit->job && ++ job_type_is_conflicting(j->type, j->unit->job->type); ++ ++ if (!stops_running_service && !changes_existing_job) ++ continue; ++ ++ if (stops_running_service) ++ log_debug("%s/%s would stop a running service.", 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)); ++ ++ /* Ok, let's get rid of this */ ++ log_debug("Deleting %s/%s to minimize impact.", j->unit->id, job_type_to_string(j->type)); ++ ++ transaction_delete_job(tr, j, true); ++ again = true; ++ break; ++ } ++ ++ if (again) ++ break; ++ } ++ ++ } while (again); ++} ++ ++static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { ++ Iterator i; ++ Job *j; ++ int r; ++ ++ /* Moves the transaction jobs to the set of active jobs */ ++ ++ if (mode == JOB_ISOLATE) { ++ ++ /* When isolating first kill all installed jobs which ++ * aren't part of the new transaction */ ++ rescan: ++ HASHMAP_FOREACH(j, m->jobs, i) { ++ assert(j->installed); ++ ++ if (hashmap_get(tr->jobs, j->unit)) ++ continue; ++ ++ /* 'j' itself is safe to remove, but if other jobs ++ are invalidated recursively, our iterator may become ++ invalid and we need to start over. */ ++ if (job_finish_and_invalidate(j, JOB_CANCELED) > 0) ++ goto rescan; ++ } ++ } ++ ++ HASHMAP_FOREACH(j, tr->jobs, i) { ++ /* Assume merged */ ++ assert(!j->transaction_prev); ++ assert(!j->transaction_next); ++ ++ if (j->installed) ++ continue; ++ ++ r = hashmap_put(m->jobs, UINT32_TO_PTR(j->id), j); ++ if (r < 0) ++ goto rollback; ++ } ++ ++ while ((j = hashmap_steal_first(tr->jobs))) { ++ Job *uj; ++ if (j->installed) { ++ /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ ++ continue; ++ } ++ ++ uj = j->unit->job; ++ if (uj) { ++ job_uninstall(uj); ++ job_free(uj); ++ } ++ ++ j->unit->job = j; ++ j->installed = true; ++ m->n_installed_jobs ++; ++ ++ /* We're fully installed. Now let's free data we don't ++ * need anymore. */ ++ ++ assert(!j->transaction_next); ++ assert(!j->transaction_prev); ++ ++ /* Clean the job dependencies */ ++ transaction_unlink_job(tr, j, false); ++ ++ job_add_to_run_queue(j); ++ job_add_to_dbus_queue(j); ++ job_start_timer(j); ++ ++ log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); ++ } ++ ++ assert(!tr->anchor); ++ ++ return 0; ++ ++rollback: ++ ++ HASHMAP_FOREACH(j, tr->jobs, i) { ++ if (j->installed) ++ continue; ++ ++ hashmap_remove(m->jobs, UINT32_TO_PTR(j->id)); ++ } ++ ++ return r; ++} ++ ++int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e) { ++ int r; ++ unsigned generation = 1; ++ ++ assert(tr); ++ ++ /* This applies the changes recorded in tr->jobs to ++ * the actual list of jobs, if possible. */ ++ ++ /* First step: figure out which jobs matter */ ++ transaction_find_jobs_that_matter_to_anchor(tr, 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. */ ++ if (mode == JOB_FAIL) ++ transaction_minimize_impact(tr); ++ ++ /* Third step: Drop redundant jobs */ ++ transaction_drop_redundant(tr); ++ ++ for (;;) { ++ /* Fourth step: Let's remove unneeded jobs that might ++ * be lurking. */ ++ if (mode != JOB_ISOLATE) ++ transaction_collect_garbage(tr); ++ ++ /* Fifth step: verify order makes sense and correct ++ * cycles if necessary and possible */ ++ r = transaction_verify_order(tr, &generation, e); ++ if (r >= 0) ++ break; ++ ++ if (r != -EAGAIN) { ++ log_warning("Requested transaction contains an unfixable cyclic ordering dependency: %s", bus_error(e, r)); ++ return r; ++ } ++ ++ /* Let's see if the resulting transaction ordering ++ * graph is still cyclic... */ ++ } ++ ++ for (;;) { ++ /* Sixth step: let's drop unmergeable entries if ++ * necessary and possible, merge entries we can ++ * merge */ ++ r = transaction_merge_jobs(tr, e); ++ if (r >= 0) ++ break; ++ ++ if (r != -EAGAIN) { ++ log_warning("Requested transaction contains unmergeable jobs: %s", bus_error(e, r)); ++ return r; ++ } ++ ++ /* Seventh step: an entry got dropped, let's garbage ++ * collect its dependencies. */ ++ if (mode != JOB_ISOLATE) ++ transaction_collect_garbage(tr); ++ ++ /* Let's see if the resulting transaction still has ++ * unmergeable entries ... */ ++ } ++ ++ /* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */ ++ transaction_drop_redundant(tr); ++ ++ /* Ninth step: check whether we can actually apply this */ ++ if (mode == JOB_FAIL) { ++ r = transaction_is_destructive(tr, e); ++ if (r < 0) { ++ log_notice("Requested transaction contradicts existing jobs: %s", bus_error(e, r)); ++ return r; ++ } ++ } ++ ++ /* Tenth step: apply changes */ ++ r = transaction_apply(tr, m, mode); ++ if (r < 0) { ++ log_warning("Failed to apply transaction: %s", strerror(-r)); ++ return r; ++ } ++ ++ assert(hashmap_isempty(tr->jobs)); ++ assert(!tr->anchor); ++ ++ return 0; ++} ++ ++static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, bool override, bool *is_new) { ++ Job *j, *f; ++ ++ assert(tr); ++ assert(unit); ++ ++ /* Looks for an existing prospective job and returns that. If ++ * it doesn't exist it is created and added to the prospective ++ * jobs list. */ ++ ++ f = hashmap_get(tr->jobs, unit); ++ ++ LIST_FOREACH(transaction, j, f) { ++ assert(j->unit == unit); ++ ++ if (j->type == type) { ++ if (is_new) ++ *is_new = false; ++ return j; ++ } ++ } ++ ++ if (unit->job && unit->job->type == type) ++ j = unit->job; ++ else { ++ j = job_new(unit->manager, type, unit); ++ if (!j) ++ return NULL; ++ } ++ ++ j->generation = 0; ++ j->marker = NULL; ++ j->matters_to_anchor = false; ++ j->override = override; ++ ++ LIST_PREPEND(Job, transaction, f, j); ++ ++ if (hashmap_replace(tr->jobs, unit, f) < 0) { ++ LIST_REMOVE(Job, transaction, f, j); ++ job_free(j); ++ return NULL; ++ } ++ ++ if (is_new) ++ *is_new = true; ++ ++ /* log_debug("Added job %s/%s to transaction.", unit->id, job_type_to_string(type)); */ ++ ++ return j; ++} ++ ++static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies) { ++ assert(tr); ++ assert(j); ++ ++ if (j->transaction_prev) ++ j->transaction_prev->transaction_next = j->transaction_next; ++ else if (j->transaction_next) ++ hashmap_replace(tr->jobs, j->unit, j->transaction_next); ++ else ++ hashmap_remove_value(tr->jobs, j->unit, j); ++ ++ if (j->transaction_next) ++ j->transaction_next->transaction_prev = j->transaction_prev; ++ ++ j->transaction_prev = j->transaction_next = NULL; ++ ++ while (j->subject_list) ++ job_dependency_free(j->subject_list, tr); ++ ++ while (j->object_list) { ++ Job *other = j->object_list->matters ? j->object_list->subject : NULL; ++ ++ job_dependency_free(j->object_list, tr); ++ ++ if (other && delete_dependencies) { ++ log_debug("Deleting job %s/%s as dependency of job %s/%s", ++ other->unit->id, job_type_to_string(other->type), ++ j->unit->id, job_type_to_string(j->type)); ++ transaction_delete_job(tr, other, delete_dependencies); ++ } ++ } ++} ++ ++int transaction_add_job_and_dependencies( ++ Transaction *tr, ++ JobType type, ++ Unit *unit, ++ Job *by, ++ bool matters, ++ bool override, ++ bool conflicts, ++ bool ignore_requirements, ++ bool ignore_order, ++ DBusError *e, ++ Job **_ret) { ++ Job *ret; ++ Iterator i; ++ Unit *dep; ++ int r; ++ bool is_new; ++ ++ assert(tr); ++ assert(type < _JOB_TYPE_MAX); ++ assert(unit); ++ ++ /* log_debug("Pulling in %s/%s from %s/%s", */ ++ /* unit->id, job_type_to_string(type), */ ++ /* by ? by->unit->id : "NA", */ ++ /* by ? job_type_to_string(by->type) : "NA"); */ ++ ++ if (unit->load_state != UNIT_LOADED && ++ unit->load_state != UNIT_ERROR && ++ unit->load_state != UNIT_MASKED) { ++ dbus_set_error(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id); ++ return -EINVAL; ++ } ++ ++ if (type != JOB_STOP && unit->load_state == UNIT_ERROR) { ++ dbus_set_error(e, BUS_ERROR_LOAD_FAILED, ++ "Unit %s failed to load: %s. " ++ "See system logs and 'systemctl status %s' for details.", ++ unit->id, ++ strerror(-unit->load_error), ++ unit->id); ++ return -EINVAL; ++ } ++ ++ if (type != JOB_STOP && unit->load_state == UNIT_MASKED) { ++ dbus_set_error(e, BUS_ERROR_MASKED, "Unit %s is masked.", unit->id); ++ return -EINVAL; ++ } ++ ++ if (!unit_job_is_applicable(unit, type)) { ++ dbus_set_error(e, BUS_ERROR_JOB_TYPE_NOT_APPLICABLE, "Job type %s is not applicable for unit %s.", job_type_to_string(type), unit->id); ++ return -EBADR; ++ } ++ ++ /* First add the job. */ ++ ret = transaction_add_one_job(tr, type, unit, override, &is_new); ++ if (!ret) ++ return -ENOMEM; ++ ++ ret->ignore_order = ret->ignore_order || ignore_order; ++ ++ /* Then, add a link to the job. */ ++ if (!job_dependency_new(by, ret, matters, conflicts, tr)) ++ return -ENOMEM; ++ ++ if (is_new && !ignore_requirements) { ++ Set *following; ++ ++ /* If we are following some other unit, make sure we ++ * add all dependencies of everybody following. */ ++ if (unit_following_set(ret->unit, &following) > 0) { ++ SET_FOREACH(dep, following, i) { ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ set_free(following); ++ } ++ ++ /* Finally, recursively add in all dependencies. */ ++ if (type == JOB_START || type == JOB_RELOAD_OR_START) { ++ 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, NULL); ++ if (r < 0) { ++ if (r != -EBADR) ++ goto fail; ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_BIND_TO], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ if (r != -EBADR) ++ goto fail; ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ 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, NULL); ++ if (r < 0) { ++ log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ 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, NULL); ++ if (r < 0) { ++ log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ if (r != -EBADR) ++ goto fail; ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ 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, NULL); ++ if (r < 0) { ++ log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTS], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, true, override, true, false, ignore_order, e, NULL); ++ if (r < 0) { ++ if (r != -EBADR) ++ goto fail; ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ log_warning("Cannot add dependency job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ } ++ ++ if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) { ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRED_BY], i) { ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ if (r != -EBADR) ++ goto fail; ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_BOUND_BY], i) { ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ if (r != -EBADR) ++ goto fail; ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ } ++ ++ if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START) { ++ ++ SET_FOREACH(dep, ret->unit->dependencies[UNIT_PROPAGATE_RELOAD_TO], i) { ++ r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ if (r < 0) { ++ log_warning("Cannot add dependency reload job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); ++ ++ if (e) ++ dbus_error_free(e); ++ } ++ } ++ } ++ ++ /* JOB_VERIFY_STARTED, JOB_RELOAD require no dependency handling */ ++ } ++ ++ if (_ret) ++ *_ret = ret; ++ ++ return 0; ++ ++fail: ++ return r; ++} ++ ++int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { ++ Iterator i; ++ Unit *u; ++ char *k; ++ int r; ++ ++ assert(tr); ++ assert(m); ++ ++ HASHMAP_FOREACH_KEY(u, k, m->units, i) { ++ ++ /* ignore aliases */ ++ if (u->id != k) ++ continue; ++ ++ if (u->ignore_on_isolate) ++ continue; ++ ++ /* No need to stop inactive jobs */ ++ if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(u)) && !u->job) ++ continue; ++ ++ /* Is there already something listed for this? */ ++ if (hashmap_get(tr->jobs, u)) ++ continue; ++ ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, NULL, true, false, false, false, false, NULL, NULL); ++ if (r < 0) ++ log_warning("Cannot add isolate job for unit %s, ignoring: %s", u->id, strerror(-r)); ++ } ++ ++ return 0; ++} ++ ++Transaction *transaction_new(void) { ++ Transaction *tr; ++ ++ tr = new0(Transaction, 1); ++ if (!tr) ++ return NULL; ++ ++ tr->jobs = hashmap_new(trivial_hash_func, trivial_compare_func); ++ if (!tr->jobs) { ++ free(tr); ++ return NULL; ++ } ++ ++ return tr; ++} ++ ++void transaction_free(Transaction *tr) { ++ assert(hashmap_isempty(tr->jobs)); ++ hashmap_free(tr->jobs); ++ free(tr); ++} +diff --git a/src/transaction.h b/src/transaction.h +new file mode 100644 +index 0000000..d519ffb +--- /dev/null ++++ b/src/transaction.h +@@ -0,0 +1,36 @@ ++#ifndef footransactionhfoo ++#define footransactionhfoo ++ ++typedef struct Transaction Transaction; ++ ++#include "unit.h" ++#include "manager.h" ++#include "job.h" ++#include "hashmap.h" ++ ++struct Transaction { ++ /* Jobs to be added */ ++ Hashmap *jobs; /* Unit object => Job object list 1:1 */ ++ JobDependency *anchor; ++}; ++ ++Transaction *transaction_new(void); ++void transaction_free(Transaction *tr); ++ ++int transaction_add_job_and_dependencies( ++ Transaction *tr, ++ JobType type, ++ Unit *unit, ++ Job *by, ++ bool matters, ++ bool override, ++ bool conflicts, ++ bool ignore_requirements, ++ bool ignore_order, ++ DBusError *e, ++ Job **_ret); ++int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e); ++int transaction_add_isolate_jobs(Transaction *tr, Manager *m); ++void transaction_abort(Transaction *tr); ++ ++#endif +-- +1.7.7 + + +From bd61057f3d44d7bb4e279310a16646eb8ab9bb8c Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Wed, 18 Apr 2012 01:39:20 +0200 +Subject: [PATCH 09/30] job: job_new() can find the manager from the unit + (cherry picked from commit + 668ad332a404736474749cbcc8404af3e4447170) + +--- + src/job.c | 7 +++---- + src/job.h | 2 +- + src/transaction.c | 2 +- + 3 files changed, 5 insertions(+), 6 deletions(-) + +diff --git a/src/job.c b/src/job.c +index 6d48748..c30bfda 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -33,18 +33,17 @@ + #include "log.h" + #include "dbus-job.h" + +-Job* job_new(Manager *m, JobType type, Unit *unit) { ++Job* job_new(Unit *unit, JobType type) { + Job *j; + +- assert(m); + assert(type < _JOB_TYPE_MAX); + assert(unit); + + if (!(j = new0(Job, 1))) + return NULL; + +- j->manager = m; +- j->id = m->current_job_id++; ++ j->manager = unit->manager; ++ j->id = j->manager->current_job_id++; + j->type = type; + j->unit = unit; + +diff --git a/src/job.h b/src/job.h +index efb0af9..faa10e3 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -137,7 +137,7 @@ struct Job { + bool ignore_order:1; + }; + +-Job* job_new(Manager *m, JobType type, Unit *unit); ++Job* job_new(Unit *unit, JobType type); + void job_uninstall(Job *j); + void job_free(Job *job); + void job_dump(Job *j, FILE*f, const char *prefix); +diff --git a/src/transaction.c b/src/transaction.c +index 8fa89a7..1344e2f 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -763,7 +763,7 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b + if (unit->job && unit->job->type == type) + j = unit->job; + else { +- j = job_new(unit->manager, type, unit); ++ j = job_new(unit, type); + if (!j) + return NULL; + } +-- +1.7.7 + + +From 0d6df78cb48dfbd1c37444ee3702ba59d87ca352 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Wed, 18 Apr 2012 15:21:24 +0200 +Subject: [PATCH 10/30] job: jobs shouldn't need to know about transaction + anchors + +Let the transactions maintain their own anchor links. +(cherry picked from commit 1da4264fbd0fa5e6b1bd5e0ac4674a3503774369) +--- + src/job.c | 8 ++------ + src/job.h | 5 ++--- + src/transaction.c | 18 +++++++++++++++--- + 3 files changed, 19 insertions(+), 12 deletions(-) + +diff --git a/src/job.c b/src/job.c +index c30bfda..a5ce2d4 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -96,7 +96,7 @@ void job_free(Job *j) { + free(j); + } + +-JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts, Transaction *tr) { ++JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) { + JobDependency *l; + + assert(object); +@@ -116,21 +116,17 @@ JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool + + if (subject) + LIST_PREPEND(JobDependency, subject, subject->subject_list, l); +- else +- LIST_PREPEND(JobDependency, subject, tr->anchor, l); + + LIST_PREPEND(JobDependency, object, object->object_list, l); + + return l; + } + +-void job_dependency_free(JobDependency *l, Transaction *tr) { ++void job_dependency_free(JobDependency *l) { + assert(l); + + if (l->subject) + LIST_REMOVE(JobDependency, subject, l->subject->subject_list, l); +- else +- LIST_REMOVE(JobDependency, subject, tr->anchor, l); + + LIST_REMOVE(JobDependency, object, l->object->object_list, l); + +diff --git a/src/job.h b/src/job.h +index faa10e3..cbd10c5 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -34,7 +34,6 @@ typedef enum JobMode JobMode; + typedef enum JobResult JobResult; + + #include "manager.h" +-#include "transaction.h" + #include "unit.h" + #include "hashmap.h" + #include "list.h" +@@ -142,8 +141,8 @@ void job_uninstall(Job *j); + 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, bool conflicts, Transaction *tr); +-void job_dependency_free(JobDependency *l, Transaction *tr); ++JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts); ++void job_dependency_free(JobDependency *l); + + bool job_is_anchor(Job *j); + +diff --git a/src/transaction.c b/src/transaction.c +index 1344e2f..2a6f1de 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -789,6 +789,12 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b + return j; + } + ++static void transaction_job_dependency_free(Transaction *tr, JobDependency *l) { ++ if (!l->subject) ++ LIST_REMOVE(JobDependency, subject, tr->anchor, l); ++ job_dependency_free(l); ++} ++ + static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies) { + assert(tr); + assert(j); +@@ -806,12 +812,12 @@ static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependen + j->transaction_prev = j->transaction_next = NULL; + + while (j->subject_list) +- job_dependency_free(j->subject_list, tr); ++ transaction_job_dependency_free(tr, j->subject_list); + + while (j->object_list) { + Job *other = j->object_list->matters ? j->object_list->subject : NULL; + +- job_dependency_free(j->object_list, tr); ++ transaction_job_dependency_free(tr, j->object_list); + + if (other && delete_dependencies) { + log_debug("Deleting job %s/%s as dependency of job %s/%s", +@@ -835,6 +841,7 @@ int transaction_add_job_and_dependencies( + DBusError *e, + Job **_ret) { + Job *ret; ++ JobDependency *l; + Iterator i; + Unit *dep; + int r; +@@ -884,9 +891,14 @@ int transaction_add_job_and_dependencies( + ret->ignore_order = ret->ignore_order || ignore_order; + + /* Then, add a link to the job. */ +- if (!job_dependency_new(by, ret, matters, conflicts, tr)) ++ l = job_dependency_new(by, ret, matters, conflicts); ++ if (!l) + return -ENOMEM; + ++ /* If the link has no subject job, it's the anchor link. */ ++ if (!by) ++ LIST_PREPEND(JobDependency, subject, tr->anchor, l); ++ + if (is_new && !ignore_requirements) { + Set *following; + +-- +1.7.7 + + +From 0799fe0aa4a9e74a0a11cedeb76a630c6db853d5 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Wed, 18 Apr 2012 18:15:49 +0200 +Subject: [PATCH 11/30] transaction: do not add installed jobs to the + transaction + +Do not attempt to optimize away the job creation by refering to installed jobs. +We do not want to disturb installed jobs until commiting the transaction. + +(A later patch to job merging will make the separation of transaction jobs and +installed jobs complete.) +(cherry picked from commit 3c956cfee29fe2642fbe257f55adb3583a4af878) +--- + src/transaction.c | 10 +++------- + 1 files changed, 3 insertions(+), 7 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index 2a6f1de..c00dd45 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -760,13 +760,9 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b + } + } + +- if (unit->job && unit->job->type == type) +- j = unit->job; +- else { +- j = job_new(unit, type); +- if (!j) +- return NULL; +- } ++ j = job_new(unit, type); ++ if (!j) ++ return NULL; + + j->generation = 0; + j->marker = NULL; +-- +1.7.7 + + +From 7f60fce10d4c111bcd274d0fcd83a0139b53a9bc Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 00:33:26 +0200 +Subject: [PATCH 12/30] transaction: maintain anchor_job + +Track which job is the anchor in the transaction. +(cherry picked from commit b94fbd30781d7533492cec65bf7d86fa19a1a074) +--- + src/manager.c | 7 +++---- + src/transaction.c | 37 ++++++++++++++++++------------------- + src/transaction.h | 4 ++-- + 3 files changed, 23 insertions(+), 25 deletions(-) + +diff --git a/src/manager.c b/src/manager.c +index a129b88..86ec858 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -631,7 +631,6 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { + + int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool override, DBusError *e, Job **_ret) { + int r; +- Job *ret; + Transaction *tr; + + assert(m); +@@ -657,7 +656,7 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove + + r = transaction_add_job_and_dependencies(tr, type, unit, NULL, true, override, false, + mode == JOB_IGNORE_DEPENDENCIES || mode == JOB_IGNORE_REQUIREMENTS, +- mode == JOB_IGNORE_DEPENDENCIES, e, &ret); ++ mode == JOB_IGNORE_DEPENDENCIES, e); + if (r < 0) + goto tr_abort; + +@@ -671,10 +670,10 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove + if (r < 0) + goto tr_abort; + +- log_debug("Enqueued job %s/%s as %u", unit->id, job_type_to_string(type), (unsigned) ret->id); ++ log_debug("Enqueued job %s/%s as %u", unit->id, job_type_to_string(type), (unsigned) tr->anchor_job->id); + + if (_ret) +- *_ret = ret; ++ *_ret = tr->anchor_job; + + transaction_free(tr); + return 0; +diff --git a/src/transaction.c b/src/transaction.c +index c00dd45..d7ecfdb 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -834,8 +834,7 @@ int transaction_add_job_and_dependencies( + bool conflicts, + bool ignore_requirements, + bool ignore_order, +- DBusError *e, +- Job **_ret) { ++ DBusError *e) { + Job *ret; + JobDependency *l; + Iterator i; +@@ -892,8 +891,11 @@ int transaction_add_job_and_dependencies( + return -ENOMEM; + + /* If the link has no subject job, it's the anchor link. */ +- if (!by) ++ if (!by) { + LIST_PREPEND(JobDependency, subject, tr->anchor, l); ++ if (!tr->anchor_job) ++ tr->anchor_job = ret; ++ } + + if (is_new && !ignore_requirements) { + Set *following; +@@ -902,7 +904,7 @@ int transaction_add_job_and_dependencies( + * add all dependencies of everybody following. */ + if (unit_following_set(ret->unit, &following) > 0) { + SET_FOREACH(dep, following, i) { +- r = transaction_add_job_and_dependencies(tr, type, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, false, 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)); + +@@ -917,7 +919,7 @@ int transaction_add_job_and_dependencies( + /* Finally, recursively add in all dependencies. */ + if (type == JOB_START || type == JOB_RELOAD_OR_START) { + 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, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e); + if (r < 0) { + if (r != -EBADR) + goto fail; +@@ -928,7 +930,7 @@ int transaction_add_job_and_dependencies( + } + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_BIND_TO], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, override, false, false, ignore_order, e); + if (r < 0) { + if (r != -EBADR) + goto fail; +@@ -939,7 +941,7 @@ 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, NULL); ++ 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)); + +@@ -949,7 +951,7 @@ 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, NULL); ++ 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)); + +@@ -959,7 +961,7 @@ int transaction_add_job_and_dependencies( + } + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, true, override, false, false, ignore_order, e); + if (r < 0) { + if (r != -EBADR) + goto fail; +@@ -970,7 +972,7 @@ 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, NULL); ++ 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)); + +@@ -980,7 +982,7 @@ int transaction_add_job_and_dependencies( + } + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTS], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, true, override, true, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, true, override, true, false, ignore_order, e); + if (r < 0) { + if (r != -EBADR) + goto fail; +@@ -991,7 +993,7 @@ int transaction_add_job_and_dependencies( + } + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, false, 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)); + +@@ -1005,7 +1007,7 @@ int transaction_add_job_and_dependencies( + if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) { + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRED_BY], i) { +- r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e); + if (r < 0) { + if (r != -EBADR) + goto fail; +@@ -1016,7 +1018,7 @@ int transaction_add_job_and_dependencies( + } + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_BOUND_BY], i) { +- r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e); + if (r < 0) { + if (r != -EBADR) + goto fail; +@@ -1030,7 +1032,7 @@ int transaction_add_job_and_dependencies( + if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START) { + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_PROPAGATE_RELOAD_TO], i) { +- r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e); + if (r < 0) { + log_warning("Cannot add dependency reload job for unit %s, ignoring: %s", dep->id, bus_error(e, r)); + +@@ -1043,9 +1045,6 @@ int transaction_add_job_and_dependencies( + /* JOB_VERIFY_STARTED, JOB_RELOAD require no dependency handling */ + } + +- if (_ret) +- *_ret = ret; +- + return 0; + + fail: +@@ -1078,7 +1077,7 @@ int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { + if (hashmap_get(tr->jobs, u)) + continue; + +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, NULL, true, false, false, false, false, NULL, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, NULL, true, false, false, false, false, NULL); + if (r < 0) + log_warning("Cannot add isolate job for unit %s, ignoring: %s", u->id, strerror(-r)); + } +diff --git a/src/transaction.h b/src/transaction.h +index d519ffb..4818cea 100644 +--- a/src/transaction.h ++++ b/src/transaction.h +@@ -12,6 +12,7 @@ struct Transaction { + /* Jobs to be added */ + Hashmap *jobs; /* Unit object => Job object list 1:1 */ + JobDependency *anchor; ++ Job *anchor_job; /* the job the user asked for */ + }; + + Transaction *transaction_new(void); +@@ -27,8 +28,7 @@ int transaction_add_job_and_dependencies( + bool conflicts, + bool ignore_requirements, + bool ignore_order, +- DBusError *e, +- Job **_ret); ++ DBusError *e); + int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e); + int transaction_add_isolate_jobs(Transaction *tr, Manager *m); + void transaction_abort(Transaction *tr); +-- +1.7.7 + + +From 3020dc5e2b840b8732cfdb81c322a47120a0a42f Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 10:33:37 +0200 +Subject: [PATCH 13/30] transaction: change the linking of isolate jobs to the + anchor + +When isolating, the JOB_STOP jobs have no parent job, so they are all peers +of the real anchor job. This is a bit odd. + +Link them from the anchor job. +(cherry picked from commit 4483f694983382b4092e3e295ffb59926cff96d9) +--- + src/transaction.c | 6 +++--- + 1 files changed, 3 insertions(+), 3 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index d7ecfdb..09abe00 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -893,8 +893,8 @@ int transaction_add_job_and_dependencies( + /* If the link has no subject job, it's the anchor link. */ + if (!by) { + LIST_PREPEND(JobDependency, subject, tr->anchor, l); +- if (!tr->anchor_job) +- tr->anchor_job = ret; ++ assert(!tr->anchor_job); ++ tr->anchor_job = ret; + } + + if (is_new && !ignore_requirements) { +@@ -1077,7 +1077,7 @@ int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { + if (hashmap_get(tr->jobs, u)) + continue; + +- r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, NULL, true, false, false, false, false, NULL); ++ r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, tr->anchor_job, true, false, false, false, false, NULL); + if (r < 0) + log_warning("Cannot add isolate job for unit %s, ignoring: %s", u->id, strerror(-r)); + } +-- +1.7.7 + + +From 1ceeee2cf56d60ac37199ba9ebbe02144200857c Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 01:20:14 +0200 +Subject: [PATCH 14/30] transaction: simplify + transaction_find_jobs_that_matter_to_anchor() (cherry + picked from commit + 0d9989aa68963037a18fb7ed4f309f6155927d70) + +--- + src/transaction.c | 19 ++++++------------- + 1 files changed, 6 insertions(+), 13 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index 09abe00..cac58e6 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -38,22 +38,18 @@ void transaction_abort(Transaction *tr) { + assert(!tr->anchor); + } + +-static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, Job *j, unsigned generation) { ++static void transaction_find_jobs_that_matter_to_anchor(Job *j, unsigned generation) { + JobDependency *l; + +- assert(tr); +- + /* A recursive sweep through the graph that marks all units + * that matter to the anchor job, i.e. are directly or + * indirectly a dependency of the anchor job via paths that + * are fully marked as mattering. */ + +- if (j) +- l = j->subject_list; +- else +- l = tr->anchor; ++ j->matters_to_anchor = true; ++ j->generation = generation; + +- LIST_FOREACH(subject, l, l) { ++ LIST_FOREACH(subject, l, j->subject_list) { + + /* This link does not matter */ + if (!l->matters) +@@ -63,10 +59,7 @@ static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, Job *j, + if (l->object->generation == generation) + continue; + +- l->object->matters_to_anchor = true; +- l->object->generation = generation; +- +- transaction_find_jobs_that_matter_to_anchor(tr, l->object, generation); ++ transaction_find_jobs_that_matter_to_anchor(l->object, generation); + } + } + +@@ -659,7 +652,7 @@ int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e + * the actual list of jobs, if possible. */ + + /* First step: figure out which jobs matter */ +- transaction_find_jobs_that_matter_to_anchor(tr, NULL, generation++); ++ transaction_find_jobs_that_matter_to_anchor(tr->anchor_job, generation++); + + /* Second step: Try not to stop any running services if + * we don't have to. Don't try to reverse running +-- +1.7.7 + + +From ba088fafbe639c1a8e859406e9f82d0d8bffc804 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 02:48:24 +0200 +Subject: [PATCH 15/30] transaction: avoid garbage collecting the anchor job + +Make sure the anchor job is never considered garbage, even if it has no links +leading to it (this will be allowed in the next patch). +(cherry picked from commit 38809d9dfed4c75d9e97c4e5da2ff957723c4cad) +--- + src/transaction.c | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index cac58e6..ddb02c0 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -454,7 +454,7 @@ static void transaction_collect_garbage(Transaction *tr) { + again = false; + + HASHMAP_FOREACH(j, tr->jobs, i) { +- if (j->object_list) { ++ 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", */ +-- +1.7.7 + + +From 436271253850ec21fccfaa435fa2c6ce445721ac Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 02:11:14 +0200 +Subject: [PATCH 16/30] transaction: remove the anchor link + +tr->anchor_job is sufficient. +(cherry picked from commit e6eda1f23efab618bb26e7015230d8552b401dc6) +--- + src/job.c | 12 ------------ + src/job.h | 2 -- + src/transaction.c | 31 ++++++++----------------------- + src/transaction.h | 1 - + 4 files changed, 8 insertions(+), 38 deletions(-) + +diff --git a/src/job.c b/src/job.c +index a5ce2d4..6b93b63 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -151,18 +151,6 @@ void job_dump(Job *j, FILE*f, const char *prefix) { + prefix, yes_no(j->override)); + } + +-bool job_is_anchor(Job *j) { +- JobDependency *l; +- +- assert(j); +- +- LIST_FOREACH(object, l, j->object_list) +- if (!l->subject) +- return true; +- +- return false; +-} +- + /* + * Merging is commutative, so imagine the matrix as symmetric. We store only + * its lower triangle to avoid duplication. We don't store the main diagonal, +diff --git a/src/job.h b/src/job.h +index cbd10c5..6b06c2a 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -144,8 +144,6 @@ void job_dump(Job *j, FILE*f, const char *prefix); + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts); + void job_dependency_free(JobDependency *l); + +-bool job_is_anchor(Job *j); +- + int job_merge(Job *j, Job *other); + + JobType job_type_lookup_merge(JobType a, JobType b); +diff --git a/src/transaction.c b/src/transaction.c +index ddb02c0..39cfe54 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -34,8 +34,6 @@ void transaction_abort(Transaction *tr) { + transaction_delete_job(tr, j, true); + + assert(hashmap_isempty(tr->jobs)); +- +- assert(!tr->anchor); + } + + static void transaction_find_jobs_that_matter_to_anchor(Job *j, unsigned generation) { +@@ -287,7 +285,7 @@ static void transaction_drop_redundant(Transaction *tr) { + + LIST_FOREACH(transaction, k, j) { + +- if (!job_is_anchor(k) && ++ if (tr->anchor_job != k && + (k->installed || 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; +@@ -626,8 +624,6 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); + } + +- assert(!tr->anchor); +- + return 0; + + rollback: +@@ -726,7 +722,6 @@ int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e + } + + assert(hashmap_isempty(tr->jobs)); +- assert(!tr->anchor); + + return 0; + } +@@ -778,12 +773,6 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b + return j; + } + +-static void transaction_job_dependency_free(Transaction *tr, JobDependency *l) { +- if (!l->subject) +- LIST_REMOVE(JobDependency, subject, tr->anchor, l); +- job_dependency_free(l); +-} +- + static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies) { + assert(tr); + assert(j); +@@ -801,12 +790,12 @@ static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependen + j->transaction_prev = j->transaction_next = NULL; + + while (j->subject_list) +- transaction_job_dependency_free(tr, j->subject_list); ++ job_dependency_free(j->subject_list); + + while (j->object_list) { + Job *other = j->object_list->matters ? j->object_list->subject : NULL; + +- transaction_job_dependency_free(tr, j->object_list); ++ job_dependency_free(j->object_list); + + if (other && delete_dependencies) { + log_debug("Deleting job %s/%s as dependency of job %s/%s", +@@ -829,7 +818,6 @@ int transaction_add_job_and_dependencies( + bool ignore_order, + DBusError *e) { + Job *ret; +- JobDependency *l; + Iterator i; + Unit *dep; + int r; +@@ -879,17 +867,14 @@ int transaction_add_job_and_dependencies( + ret->ignore_order = ret->ignore_order || ignore_order; + + /* Then, add a link to the job. */ +- l = job_dependency_new(by, ret, matters, conflicts); +- if (!l) +- return -ENOMEM; +- +- /* If the link has no subject job, it's the anchor link. */ +- if (!by) { +- LIST_PREPEND(JobDependency, subject, tr->anchor, l); ++ if (by) { ++ if (!job_dependency_new(by, ret, matters, conflicts)) ++ return -ENOMEM; ++ } else { ++ /* If the job has no parent job, it is the anchor job. */ + assert(!tr->anchor_job); + tr->anchor_job = ret; + } +- + if (is_new && !ignore_requirements) { + Set *following; + +diff --git a/src/transaction.h b/src/transaction.h +index 4818cea..74d7461 100644 +--- a/src/transaction.h ++++ b/src/transaction.h +@@ -11,7 +11,6 @@ typedef struct Transaction Transaction; + struct Transaction { + /* Jobs to be added */ + Hashmap *jobs; /* Unit object => Job object list 1:1 */ +- JobDependency *anchor; + Job *anchor_job; /* the job the user asked for */ + }; + +-- +1.7.7 + + +From cdfe041c84fb813d883de58003e4a986ebde35e6 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Thu, 19 Apr 2012 23:56:26 +0200 +Subject: [PATCH 17/30] transaction: remove a couple of asserts + +We already asserted these facts in the previous loop. +(cherry picked from commit f1c2bdca422dba1bc5615f72662dee5ce69c147b) +--- + src/transaction.c | 3 --- + 1 files changed, 0 insertions(+), 3 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index 39cfe54..41f7b82 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -611,9 +611,6 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + /* We're fully installed. Now let's free data we don't + * need anymore. */ + +- assert(!j->transaction_next); +- assert(!j->transaction_prev); +- + /* Clean the job dependencies */ + transaction_unlink_job(tr, j, false); + +-- +1.7.7 + + +From ff3ec7f35ab49a1e306f90b33866f4dc7fba282f Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 02:04:01 +0200 +Subject: [PATCH 18/30] job: separate job_install() + +Let the jobs install themselves. +(cherry picked from commit 05d576f1f77699ea5c2e5ed0e04451b14dfc45a0) +--- + src/job.c | 44 +++++++++++++++++++++++++++++--------------- + src/job.h | 3 ++- + src/transaction.c | 18 ++---------------- + 3 files changed, 33 insertions(+), 32 deletions(-) + +diff --git a/src/job.c b/src/job.c +index 6b93b63..c5bf9cd 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -54,21 +54,6 @@ Job* job_new(Unit *unit, JobType type) { + return j; + } + +-void job_uninstall(Job *j) { +- assert(j->installed); +- /* Detach from next 'bigger' objects */ +- +- bus_job_send_removed_signal(j); +- +- if (j->unit->job == j) { +- j->unit->job = NULL; +- unit_add_to_gc_queue(j->unit); +- } +- +- hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); +- j->installed = false; +-} +- + void job_free(Job *j) { + assert(j); + assert(!j->installed); +@@ -96,6 +81,35 @@ void job_free(Job *j) { + free(j); + } + ++void job_uninstall(Job *j) { ++ assert(j->installed); ++ /* Detach from next 'bigger' objects */ ++ ++ bus_job_send_removed_signal(j); ++ ++ if (j->unit->job == j) { ++ j->unit->job = NULL; ++ unit_add_to_gc_queue(j->unit); ++ } ++ ++ hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); ++ j->installed = false; ++} ++ ++void job_install(Job *j) { ++ Job *uj = j->unit->job; ++ ++ if (uj) { ++ job_uninstall(uj); ++ job_free(uj); ++ } ++ ++ j->unit->job = j; ++ j->installed = true; ++ j->manager->n_installed_jobs ++; ++ log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); ++} ++ + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) { + JobDependency *l; + +diff --git a/src/job.h b/src/job.h +index 6b06c2a..8fa9046 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -137,8 +137,9 @@ struct Job { + }; + + Job* job_new(Unit *unit, JobType type); +-void job_uninstall(Job *j); + void job_free(Job *job); ++void job_install(Job *j); ++void job_uninstall(Job *j); + void job_dump(Job *j, FILE*f, const char *prefix); + + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts); +diff --git a/src/transaction.c b/src/transaction.c +index 41f7b82..d495cbd 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -592,33 +592,19 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + } + + while ((j = hashmap_steal_first(tr->jobs))) { +- Job *uj; + if (j->installed) { + /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ + continue; + } + +- uj = j->unit->job; +- if (uj) { +- job_uninstall(uj); +- job_free(uj); +- } +- +- j->unit->job = j; +- j->installed = true; +- m->n_installed_jobs ++; +- +- /* We're fully installed. Now let's free data we don't +- * need anymore. */ +- + /* Clean the job dependencies */ + transaction_unlink_job(tr, j, false); + ++ job_install(j); ++ + job_add_to_run_queue(j); + job_add_to_dbus_queue(j); + job_start_timer(j); +- +- log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); + } + + return 0; +-- +1.7.7 + + +From 78c1339da1446bac0f4a5fbb0f1add66a6246bec Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 09:38:43 +0200 +Subject: [PATCH 19/30] transaction: rework merging with installed jobs + +Previously transactions could reference installed jobs. It made some issues +difficult to fix. + +This sets new rules for jobs: +A job cannot be both a member of a transaction and installed. When jobs are +created, they are linked to a transaction. The whole transaction is constructed +(with merging of jobs within, etc.). When it's complete, all the jobs are +unlinked from it one by one and let to install themselves. It is during the +installation when merging with previously installed jobs (from older +transactions) is contemplated. + +Merging with installed jobs has different rules than merging within a +transaction: + - An installed conflicting job gets cancelled. It cannot be simply deleted, + because someone might be waiting for its completion on DBus. + - An installed, but still waiting, job can be safely merged into. + - An installed and running job can be tricky. For some job types it is safe to + just merge. For the other types we merge anyway, but put the job back into + JOB_WAITING to allow it to run again. This may be suboptimal, but it is not + currently possible to have more than one installed job for a unit. + +Note this also fixes a bug where the anchor job could be deleted during merging +within the transaction. +(cherry picked from commit 656bbffc6c45bdd8d5c28a96ca948ba16c546547) +--- + src/job.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-- + src/job.h | 2 +- + src/transaction.c | 21 +++++++++++-------- + 3 files changed, 68 insertions(+), 13 deletions(-) + +diff --git a/src/job.c b/src/job.c +index c5bf9cd..e6737df 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -96,18 +96,70 @@ void job_uninstall(Job *j) { + j->installed = false; + } + +-void job_install(Job *j) { ++static bool job_type_allows_late_merge(JobType t) { ++ /* Tells whether it is OK to merge a job of type 't' with an already ++ * running job. ++ * Reloads cannot be merged this way. Think of the sequence: ++ * 1. Reload of a daemon is in progress; the daemon has already loaded ++ * its config file, but hasn't completed the reload operation yet. ++ * 2. Edit foo's config file. ++ * 3. Trigger another reload to have the daemon use the new config. ++ * Should the second reload job be merged into the first one, the daemon ++ * would not know about the new config. ++ * JOB_RESTART jobs on the other hand can be merged, because they get ++ * patched into JOB_START after stopping the unit. So if we see a ++ * JOB_RESTART running, it means the unit hasn't stopped yet and at ++ * this time the merge is still allowed. */ ++ return !(t == JOB_RELOAD || t == JOB_RELOAD_OR_START); ++} ++ ++static void job_merge_into_installed(Job *j, Job *other) { ++ assert(j->installed); ++ assert(j->unit == other->unit); ++ ++ j->type = job_type_lookup_merge(j->type, other->type); ++ assert(j->type >= 0); ++ ++ j->override = j->override || other->override; ++} ++ ++Job* job_install(Job *j) { + Job *uj = j->unit->job; + ++ assert(!j->installed); ++ + if (uj) { +- job_uninstall(uj); +- job_free(uj); ++ if (job_type_is_conflicting(uj->type, j->type)) ++ job_finish_and_invalidate(uj, JOB_CANCELED); ++ else { ++ /* not conflicting, i.e. mergeable */ ++ ++ if (uj->state == JOB_WAITING || ++ (job_type_allows_late_merge(j->type) && job_type_is_superset(uj->type, j->type))) { ++ job_merge_into_installed(uj, j); ++ log_debug("Merged into installed job %s/%s as %u", ++ uj->unit->id, job_type_to_string(uj->type), (unsigned) uj->id); ++ return uj; ++ } else { ++ /* already running and not safe to merge into */ ++ /* Patch uj to become a merged job and re-run it. */ ++ /* XXX It should be safer to queue j to run after uj finishes, but it is ++ * not currently possible to have more than one installed job per unit. */ ++ job_merge_into_installed(uj, j); ++ log_debug("Merged into running job, re-running: %s/%s as %u", ++ uj->unit->id, job_type_to_string(uj->type), (unsigned) uj->id); ++ uj->state = JOB_WAITING; ++ return uj; ++ } ++ } + } + ++ /* Install the job */ + j->unit->job = j; + j->installed = true; + j->manager->n_installed_jobs ++; + log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); ++ return j; + } + + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) { +diff --git a/src/job.h b/src/job.h +index 8fa9046..eab0e07 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -138,7 +138,7 @@ struct Job { + + Job* job_new(Unit *unit, JobType type); + void job_free(Job *job); +-void job_install(Job *j); ++Job* job_install(Job *j); + void job_uninstall(Job *j); + void job_dump(Job *j, FILE*f, const char *prefix); + +diff --git a/src/transaction.c b/src/transaction.c +index d495cbd..aa0cedf 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -243,21 +243,14 @@ static int transaction_merge_jobs(Transaction *tr, DBusError *e) { + LIST_FOREACH(transaction, k, j->transaction_next) + assert_se(job_type_merge(&t, k->type) == 0); + +- /* If an active job is mergeable, merge it too */ +- if (j->unit->job) +- job_type_merge(&t, j->unit->job->type); /* Might fail. Which is OK */ +- + while ((k = j->transaction_next)) { +- if (j->installed) { ++ if (tr->anchor_job == k) { + transaction_merge_and_delete_job(tr, k, j, t); + j = k; + } else + transaction_merge_and_delete_job(tr, j, k, t); + } + +- if (j->unit->job && !j->installed) +- transaction_merge_and_delete_job(tr, j, j->unit->job, t); +- + assert(!j->transaction_next); + assert(!j->transaction_prev); + } +@@ -592,6 +585,8 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + } + + while ((j = hashmap_steal_first(tr->jobs))) { ++ Job *installed_job; ++ + if (j->installed) { + /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ + continue; +@@ -600,7 +595,15 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + /* Clean the job dependencies */ + transaction_unlink_job(tr, j, false); + +- job_install(j); ++ installed_job = job_install(j); ++ if (installed_job != j) { ++ /* j has been merged into a previously installed job */ ++ if (tr->anchor_job == j) ++ tr->anchor_job = installed_job; ++ hashmap_remove(m->jobs, UINT32_TO_PTR(j->id)); ++ job_free(j); ++ j = installed_job; ++ } + + job_add_to_run_queue(j); + job_add_to_dbus_queue(j); +-- +1.7.7 + + +From 3d47dab6ed99778f9812ec4039135feb950f02da Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 10:02:05 +0200 +Subject: [PATCH 20/30] transaction: remove checks for installed + +Transactions cannot contain installed jobs anymore. Remove the now pointless +checks. +(cherry picked from commit d6a093d098054b6fe866441251ad9485c9e31584) +--- + src/job.c | 7 +++---- + src/transaction.c | 21 +++------------------ + 2 files changed, 6 insertions(+), 22 deletions(-) + +diff --git a/src/job.c b/src/job.c +index e6737df..fb759a5 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -83,14 +83,13 @@ void job_free(Job *j) { + + void job_uninstall(Job *j) { + assert(j->installed); ++ assert(j->unit->job == j); + /* Detach from next 'bigger' objects */ + + bus_job_send_removed_signal(j); + +- if (j->unit->job == j) { +- j->unit->job = NULL; +- unit_add_to_gc_queue(j->unit); +- } ++ j->unit->job = NULL; ++ unit_add_to_gc_queue(j->unit); + + hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); + j->installed = false; +diff --git a/src/transaction.c b/src/transaction.c +index aa0cedf..c3e1e13 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -11,8 +11,7 @@ static void transaction_delete_job(Transaction *tr, Job *j, bool delete_dependen + + transaction_unlink_job(tr, j, delete_dependencies); + +- if (!j->installed) +- job_free(j); ++ job_free(j); + } + + static void transaction_delete_unit(Transaction *tr, Unit *u) { +@@ -279,7 +278,7 @@ static void transaction_drop_redundant(Transaction *tr) { + LIST_FOREACH(transaction, k, j) { + + if (tr->anchor_job != k && +- (k->installed || job_type_is_redundant(k->type, unit_active_state(k->unit))) && ++ 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; + +@@ -349,7 +348,6 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi + log_info("Walked on cycle path to %s/%s", k->unit->id, job_type_to_string(k->type)); + + if (!delete && +- !k->installed && + !unit_matters_to_anchor(k->unit, k)) { + /* Ok, we can drop this one, so let's + * do so. */ +@@ -478,7 +476,6 @@ static int transaction_is_destructive(Transaction *tr, DBusError *e) { + assert(!j->transaction_next); + + if (j->unit->job && +- j->unit->job != j && + !job_type_is_superset(j->type, j->unit->job->type)) { + + dbus_set_error(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE, "Transaction is destructive."); +@@ -576,9 +573,6 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + assert(!j->transaction_prev); + assert(!j->transaction_next); + +- if (j->installed) +- continue; +- + r = hashmap_put(m->jobs, UINT32_TO_PTR(j->id), j); + if (r < 0) + goto rollback; +@@ -587,11 +581,6 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + while ((j = hashmap_steal_first(tr->jobs))) { + Job *installed_job; + +- if (j->installed) { +- /* log_debug("Skipping already installed job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); */ +- continue; +- } +- + /* Clean the job dependencies */ + transaction_unlink_job(tr, j, false); + +@@ -614,12 +603,8 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + + rollback: + +- HASHMAP_FOREACH(j, tr->jobs, i) { +- if (j->installed) +- continue; +- ++ HASHMAP_FOREACH(j, tr->jobs, i) + hashmap_remove(m->jobs, UINT32_TO_PTR(j->id)); +- } + + return r; + } +-- +1.7.7 + + +From 55284fc0ba28bf10d4fa535a86921bad23505c96 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 12:28:31 +0200 +Subject: [PATCH 21/30] dbus-job: allow multiple bus clients + +Merging of jobs can result in more than one client being interested in a job. +(cherry picked from commit 97e6a11996855800f68dc41c13537784198c8b61) +--- + src/dbus-job.c | 146 +++++++++++++++++++++++++++++++--------------------- + src/dbus-manager.c | 6 ++- + src/dbus.c | 14 +++-- + src/job.c | 21 +++++++- + src/job.h | 15 ++++- + 5 files changed, 131 insertions(+), 71 deletions(-) + +diff --git a/src/dbus-job.c b/src/dbus-job.c +index ab6d610..aa1b4eb 100644 +--- a/src/dbus-job.c ++++ b/src/dbus-job.c +@@ -225,61 +225,71 @@ const DBusObjectPathVTable bus_job_vtable = { + .message_function = bus_job_message_handler + }; + +-static int job_send_message(Job *j, DBusMessage *m) { ++static int job_send_message(Job *j, DBusMessage* (*new_message)(Job *j)) { ++ DBusMessage *m = NULL; + int r; + + assert(j); +- assert(m); ++ assert(new_message); + + if (bus_has_subscriber(j->manager)) { +- if ((r = bus_broadcast(j->manager, m)) < 0) ++ m = new_message(j); ++ if (!m) ++ goto oom; ++ r = bus_broadcast(j->manager, m); ++ dbus_message_unref(m); ++ if (r < 0) + return r; + +- } else if (j->bus_client) { ++ } else { + /* If nobody is subscribed, we just send the message +- * to the client which created the job */ ++ * to the client(s) which created the job */ ++ JobBusClient *cl; ++ assert(j->bus_client_list); ++ LIST_FOREACH(client, cl, j->bus_client_list) { ++ assert(cl->bus); + +- assert(j->bus); ++ m = new_message(j); ++ if (!m) ++ goto oom; + +- if (!dbus_message_set_destination(m, j->bus_client)) +- return -ENOMEM; ++ if (!dbus_message_set_destination(m, cl->name)) ++ goto oom; ++ ++ if (!dbus_connection_send(cl->bus, m, NULL)) ++ goto oom; + +- if (!dbus_connection_send(j->bus, m, NULL)) +- return -ENOMEM; ++ dbus_message_unref(m); ++ m = NULL; ++ } + } + + return 0; ++oom: ++ if (m) ++ dbus_message_unref(m); ++ return -ENOMEM; + } + +-void bus_job_send_change_signal(Job *j) { +- char *p = NULL; ++static DBusMessage* new_change_signal_message(Job *j) { + DBusMessage *m = NULL; ++ char *p = NULL; + +- assert(j); +- +- if (j->in_dbus_queue) { +- LIST_REMOVE(Job, dbus_queue, j->manager->dbus_job_queue, j); +- j->in_dbus_queue = false; +- } +- +- if (!bus_has_subscriber(j->manager) && !j->bus_client) { +- j->sent_dbus_new_signal = true; +- return; +- } +- +- if (!(p = job_dbus_path(j))) ++ p = job_dbus_path(j); ++ if (!p) + goto oom; + + if (j->sent_dbus_new_signal) { + /* Send a properties changed signal */ +- +- if (!(m = bus_properties_changed_new(p, "org.freedesktop.systemd1.Job", INVALIDATING_PROPERTIES))) ++ m = bus_properties_changed_new(p, "org.freedesktop.systemd1.Job", INVALIDATING_PROPERTIES); ++ if (!m) + goto oom; + + } else { + /* Send a new signal */ + +- if (!(m = dbus_message_new_signal("/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "JobNew"))) ++ m = dbus_message_new_signal("/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "JobNew"); ++ if (!m) + goto oom; + + if (!dbus_message_append_args(m, +@@ -289,42 +299,26 @@ void bus_job_send_change_signal(Job *j) { + goto oom; + } + +- if (job_send_message(j, m) < 0) +- goto oom; +- +- free(p); +- dbus_message_unref(m); +- +- j->sent_dbus_new_signal = true; +- +- return; ++ return m; + + oom: +- free(p); +- + if (m) + dbus_message_unref(m); +- +- log_error("Failed to allocate job change signal."); ++ free(p); ++ return NULL; + } + +-void bus_job_send_removed_signal(Job *j) { +- char *p = NULL; ++static DBusMessage* new_removed_signal_message(Job *j) { + DBusMessage *m = NULL; ++ char *p = NULL; + const char *r; + +- assert(j); +- +- if (!bus_has_subscriber(j->manager) && !j->bus_client) +- return; +- +- if (!j->sent_dbus_new_signal) +- bus_job_send_change_signal(j); +- +- if (!(p = job_dbus_path(j))) ++ p = job_dbus_path(j); ++ if (!p) + goto oom; + +- if (!(m = dbus_message_new_signal("/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "JobRemoved"))) ++ m = dbus_message_new_signal("/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "JobRemoved"); ++ if (!m) + goto oom; + + r = job_result_to_string(j->result); +@@ -336,19 +330,53 @@ void bus_job_send_removed_signal(Job *j) { + DBUS_TYPE_INVALID)) + goto oom; + +- if (job_send_message(j, m) < 0) +- goto oom; ++ return m; + ++oom: ++ if (m) ++ dbus_message_unref(m); + free(p); +- dbus_message_unref(m); ++ return NULL; ++} ++ ++void bus_job_send_change_signal(Job *j) { ++ assert(j); ++ ++ if (j->in_dbus_queue) { ++ LIST_REMOVE(Job, dbus_queue, j->manager->dbus_job_queue, j); ++ j->in_dbus_queue = false; ++ } ++ ++ if (!bus_has_subscriber(j->manager) && !j->bus_client_list) { ++ j->sent_dbus_new_signal = true; ++ return; ++ } ++ ++ if (job_send_message(j, new_change_signal_message) < 0) ++ goto oom; ++ ++ j->sent_dbus_new_signal = true; + + return; + + oom: +- free(p); ++ log_error("Failed to allocate job change signal."); ++} + +- if (m) +- dbus_message_unref(m); ++void bus_job_send_removed_signal(Job *j) { ++ assert(j); ++ ++ if (!bus_has_subscriber(j->manager) && !j->bus_client_list) ++ return; ++ ++ if (!j->sent_dbus_new_signal) ++ bus_job_send_change_signal(j); + ++ if (job_send_message(j, new_removed_signal_message) < 0) ++ goto oom; ++ ++ return; ++ ++oom: + log_error("Failed to allocate job remove signal."); + } +diff --git a/src/dbus-manager.c b/src/dbus-manager.c +index 6d272cb..0b78375 100644 +--- a/src/dbus-manager.c ++++ b/src/dbus-manager.c +@@ -1435,6 +1435,7 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection, + const char *name, *smode, *old_name = NULL; + JobMode mode; + Job *j; ++ JobBusClient *cl; + Unit *u; + bool b; + +@@ -1492,10 +1493,11 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection, + if ((r = manager_add_job(m, job_type, u, mode, true, &error, &j)) < 0) + return bus_send_error_reply(connection, message, &error, r); + +- if (!(j->bus_client = strdup(message_get_sender_with_fallback(message)))) ++ cl = job_bus_client_new(connection, message_get_sender_with_fallback(message)); ++ if (!cl) + goto oom; + +- j->bus = connection; ++ LIST_PREPEND(JobBusClient, client, j->bus_client_list, cl); + + if (!(reply = dbus_message_new_method_return(message))) + goto oom; +diff --git a/src/dbus.c b/src/dbus.c +index 8e6e9fd..50e8c9c 100644 +--- a/src/dbus.c ++++ b/src/dbus.c +@@ -1166,13 +1166,15 @@ static void shutdown_connection(Manager *m, DBusConnection *c) { + Job *j; + Iterator i; + +- HASHMAP_FOREACH(j, m->jobs, i) +- if (j->bus == c) { +- free(j->bus_client); +- j->bus_client = NULL; +- +- j->bus = NULL; ++ HASHMAP_FOREACH(j, m->jobs, i) { ++ JobBusClient *cl, *nextcl; ++ LIST_FOREACH_SAFE(client, cl, nextcl, j->bus_client_list) { ++ if (cl->bus == c) { ++ LIST_REMOVE(JobBusClient, client, j->bus_client_list, cl); ++ free(cl); ++ } + } ++ } + + set_remove(m->bus_connections, c); + set_remove(m->bus_connections_for_dispatch, c); +diff --git a/src/job.c b/src/job.c +index fb759a5..de74751 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -33,6 +33,20 @@ + #include "log.h" + #include "dbus-job.h" + ++JobBusClient* job_bus_client_new(DBusConnection *connection, const char *name) { ++ JobBusClient *cl; ++ size_t name_len; ++ ++ name_len = strlen(name); ++ cl = malloc0(sizeof(JobBusClient) + name_len + 1); ++ if (!cl) ++ return NULL; ++ ++ cl->bus = connection; ++ memcpy(cl->name, name, name_len + 1); ++ return cl; ++} ++ + Job* job_new(Unit *unit, JobType type) { + Job *j; + +@@ -55,6 +69,8 @@ Job* job_new(Unit *unit, JobType type) { + } + + void job_free(Job *j) { ++ JobBusClient *cl; ++ + assert(j); + assert(!j->installed); + assert(!j->transaction_prev); +@@ -77,7 +93,10 @@ void job_free(Job *j) { + close_nointr_nofail(j->timer_watch.fd); + } + +- free(j->bus_client); ++ while ((cl = j->bus_client_list)) { ++ LIST_REMOVE(JobBusClient, client, j->bus_client_list, cl); ++ free(cl); ++ } + free(j); + } + +diff --git a/src/job.h b/src/job.h +index eab0e07..2025b5b 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -28,6 +28,7 @@ + + typedef struct Job Job; + typedef struct JobDependency JobDependency; ++typedef struct JobBusClient JobBusClient; + typedef enum JobType JobType; + typedef enum JobState JobState; + typedef enum JobMode JobMode; +@@ -99,6 +100,13 @@ struct JobDependency { + bool conflicts; + }; + ++struct JobBusClient { ++ LIST_FIELDS(JobBusClient, client); ++ /* Note that this bus object is not ref counted here. */ ++ DBusConnection *bus; ++ char name[0]; ++}; ++ + struct Job { + Manager *manager; + Unit *unit; +@@ -121,9 +129,8 @@ struct Job { + + Watch timer_watch; + +- /* Note that this bus object is not ref counted here. */ +- DBusConnection *bus; +- char *bus_client; ++ /* There can be more than one client, because of job merging. */ ++ LIST_HEAD(JobBusClient, bus_client_list); + + JobResult result; + +@@ -136,6 +143,8 @@ struct Job { + bool ignore_order:1; + }; + ++JobBusClient* job_bus_client_new(DBusConnection *connection, const char *name); ++ + Job* job_new(Unit *unit, JobType type); + void job_free(Job *job); + Job* job_install(Job *j); +-- +1.7.7 + + +From 45aca411e44be019c9ba1bc1500e0b7df73bbf75 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Fri, 20 Apr 2012 14:44:00 +0200 +Subject: [PATCH 22/30] transaction: add starting requirements for JOB_RESTART + +While having a Requires= dependency between units, the dependency is started +automatically on "systemctl start", but it's not started on "systemctl +restart". + +JOB_RESTART jobs did not pull the dependencies for starting into the +transaction. + +https://bugzilla.redhat.com/show_bug.cgi?id=802770 + +Note that the other bug noted in comment #2 has been fixed already by avoiding +the deletion of anchor jobs. +(cherry picked from commit 65304075249449a713b4e4842b8538ef4aa1c725) +--- + src/transaction.c | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index c3e1e13..a2efcbc 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -866,7 +866,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) { +-- +1.7.7 + + +From 029699f2de954461054efb21e91994ca6e9df46a Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Sun, 22 Apr 2012 15:22:52 +0200 +Subject: [PATCH 23/30] transaction: downgrade warnings about masked units + (cherry picked from commit + 59e132a7f416d7c4a33a46d791f250e03d2c2cd0) + +--- + src/transaction.c | 11 +++++++---- + 1 files changed, 7 insertions(+), 4 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index a2efcbc..a36f1df 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -822,7 +822,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)) { +@@ -892,7 +892,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 +903,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 +925,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); +-- +1.7.7 + + +From 0ee2f52343ed0da6dc9006be54c9de57b762e563 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Sat, 21 Apr 2012 21:40:40 +0200 +Subject: [PATCH 24/30] transaction: improve readability + +The functions looked complicated with the nested loops with breaks, +continues, and "while (again)". +Here using goto actually makes them easier to understand. + +Also correcting the comment about redundant jobs. +(cherry picked from commit 055163ad15a5ca1eb5626c63fa7163e152698e2b) +--- + src/transaction.c | 152 ++++++++++++++++++++++------------------------------- + 1 files changed, 62 insertions(+), 90 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index a36f1df..2bd6541 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -258,44 +258,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 +418,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 +468,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; ++rescan: ++ 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; ++ /* 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 */ ++ /* Would this stop a running service? ++ * Would this change an existing job? ++ * If so, let's drop this entry */ + +- stops_running_service = +- j->type == JOB_STOP && UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(j->unit)); ++ stops_running_service = ++ j->type == JOB_STOP && UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(j->unit)); + +- changes_existing_job = +- j->unit->job && +- job_type_is_conflicting(j->type, j->unit->job->type); ++ changes_existing_job = ++ j->unit->job && ++ job_type_is_conflicting(j->type, j->unit->job->type); + +- if (!stops_running_service && !changes_existing_job) +- continue; ++ if (!stops_running_service && !changes_existing_job) ++ continue; + +- 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) ++ log_debug("%s/%s would stop a running service.", 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)); ++ if (changes_existing_job) ++ log_debug("%s/%s would change existing job.", 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)); ++ /* Ok, let's get rid of this */ ++ log_debug("Deleting %s/%s to minimize impact.", j->unit->id, job_type_to_string(j->type)); + +- transaction_delete_job(tr, j, true); +- again = true; +- break; +- } +- +- if (again) +- break; ++ transaction_delete_job(tr, j, true); ++ goto rescan; + } +- +- } while (again); ++ } + } + + static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { +-- +1.7.7 + + +From f613fa87549229f257444c41f5155fb1f5f30f44 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Sun, 22 Apr 2012 02:09:04 +0200 +Subject: [PATCH 25/30] transaction: fix detection of cycles involving + installed jobs + +A transaction can be acyclic, but when it's added to installed jobs, +a cycle may result. + +transaction_verify_order_one() attempts to detect these cases, but it +fails because the installed jobs often have the exact generation number +that makes them look as if they were walked already. + +Fix it by resetting the generation numbers of all installed jobs before +detecting cycles. + +An alternative fix could be to add the generation counter to the +Manager and use it instead of starting always from 1 in +transaction_activate(). But I prefer not having to worry about it +wrapping around. +(cherry picked from commit 4e7bd268ae1f39675988b9ac61b9378a67e3ae3e) +--- + src/transaction.c | 8 ++++++++ + 1 files changed, 8 insertions(+), 0 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index 2bd6541..9676b57 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -582,6 +582,8 @@ rollback: + } + + int transaction_activate(Transaction *tr, Manager *m, JobMode mode, DBusError *e) { ++ Iterator i; ++ Job *j; + int r; + unsigned generation = 1; + +@@ -590,6 +592,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++); + +-- +1.7.7 + + +From 374998cf2930534951bc0dace5025468cdf3c247 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Sun, 22 Apr 2012 10:54:58 +0200 +Subject: [PATCH 26/30] transaction: abort does not need to use recursive + deletion + +Recursion is unnecessary, because we're deleting all transaction jobs +anyway. And the recursive deletion produces debug messages that are +pointless in transaction abort. +(cherry picked from commit 1b9cea0caa85dce6d9f117638a296b141c49a8fd) +--- + src/transaction.c | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/src/transaction.c b/src/transaction.c +index 9676b57..b0a26f2 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -30,7 +30,7 @@ void transaction_abort(Transaction *tr) { + assert(tr); + + while ((j = hashmap_first(tr->jobs))) +- transaction_delete_job(tr, j, true); ++ transaction_delete_job(tr, j, false); + + assert(hashmap_isempty(tr->jobs)); + } +-- +1.7.7 + + +From 0d806f0c0bebf755867cfe0b3cc41d55aebea66a Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Mon, 23 Apr 2012 01:24:04 +0200 +Subject: [PATCH 27/30] job: serialize jobs properly + +Jobs were not preserved correctly over a daemon-reload operation. +A systemctl process waiting for a job completion received a job removal +signal. The job itself changed its id. The job timeout started ticking all +over again. + +This fixes the deficiencies. +(cherry picked from commit 39a18c60d07319ebfcfd476556729c2cadd616d6) +--- + src/dbus-job.c | 6 +- + src/job.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++++--- + src/job.h | 6 ++ + src/unit.c | 45 ++++++++++++--- + src/unit.h | 4 +- + 5 files changed, 203 insertions(+), 20 deletions(-) + +diff --git a/src/dbus-job.c b/src/dbus-job.c +index aa1b4eb..63e5846 100644 +--- a/src/dbus-job.c ++++ b/src/dbus-job.c +@@ -232,7 +232,7 @@ static int job_send_message(Job *j, DBusMessage* (*new_message)(Job *j)) { + assert(j); + assert(new_message); + +- if (bus_has_subscriber(j->manager)) { ++ if (bus_has_subscriber(j->manager) || j->forgot_bus_clients) { + m = new_message(j); + if (!m) + goto oom; +@@ -347,7 +347,7 @@ void bus_job_send_change_signal(Job *j) { + j->in_dbus_queue = false; + } + +- if (!bus_has_subscriber(j->manager) && !j->bus_client_list) { ++ if (!bus_has_subscriber(j->manager) && !j->bus_client_list && !j->forgot_bus_clients) { + j->sent_dbus_new_signal = true; + return; + } +@@ -366,7 +366,7 @@ oom: + void bus_job_send_removed_signal(Job *j) { + assert(j); + +- if (!bus_has_subscriber(j->manager) && !j->bus_client_list) ++ if (!bus_has_subscriber(j->manager) && !j->bus_client_list && !j->forgot_bus_clients) + return; + + if (!j->sent_dbus_new_signal) +diff --git a/src/job.c b/src/job.c +index de74751..69df024 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -47,22 +47,36 @@ JobBusClient* job_bus_client_new(DBusConnection *connection, const char *name) { + return cl; + } + +-Job* job_new(Unit *unit, JobType type) { ++Job* job_new_raw(Unit *unit) { + Job *j; + +- assert(type < _JOB_TYPE_MAX); ++ /* used for deserialization */ ++ + assert(unit); + +- if (!(j = new0(Job, 1))) ++ j = new0(Job, 1); ++ if (!j) + return NULL; + + j->manager = unit->manager; +- j->id = j->manager->current_job_id++; +- j->type = type; + j->unit = unit; +- + j->timer_watch.type = WATCH_INVALID; + ++ return j; ++} ++ ++Job* job_new(Unit *unit, JobType type) { ++ Job *j; ++ ++ assert(type < _JOB_TYPE_MAX); ++ ++ j = job_new_raw(unit); ++ if (!j) ++ return NULL; ++ ++ j->id = j->manager->current_job_id++; ++ j->type = type; ++ + /* We don't link it here, that's what job_dependency() is for */ + + return j; +@@ -105,7 +119,9 @@ void job_uninstall(Job *j) { + assert(j->unit->job == j); + /* Detach from next 'bigger' objects */ + +- bus_job_send_removed_signal(j); ++ /* daemon-reload should be transparent to job observers */ ++ if (j->manager->n_reloading <= 0) ++ bus_job_send_removed_signal(j); + + j->unit->job = NULL; + unit_add_to_gc_queue(j->unit); +@@ -180,6 +196,18 @@ Job* job_install(Job *j) { + return j; + } + ++void job_install_deserialized(Job *j) { ++ assert(!j->installed); ++ ++ if (j->unit->job) { ++ log_debug("Unit %s already has a job installed. Not installing deserialized job.", j->unit->id); ++ return; ++ } ++ j->unit->job = j; ++ j->installed = true; ++ log_debug("Reinstalled deserialized job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); ++} ++ + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) { + JobDependency *l; + +@@ -732,6 +760,126 @@ void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w) { + job_finish_and_invalidate(j, JOB_TIMEOUT); + } + ++int job_serialize(Job *j, FILE *f, FDSet *fds) { ++ fprintf(f, "job-id=%u\n", j->id); ++ fprintf(f, "job-type=%s\n", job_type_to_string(j->type)); ++ fprintf(f, "job-state=%s\n", job_state_to_string(j->state)); ++ fprintf(f, "job-override=%s\n", yes_no(j->override)); ++ fprintf(f, "job-sent-dbus-new-signal=%s\n", yes_no(j->sent_dbus_new_signal)); ++ fprintf(f, "job-ignore-order=%s\n", yes_no(j->ignore_order)); ++ /* Cannot save bus clients. Just note the fact that we're losing ++ * them. job_send_message() will fallback to broadcasting. */ ++ fprintf(f, "job-forgot-bus-clients=%s\n", ++ yes_no(j->forgot_bus_clients || j->bus_client_list)); ++ if (j->timer_watch.type == WATCH_JOB_TIMER) { ++ int copy = fdset_put_dup(fds, j->timer_watch.fd); ++ if (copy < 0) ++ return copy; ++ fprintf(f, "job-timer-watch-fd=%d\n", copy); ++ } ++ ++ /* End marker */ ++ fputc('\n', f); ++ return 0; ++} ++ ++int job_deserialize(Job *j, FILE *f, FDSet *fds) { ++ for (;;) { ++ char line[LINE_MAX], *l, *v; ++ size_t k; ++ ++ if (!fgets(line, sizeof(line), f)) { ++ if (feof(f)) ++ return 0; ++ return -errno; ++ } ++ ++ char_array_0(line); ++ l = strstrip(line); ++ ++ /* End marker */ ++ if (l[0] == 0) ++ return 0; ++ ++ k = strcspn(l, "="); ++ ++ if (l[k] == '=') { ++ l[k] = 0; ++ v = l+k+1; ++ } else ++ v = l+k; ++ ++ if (streq(l, "job-id")) { ++ if (safe_atou32(v, &j->id) < 0) ++ log_debug("Failed to parse job id value %s", v); ++ } else if (streq(l, "job-type")) { ++ JobType t = job_type_from_string(v); ++ if (t < 0) ++ log_debug("Failed to parse job type %s", v); ++ else ++ j->type = t; ++ } else if (streq(l, "job-state")) { ++ JobState s = job_state_from_string(v); ++ if (s < 0) ++ log_debug("Failed to parse job state %s", v); ++ else ++ j->state = s; ++ } else if (streq(l, "job-override")) { ++ int b = parse_boolean(v); ++ if (b < 0) ++ log_debug("Failed to parse job override flag %s", v); ++ else ++ j->override = j->override || b; ++ } else if (streq(l, "job-sent-dbus-new-signal")) { ++ int b = parse_boolean(v); ++ if (b < 0) ++ log_debug("Failed to parse job sent_dbus_new_signal flag %s", v); ++ else ++ j->sent_dbus_new_signal = j->sent_dbus_new_signal || b; ++ } else if (streq(l, "job-ignore-order")) { ++ int b = parse_boolean(v); ++ if (b < 0) ++ log_debug("Failed to parse job ignore_order flag %s", v); ++ else ++ j->ignore_order = j->ignore_order || b; ++ } else if (streq(l, "job-forgot-bus-clients")) { ++ int b = parse_boolean(v); ++ if (b < 0) ++ log_debug("Failed to parse job forgot_bus_clients flag %s", v); ++ else ++ j->forgot_bus_clients = j->forgot_bus_clients || b; ++ } else if (streq(l, "job-timer-watch-fd")) { ++ int fd; ++ if (safe_atoi(v, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd)) ++ log_debug("Failed to parse job-timer-watch-fd value %s", v); ++ else { ++ if (j->timer_watch.type == WATCH_JOB_TIMER) ++ close_nointr_nofail(j->timer_watch.fd); ++ ++ j->timer_watch.type = WATCH_JOB_TIMER; ++ j->timer_watch.fd = fdset_remove(fds, fd); ++ j->timer_watch.data.job = j; ++ } ++ } ++ } ++} ++ ++int job_coldplug(Job *j) { ++ struct epoll_event ev; ++ ++ if (j->timer_watch.type != WATCH_JOB_TIMER) ++ return 0; ++ ++ zero(ev); ++ ev.data.ptr = &j->timer_watch; ++ ev.events = EPOLLIN; ++ ++ if (epoll_ctl(j->manager->epoll_fd, EPOLL_CTL_ADD, j->timer_watch.fd, &ev) < 0) ++ return -errno; ++ ++ return 0; ++} ++ + static const char* const job_state_table[_JOB_STATE_MAX] = { + [JOB_WAITING] = "waiting", + [JOB_RUNNING] = "running" +diff --git a/src/job.h b/src/job.h +index 2025b5b..eea8242 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -141,15 +141,21 @@ struct Job { + bool in_dbus_queue:1; + bool sent_dbus_new_signal:1; + bool ignore_order:1; ++ bool forgot_bus_clients:1; + }; + + JobBusClient* job_bus_client_new(DBusConnection *connection, const char *name); + + Job* job_new(Unit *unit, JobType type); ++Job* job_new_raw(Unit *unit); + void job_free(Job *job); + Job* job_install(Job *j); ++void job_install_deserialized(Job *j); + void job_uninstall(Job *j); + void job_dump(Job *j, FILE*f, const char *prefix); ++int job_serialize(Job *j, FILE *f, FDSet *fds); ++int job_deserialize(Job *j, FILE *f, FDSet *fds); ++int job_coldplug(Job *j); + + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts); + void job_dependency_free(JobDependency *l); +diff --git a/src/unit.c b/src/unit.c +index 1949995..c203a31 100644 +--- a/src/unit.c ++++ b/src/unit.c +@@ -2288,8 +2288,10 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds) { + if ((r = UNIT_VTABLE(u)->serialize(u, f, fds)) < 0) + return r; + +- if (u->job) +- unit_serialize_item(u, f, "job", job_type_to_string(u->job->type)); ++ if (u->job) { ++ fprintf(f, "job\n"); ++ job_serialize(u->job, f, fds); ++ } + + dual_timestamp_serialize(f, "inactive-exit-timestamp", &u->inactive_exit_timestamp); + dual_timestamp_serialize(f, "active-enter-timestamp", &u->active_enter_timestamp); +@@ -2368,13 +2370,32 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { + v = l+k; + + if (streq(l, "job")) { +- JobType type; ++ if (v[0] == '\0') { ++ /* new-style serialized job */ ++ Job *j = job_new_raw(u); ++ if (!j) ++ return -ENOMEM; + +- if ((type = job_type_from_string(v)) < 0) +- log_debug("Failed to parse job type value %s", v); +- else +- u->deserialized_job = type; ++ r = job_deserialize(j, f, fds); ++ if (r < 0) { ++ job_free(j); ++ return r; ++ } + ++ job_install_deserialized(j); ++ r = hashmap_put(u->manager->jobs, UINT32_TO_PTR(j->id), j); ++ if (r < 0) { ++ job_free(j); ++ return r; ++ } ++ } else { ++ /* legacy */ ++ JobType type = job_type_from_string(v); ++ if (type < 0) ++ log_debug("Failed to parse job type value %s", v); ++ else ++ u->deserialized_job = type; ++ } + continue; + } else if (streq(l, "inactive-exit-timestamp")) { + dual_timestamp_deserialize(v, &u->inactive_exit_timestamp); +@@ -2450,8 +2471,14 @@ int unit_coldplug(Unit *u) { + if ((r = UNIT_VTABLE(u)->coldplug(u)) < 0) + return r; + +- if (u->deserialized_job >= 0) { +- if ((r = manager_add_job(u->manager, u->deserialized_job, u, JOB_IGNORE_REQUIREMENTS, false, NULL, NULL)) < 0) ++ if (u->job) { ++ r = job_coldplug(u->job); ++ if (r < 0) ++ return r; ++ } else if (u->deserialized_job >= 0) { ++ /* legacy */ ++ r = manager_add_job(u->manager, u->deserialized_job, u, JOB_IGNORE_REQUIREMENTS, false, NULL, NULL); ++ if (r < 0) + return r; + + u->deserialized_job = _JOB_TYPE_INVALID; +diff --git a/src/unit.h b/src/unit.h +index 756f465..5fceabb 100644 +--- a/src/unit.h ++++ b/src/unit.h +@@ -200,7 +200,9 @@ struct Unit { + unsigned gc_marker; + + /* When deserializing, temporarily store the job type for this +- * unit here, if there was a job scheduled */ ++ * unit here, if there was a job scheduled. ++ * Only for deserializing from a legacy version. New style uses full ++ * serialized jobs. */ + int deserialized_job; /* This is actually of type JobType */ + + /* Error code when we didn't manage to load the unit (negative) */ +-- +1.7.7 + + +From f2e0e5d5c80c8a489faa584539d7010b056dee7b Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Tue, 24 Apr 2012 11:21:03 +0200 +Subject: [PATCH 28/30] transaction: cancel jobs non-recursively on isolate + +Recursive cancellation of jobs would trigger OnFailure actions of +dependent jobs. This is not desirable when isolating. + +Fixes https://bugzilla.redhat.com/show_bug.cgi?id=798328 +(cherry picked from commit 5273510e9f228a300ec6207d4502f1c6253aed5e) +--- + src/dbus-job.c | 2 +- + src/job.c | 41 ++++++++++++++++------------------------- + src/job.h | 2 +- + src/manager.c | 3 ++- + src/transaction.c | 10 ++++------ + src/unit.c | 12 ++++++------ + 6 files changed, 30 insertions(+), 40 deletions(-) + +diff --git a/src/dbus-job.c b/src/dbus-job.c +index 63e5846..8474138 100644 +--- a/src/dbus-job.c ++++ b/src/dbus-job.c +@@ -100,7 +100,7 @@ static DBusHandlerResult bus_job_message_dispatch(Job *j, DBusConnection *connec + if (!(reply = dbus_message_new_method_return(message))) + goto oom; + +- job_finish_and_invalidate(j, JOB_CANCELED); ++ job_finish_and_invalidate(j, JOB_CANCELED, true); + + } else { + const BusBoundProperties bps[] = { +diff --git a/src/job.c b/src/job.c +index 69df024..e8e0264 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -164,7 +164,7 @@ Job* job_install(Job *j) { + + if (uj) { + if (job_type_is_conflicting(uj->type, j->type)) +- job_finish_and_invalidate(uj, JOB_CANCELED); ++ job_finish_and_invalidate(uj, JOB_CANCELED, true); + else { + /* not conflicting, i.e. mergeable */ + +@@ -496,13 +496,13 @@ int job_run_and_invalidate(Job *j) { + + if ((j = manager_get_job(m, id))) { + if (r == -EALREADY) +- r = job_finish_and_invalidate(j, JOB_DONE); ++ r = job_finish_and_invalidate(j, JOB_DONE, true); + else if (r == -ENOEXEC) +- r = job_finish_and_invalidate(j, JOB_SKIPPED); ++ r = job_finish_and_invalidate(j, JOB_SKIPPED, true); + else if (r == -EAGAIN) + j->state = JOB_WAITING; + else if (r < 0) +- r = job_finish_and_invalidate(j, JOB_FAILED); ++ r = job_finish_and_invalidate(j, JOB_FAILED, true); + } + + return r; +@@ -555,12 +555,11 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) { + } + } + +-int job_finish_and_invalidate(Job *j, JobResult result) { ++int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) { + Unit *u; + Unit *other; + JobType t; + Iterator i; +- bool recursed = false; + + assert(j); + assert(j->installed); +@@ -594,7 +593,7 @@ int job_finish_and_invalidate(Job *j, JobResult result) { + job_print_status_message(u, t, result); + + /* Fail depending jobs on failure */ +- if (result != JOB_DONE) { ++ if (result != JOB_DONE && recursive) { + + if (t == JOB_START || + t == JOB_VERIFY_ACTIVE || +@@ -604,29 +603,23 @@ int job_finish_and_invalidate(Job *j, JobResult result) { + if (other->job && + (other->job->type == JOB_START || + other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) { +- job_finish_and_invalidate(other->job, JOB_DEPENDENCY); +- recursed = true; +- } ++ other->job->type == JOB_RELOAD_OR_START)) ++ job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + + SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i) + if (other->job && + (other->job->type == JOB_START || + other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) { +- job_finish_and_invalidate(other->job, JOB_DEPENDENCY); +- recursed = true; +- } ++ other->job->type == JOB_RELOAD_OR_START)) ++ job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + + SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i) + if (other->job && + !other->job->override && + (other->job->type == JOB_START || + other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) { +- job_finish_and_invalidate(other->job, JOB_DEPENDENCY); +- recursed = true; +- } ++ other->job->type == JOB_RELOAD_OR_START)) ++ job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + + } else if (t == JOB_STOP) { + +@@ -634,10 +627,8 @@ int job_finish_and_invalidate(Job *j, JobResult result) { + if (other->job && + (other->job->type == JOB_START || + other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) { +- job_finish_and_invalidate(other->job, JOB_DEPENDENCY); +- recursed = true; +- } ++ other->job->type == JOB_RELOAD_OR_START)) ++ job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + } + } + +@@ -665,7 +656,7 @@ finish: + + manager_check_finished(u->manager); + +- return recursed; ++ return 0; + } + + int job_start_timer(Job *j) { +@@ -757,7 +748,7 @@ void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w) { + assert(w == &j->timer_watch); + + log_warning("Job %s/%s timed out.", j->unit->id, job_type_to_string(j->type)); +- job_finish_and_invalidate(j, JOB_TIMEOUT); ++ job_finish_and_invalidate(j, JOB_TIMEOUT, true); + } + + int job_serialize(Job *j, FILE *f, FDSet *fds) { +diff --git a/src/job.h b/src/job.h +index eea8242..5b326ef 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -196,7 +196,7 @@ int job_start_timer(Job *j); + void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w); + + int job_run_and_invalidate(Job *j); +-int job_finish_and_invalidate(Job *j, JobResult result); ++int job_finish_and_invalidate(Job *j, JobResult result, bool recursive); + + char *job_dbus_path(Job *j); + +diff --git a/src/manager.c b/src/manager.c +index 86ec858..7bff456 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -846,7 +846,8 @@ void manager_clear_jobs(Manager *m) { + assert(m); + + while ((j = hashmap_first(m->jobs))) +- job_finish_and_invalidate(j, JOB_CANCELED); ++ /* No need to recurse. We're cancelling all jobs. */ ++ job_finish_and_invalidate(j, JOB_CANCELED, false); + } + + unsigned manager_dispatch_run_queue(Manager *m) { +diff --git a/src/transaction.c b/src/transaction.c +index b0a26f2..f687539 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -525,18 +525,16 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { + + /* When isolating first kill all installed jobs which + * aren't part of the new transaction */ +- rescan: + HASHMAP_FOREACH(j, m->jobs, i) { + assert(j->installed); + + if (hashmap_get(tr->jobs, j->unit)) + continue; + +- /* 'j' itself is safe to remove, but if other jobs +- are invalidated recursively, our iterator may become +- invalid and we need to start over. */ +- if (job_finish_and_invalidate(j, JOB_CANCELED) > 0) +- goto rescan; ++ /* Not invalidating recursively. Avoids triggering ++ * OnFailure= actions of dependent jobs. Also avoids ++ * invalidating our iterator. */ ++ job_finish_and_invalidate(j, JOB_CANCELED, false); + } + } + +diff --git a/src/unit.c b/src/unit.c +index c203a31..ff8331c 100644 +--- a/src/unit.c ++++ b/src/unit.c +@@ -1226,12 +1226,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); ++ job_finish_and_invalidate(u->job, JOB_DONE, true); + 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); ++ job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true); + } + + break; +@@ -1241,12 +1241,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); ++ job_finish_and_invalidate(u->job, reload_success ? JOB_DONE : JOB_FAILED, true); + 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); ++ job_finish_and_invalidate(u->job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE, true); + } + } + +@@ -1257,10 +1257,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); ++ job_finish_and_invalidate(u->job, JOB_DONE, true); + else if (u->job->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) { + unexpected = true; +- job_finish_and_invalidate(u->job, JOB_FAILED); ++ job_finish_and_invalidate(u->job, JOB_FAILED, true); + } + + break; +-- +1.7.7 + + +From 38a2f4227415ebf3519369d27a23750948f6dddf Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Wed, 25 Apr 2012 11:58:27 +0200 +Subject: [PATCH 29/30] core: add NOP jobs, job type collapsing + +Two of our current job types are special: +JOB_TRY_RESTART, JOB_RELOAD_OR_START. + +They differ from other job types by being sensitive to the unit active state. +They perform some action when the unit is active and some other action +otherwise. This raises a question: when exactly should the unit state be +checked to make the decision? + +Currently the unit state is checked when the job becomes runnable. It's more +sensible to check the state immediately when the job is added by the user. +When the user types "systemctl try-restart foo.service", he really intends +to restart the service if it's running right now. If it isn't running right +now, the restart is pointless. + +Consider the example (from Bugzilla[1]): + +sleep.service takes some time to start. +hello.service has After=sleep.service. +Both services get started. Two jobs will appear: + hello.service/start waiting + sleep.service/start running +Then someone runs "systemctl try-restart hello.service". + +Currently the try-restart operation will block and wait for +sleep.service/start to complete. + +The correct result is to complete the try-restart operation immediately +with success, because hello.service is not running. The two original +jobs must not be disturbed by this. + +To fix this we introduce two new concepts: +- a new job type: JOB_NOP + A JOB_NOP job does not do anything to the unit. It does not pull in any + dependencies. It is always immediately runnable. When installed to a unit, + it sits in a special slot (u->nop_job) where it never conflicts with + the installed job (u->job) of a different type. It never merges with jobs + of other types, but it can merge into an already installed JOB_NOP job. + +- "collapsing" of job types + When a job of one of the two special types is added, the state of the unit + is checked immediately and the job type changes: + JOB_TRY_RESTART -> JOB_RESTART or JOB_NOP + JOB_RELOAD_OR_START -> JOB_RELOAD or JOB_START + Should a job type JOB_RELOAD_OR_START appear later during job merging, it + collapses immediately afterwards. + Collapsing actually makes some things simpler, because there are now fewer + job types that are allowed in the transaction. + +[1] Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=753586 +(cherry picked from commit e0209d83e7b30153f43b1a633c955f66eb2c2e4a) + +Conflicts: + + Makefile.am + src/job.c +--- + src/job.c | 163 ++++++++++++++++++++++++++++++++------------------- + src/job.h | 44 ++++++++++---- + src/manager.c | 2 + + src/test-job-type.c | 85 +++++++++++++++++---------- + src/transaction.c | 18 +++--- + src/unit.c | 29 +++++++++- + src/unit.h | 6 +- + 7 files changed, 230 insertions(+), 117 deletions(-) + +diff --git a/src/job.c b/src/job.c +index e8e0264..2438f39 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -60,6 +60,7 @@ Job* job_new_raw(Unit *unit) { + + j->manager = unit->manager; + j->unit = unit; ++ j->type = _JOB_TYPE_INVALID; + j->timer_watch.type = WATCH_INVALID; + + return j; +@@ -115,15 +116,21 @@ void job_free(Job *j) { + } + + void job_uninstall(Job *j) { ++ Job **pj; ++ + assert(j->installed); +- assert(j->unit->job == j); ++ ++ pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; ++ assert(*pj == j); ++ + /* Detach from next 'bigger' objects */ + + /* daemon-reload should be transparent to job observers */ + if (j->manager->n_reloading <= 0) + bus_job_send_removed_signal(j); + +- j->unit->job = NULL; ++ *pj = NULL; ++ + unit_add_to_gc_queue(j->unit); + + hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id)); +@@ -144,31 +151,38 @@ static bool job_type_allows_late_merge(JobType t) { + * patched into JOB_START after stopping the unit. So if we see a + * JOB_RESTART running, it means the unit hasn't stopped yet and at + * this time the merge is still allowed. */ +- return !(t == JOB_RELOAD || t == JOB_RELOAD_OR_START); ++ return t != JOB_RELOAD; + } + + static void job_merge_into_installed(Job *j, Job *other) { + assert(j->installed); + assert(j->unit == other->unit); + +- j->type = job_type_lookup_merge(j->type, other->type); +- assert(j->type >= 0); ++ if (j->type != JOB_NOP) ++ job_type_merge_and_collapse(&j->type, other->type, j->unit); ++ else ++ assert(other->type == JOB_NOP); + + j->override = j->override || other->override; + } + + Job* job_install(Job *j) { +- Job *uj = j->unit->job; ++ Job **pj; ++ Job *uj; + + assert(!j->installed); ++ assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); ++ ++ pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; ++ uj = *pj; + + if (uj) { +- if (job_type_is_conflicting(uj->type, j->type)) ++ if (j->type != JOB_NOP && job_type_is_conflicting(uj->type, j->type)) + job_finish_and_invalidate(uj, JOB_CANCELED, true); + else { + /* not conflicting, i.e. mergeable */ + +- if (uj->state == JOB_WAITING || ++ if (j->type == JOB_NOP || uj->state == JOB_WAITING || + (job_type_allows_late_merge(j->type) && job_type_is_superset(uj->type, j->type))) { + job_merge_into_installed(uj, j); + log_debug("Merged into installed job %s/%s as %u", +@@ -189,23 +203,33 @@ Job* job_install(Job *j) { + } + + /* Install the job */ +- j->unit->job = j; ++ *pj = j; + j->installed = true; + j->manager->n_installed_jobs ++; + log_debug("Installed new job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); + return j; + } + +-void job_install_deserialized(Job *j) { ++int job_install_deserialized(Job *j) { ++ Job **pj; ++ + assert(!j->installed); + +- if (j->unit->job) { ++ if (j->type < 0 || j->type >= _JOB_TYPE_MAX_IN_TRANSACTION) { ++ log_debug("Invalid job type %s in deserialization.", strna(job_type_to_string(j->type))); ++ return -EINVAL; ++ } ++ ++ pj = (j->type == JOB_NOP) ? &j->unit->nop_job : &j->unit->job; ++ ++ if (*pj) { + log_debug("Unit %s already has a job installed. Not installing deserialized job.", j->unit->id); +- return; ++ return -EEXIST; + } +- j->unit->job = j; ++ *pj = j; + j->installed = true; + log_debug("Reinstalled deserialized job %s/%s as %u", j->unit->id, job_type_to_string(j->type), (unsigned) j->id); ++ return 0; + } + + JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts) { +@@ -268,6 +292,10 @@ void job_dump(Job *j, FILE*f, const char *prefix) { + * its lower triangle to avoid duplication. We don't store the main diagonal, + * because A merged with A is simply A. + * ++ * If the resulting type is collapsed immediately afterwards (to get rid of ++ * the JOB_RELOAD_OR_START, which lies outside the lookup function's domain), ++ * the following properties hold: ++ * + * Merging is associative! A merged with B merged with C is the same as + * A merged with C merged with B. + * +@@ -278,21 +306,19 @@ void job_dump(Job *j, FILE*f, const char *prefix) { + * be merged with C either. + */ + static const JobType job_merging_table[] = { +-/* What \ With * JOB_START JOB_VERIFY_ACTIVE JOB_STOP JOB_RELOAD JOB_RELOAD_OR_START JOB_RESTART JOB_TRY_RESTART */ +-/************************************************************************************************************************************/ ++/* What \ With * JOB_START JOB_VERIFY_ACTIVE JOB_STOP JOB_RELOAD */ ++/*********************************************************************************/ + /*JOB_START */ + /*JOB_VERIFY_ACTIVE */ JOB_START, + /*JOB_STOP */ -1, -1, + /*JOB_RELOAD */ JOB_RELOAD_OR_START, JOB_RELOAD, -1, +-/*JOB_RELOAD_OR_START*/ JOB_RELOAD_OR_START, JOB_RELOAD_OR_START, -1, JOB_RELOAD_OR_START, +-/*JOB_RESTART */ JOB_RESTART, JOB_RESTART, -1, JOB_RESTART, JOB_RESTART, +-/*JOB_TRY_RESTART */ JOB_RESTART, JOB_TRY_RESTART, -1, JOB_TRY_RESTART, JOB_RESTART, JOB_RESTART, ++/*JOB_RESTART */ JOB_RESTART, JOB_RESTART, -1, JOB_RESTART, + }; + + JobType job_type_lookup_merge(JobType a, JobType b) { +- assert_cc(ELEMENTSOF(job_merging_table) == _JOB_TYPE_MAX * (_JOB_TYPE_MAX - 1) / 2); +- assert(a >= 0 && a < _JOB_TYPE_MAX); +- assert(b >= 0 && b < _JOB_TYPE_MAX); ++ assert_cc(ELEMENTSOF(job_merging_table) == _JOB_TYPE_MAX_MERGING * (_JOB_TYPE_MAX_MERGING - 1) / 2); ++ assert(a >= 0 && a < _JOB_TYPE_MAX_MERGING); ++ assert(b >= 0 && b < _JOB_TYPE_MAX_MERGING); + + if (a == b) + return a; +@@ -328,24 +354,50 @@ bool job_type_is_redundant(JobType a, UnitActiveState b) { + return + b == UNIT_RELOADING; + +- case JOB_RELOAD_OR_START: +- return +- b == UNIT_ACTIVATING || +- b == UNIT_RELOADING; +- + case JOB_RESTART: + return + b == UNIT_ACTIVATING; + ++ default: ++ assert_not_reached("Invalid job type"); ++ } ++} ++ ++void job_type_collapse(JobType *t, Unit *u) { ++ UnitActiveState s; ++ ++ switch (*t) { ++ + case JOB_TRY_RESTART: +- return +- b == UNIT_ACTIVATING; ++ s = unit_active_state(u); ++ if (UNIT_IS_INACTIVE_OR_DEACTIVATING(s)) ++ *t = JOB_NOP; ++ else ++ *t = JOB_RESTART; ++ break; ++ ++ case JOB_RELOAD_OR_START: ++ s = unit_active_state(u); ++ if (UNIT_IS_INACTIVE_OR_DEACTIVATING(s)) ++ *t = JOB_START; ++ else ++ *t = JOB_RELOAD; ++ break; + + default: +- assert_not_reached("Invalid job type"); ++ ; + } + } + ++int job_type_merge_and_collapse(JobType *a, JobType b, Unit *u) { ++ JobType t = job_type_lookup_merge(*a, b); ++ if (t < 0) ++ return -EEXIST; ++ *a = t; ++ job_type_collapse(a, u); ++ return 0; ++} ++ + bool job_is_runnable(Job *j) { + Iterator i; + Unit *other; +@@ -362,10 +414,12 @@ bool job_is_runnable(Job *j) { + if (j->ignore_order) + return true; + ++ if (j->type == JOB_NOP) ++ return true; ++ + if (j->type == JOB_START || + j->type == JOB_VERIFY_ACTIVE || +- j->type == JOB_RELOAD || +- j->type == JOB_RELOAD_OR_START) { ++ j->type == JOB_RELOAD) { + + /* Immediate result is that the job is or might be + * started. In this case lets wait for the +@@ -383,8 +437,7 @@ bool job_is_runnable(Job *j) { + SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) + if (other->job && + (other->job->type == JOB_STOP || +- other->job->type == JOB_RESTART || +- other->job->type == JOB_TRY_RESTART)) ++ other->job->type == JOB_RESTART)) + return false; + + /* This means that for a service a and a service b where b +@@ -416,6 +469,7 @@ int job_run_and_invalidate(Job *j) { + + assert(j); + assert(j->installed); ++ assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); + + if (j->in_run_queue) { + LIST_REMOVE(Job, run_queue, j->manager->run_queue, j); +@@ -441,15 +495,6 @@ int job_run_and_invalidate(Job *j) { + + switch (j->type) { + +- case JOB_RELOAD_OR_START: +- if (unit_active_state(j->unit) == UNIT_ACTIVE) { +- j->type = JOB_RELOAD; +- r = unit_reload(j->unit); +- break; +- } +- j->type = JOB_START; +- /* fall through */ +- + case JOB_START: + r = unit_start(j->unit); + +@@ -469,14 +514,6 @@ int job_run_and_invalidate(Job *j) { + break; + } + +- case JOB_TRY_RESTART: +- if (UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(j->unit))) { +- r = -ENOEXEC; +- break; +- } +- j->type = JOB_RESTART; +- /* fall through */ +- + case JOB_STOP: + case JOB_RESTART: + r = unit_stop(j->unit); +@@ -490,11 +527,16 @@ int job_run_and_invalidate(Job *j) { + r = unit_reload(j->unit); + break; + ++ case JOB_NOP: ++ r = -EALREADY; ++ break; ++ + default: + assert_not_reached("Unknown job type"); + } + +- if ((j = manager_get_job(m, id))) { ++ j = manager_get_job(m, id); ++ if (j) { + if (r == -EALREADY) + r = job_finish_and_invalidate(j, JOB_DONE, true); + else if (r == -ENOEXEC) +@@ -563,6 +605,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) { + + assert(j); + assert(j->installed); ++ assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); + + job_add_to_dbus_queue(j); + +@@ -596,29 +639,25 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) { + if (result != JOB_DONE && recursive) { + + if (t == JOB_START || +- t == JOB_VERIFY_ACTIVE || +- t == JOB_RELOAD_OR_START) { ++ t == JOB_VERIFY_ACTIVE) { + + SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY], i) + if (other->job && + (other->job->type == JOB_START || +- other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) ++ other->job->type == JOB_VERIFY_ACTIVE)) + job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + + SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i) + if (other->job && + (other->job->type == JOB_START || +- other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) ++ other->job->type == JOB_VERIFY_ACTIVE)) + job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + + SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i) + if (other->job && + !other->job->override && + (other->job->type == JOB_START || +- other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) ++ other->job->type == JOB_VERIFY_ACTIVE)) + job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + + } else if (t == JOB_STOP) { +@@ -626,8 +665,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) { + SET_FOREACH(other, u->dependencies[UNIT_CONFLICTED_BY], i) + if (other->job && + (other->job->type == JOB_START || +- other->job->type == JOB_VERIFY_ACTIVE || +- other->job->type == JOB_RELOAD_OR_START)) ++ other->job->type == JOB_VERIFY_ACTIVE)) + job_finish_and_invalidate(other->job, JOB_DEPENDENCY, true); + } + } +@@ -807,6 +845,8 @@ int job_deserialize(Job *j, FILE *f, FDSet *fds) { + JobType t = job_type_from_string(v); + if (t < 0) + log_debug("Failed to parse job type %s", v); ++ else if (t >= _JOB_TYPE_MAX_IN_TRANSACTION) ++ log_debug("Cannot deserialize job of type %s", v); + else + j->type = t; + } else if (streq(l, "job-state")) { +@@ -886,6 +926,7 @@ static const char* const job_type_table[_JOB_TYPE_MAX] = { + [JOB_RELOAD_OR_START] = "reload-or-start", + [JOB_RESTART] = "restart", + [JOB_TRY_RESTART] = "try-restart", ++ [JOB_NOP] = "nop", + }; + + DEFINE_STRING_TABLE_LOOKUP(job_type, JobType); +diff --git a/src/job.h b/src/job.h +index 5b326ef..4edf3b9 100644 +--- a/src/job.h ++++ b/src/job.h +@@ -46,14 +46,34 @@ enum JobType { + + JOB_STOP, + +- JOB_RELOAD, /* if running reload */ +- JOB_RELOAD_OR_START, /* if running reload, if not running start */ ++ JOB_RELOAD, /* if running, reload */ + + /* Note that restarts are first treated like JOB_STOP, but + * then instead of finishing are patched to become + * JOB_START. */ +- JOB_RESTART, /* if running stop, then start unconditionally */ +- JOB_TRY_RESTART, /* if running stop and then start */ ++ JOB_RESTART, /* If running, stop. Then start unconditionally. */ ++ ++ _JOB_TYPE_MAX_MERGING, ++ ++ /* JOB_NOP can enter into a transaction, but as it won't pull in ++ * any dependencies, it won't have to merge with anything. ++ * job_install() avoids the problem of merging JOB_NOP too (it's ++ * special-cased, only merges with other JOB_NOPs). */ ++ JOB_NOP = _JOB_TYPE_MAX_MERGING, /* do nothing */ ++ ++ _JOB_TYPE_MAX_IN_TRANSACTION, ++ ++ /* JOB_TRY_RESTART can never appear in a transaction, because ++ * it always collapses into JOB_RESTART or JOB_NOP before entering. ++ * Thus we never need to merge it with anything. */ ++ JOB_TRY_RESTART = _JOB_TYPE_MAX_IN_TRANSACTION, /* if running, stop and then start */ ++ ++ /* JOB_RELOAD_OR_START won't enter into a transaction and cannot result ++ * from transaction merging (there's no way for JOB_RELOAD and ++ * JOB_START to meet in one transaction). It can result from a merge ++ * during job installation, but then it will immediately collapse into ++ * one of the two simpler types. */ ++ JOB_RELOAD_OR_START, /* if running, reload, otherwise start */ + + _JOB_TYPE_MAX, + _JOB_TYPE_INVALID = -1 +@@ -150,7 +170,7 @@ Job* job_new(Unit *unit, JobType type); + Job* job_new_raw(Unit *unit); + void job_free(Job *job); + Job* job_install(Job *j); +-void job_install_deserialized(Job *j); ++int job_install_deserialized(Job *j); + void job_uninstall(Job *j); + void job_dump(Job *j, FILE*f, const char *prefix); + int job_serialize(Job *j, FILE *f, FDSet *fds); +@@ -164,14 +184,6 @@ int job_merge(Job *j, Job *other); + + JobType job_type_lookup_merge(JobType a, JobType b); + +-static inline int job_type_merge(JobType *a, JobType b) { +- JobType t = job_type_lookup_merge(*a, b); +- if (t < 0) +- return -EEXIST; +- *a = t; +- return 0; +-} +- + static inline bool job_type_is_mergeable(JobType a, JobType b) { + return job_type_lookup_merge(a, b) >= 0; + } +@@ -187,6 +199,12 @@ static inline bool job_type_is_superset(JobType a, JobType b) { + + bool job_type_is_redundant(JobType a, UnitActiveState b); + ++/* Collapses a state-dependent job type into a simpler type by observing ++ * the state of the unit which it is going to be applied to. */ ++void job_type_collapse(JobType *t, Unit *u); ++ ++int job_type_merge_and_collapse(JobType *a, JobType b, Unit *u); ++ + bool job_is_runnable(Job *j); + + void job_add_to_run_queue(Job *j); +diff --git a/src/manager.c b/src/manager.c +index 7bff456..0f87e9f 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -650,6 +650,8 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove + + log_debug("Trying to enqueue job %s/%s/%s", unit->id, job_type_to_string(type), job_mode_to_string(mode)); + ++ job_type_collapse(&type, unit); ++ + tr = transaction_new(); + if (!tr) + return -ENOMEM; +diff --git a/src/test-job-type.c b/src/test-job-type.c +index 9de21e1..c7a9b5a 100644 +--- a/src/test-job-type.c ++++ b/src/test-job-type.c +@@ -25,59 +25,80 @@ + #include + + #include "job.h" ++#include "unit.h" ++#include "service.h" + + int main(int argc, char*argv[]) { +- JobType a, b, c, d, e, f, g; ++ JobType a, b, c, ab, bc, ab_c, bc_a, a_bc; ++ const ServiceState test_states[] = { SERVICE_DEAD, SERVICE_RUNNING }; ++ unsigned i; ++ bool merged_ab; + +- for (a = 0; a < _JOB_TYPE_MAX; a++) +- for (b = 0; b < _JOB_TYPE_MAX; b++) { ++ /* fake a unit */ ++ static Service s = { ++ .meta.load_state = UNIT_LOADED, ++ .type = UNIT_SERVICE ++ }; ++ Unit *u = UNIT(&s); + +- if (!job_type_is_mergeable(a, b)) +- printf("Not mergeable: %s + %s\n", job_type_to_string(a), job_type_to_string(b)); ++ for (i = 0; i < ELEMENTSOF(test_states); i++) { ++ s.state = test_states[i]; ++ printf("\nWith collapsing for service state %s\n" ++ "=========================================\n", service_state_to_string(s.state)); ++ for (a = 0; a < _JOB_TYPE_MAX_MERGING; a++) { ++ for (b = 0; b < _JOB_TYPE_MAX_MERGING; b++) { + +- for (c = 0; c < _JOB_TYPE_MAX; c++) { ++ ab = a; ++ merged_ab = (job_type_merge_and_collapse(&ab, b, u) >= 0); + +- /* Verify transitivity of mergeability +- * of job types */ +- assert(!job_type_is_mergeable(a, b) || +- !job_type_is_mergeable(b, c) || +- job_type_is_mergeable(a, c)); ++ if (!job_type_is_mergeable(a, b)) { ++ assert(!merged_ab); ++ printf("Not mergeable: %s + %s\n", job_type_to_string(a), job_type_to_string(b)); ++ continue; ++ } ++ ++ assert(merged_ab); ++ printf("%s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(ab)); + +- d = a; +- if (job_type_merge(&d, b) >= 0) { ++ for (c = 0; c < _JOB_TYPE_MAX_MERGING; c++) { + +- printf("%s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(d)); ++ /* Verify transitivity of mergeability of job types */ ++ assert(!job_type_is_mergeable(a, b) || ++ !job_type_is_mergeable(b, c) || ++ job_type_is_mergeable(a, c)); + +- /* Verify that merged entries can be +- * merged with the same entries they +- * can be merged with separately */ +- assert(!job_type_is_mergeable(a, c) || job_type_is_mergeable(d, c)); +- assert(!job_type_is_mergeable(b, c) || job_type_is_mergeable(d, c)); ++ /* Verify that merged entries can be merged with the same entries ++ * they can be merged with separately */ ++ assert(!job_type_is_mergeable(a, c) || job_type_is_mergeable(ab, c)); ++ assert(!job_type_is_mergeable(b, c) || job_type_is_mergeable(ab, c)); + +- /* Verify that if a merged +- * with b is not mergeable with +- * c then either a or b is not +- * mergeable with c either. */ +- assert(job_type_is_mergeable(d, c) || !job_type_is_mergeable(a, c) || !job_type_is_mergeable(b, c)); ++ /* Verify that if a merged with b is not mergeable with c, then ++ * either a or b is not mergeable with c either. */ ++ assert(job_type_is_mergeable(ab, c) || !job_type_is_mergeable(a, c) || !job_type_is_mergeable(b, c)); + +- e = b; +- if (job_type_merge(&e, c) >= 0) { ++ bc = b; ++ if (job_type_merge_and_collapse(&bc, c, u) >= 0) { + + /* Verify associativity */ + +- f = d; +- assert(job_type_merge(&f, c) == 0); ++ ab_c = ab; ++ assert(job_type_merge_and_collapse(&ab_c, c, u) == 0); ++ ++ bc_a = bc; ++ assert(job_type_merge_and_collapse(&bc_a, a, u) == 0); + +- g = e; +- assert(job_type_merge(&g, a) == 0); ++ a_bc = a; ++ assert(job_type_merge_and_collapse(&a_bc, bc, u) == 0); + +- assert(f == g); ++ assert(ab_c == bc_a); ++ assert(ab_c == a_bc); + +- printf("%s + %s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(c), job_type_to_string(d)); ++ printf("%s + %s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(c), job_type_to_string(ab_c)); + } + } + } + } ++ } + + + return 0; +diff --git a/src/transaction.c b/src/transaction.c +index f687539..ceac5a6 100644 +--- a/src/transaction.c ++++ b/src/transaction.c +@@ -212,7 +212,7 @@ static int transaction_merge_jobs(Transaction *tr, DBusError *e) { + + t = j->type; + LIST_FOREACH(transaction, k, j->transaction_next) { +- if (job_type_merge(&t, k->type) >= 0) ++ if (job_type_merge_and_collapse(&t, k->type, j->unit) >= 0) + continue; + + /* OK, we could not merge all jobs for this +@@ -238,9 +238,9 @@ static int transaction_merge_jobs(Transaction *tr, DBusError *e) { + JobType t = j->type; + Job *k; + +- /* Merge all transactions */ ++ /* Merge all transaction jobs for j->unit */ + LIST_FOREACH(transaction, k, j->transaction_next) +- assert_se(job_type_merge(&t, k->type) == 0); ++ assert_se(job_type_merge_and_collapse(&t, k->type, j->unit) == 0); + + while ((k = j->transaction_next)) { + if (tr->anchor_job == k) { +@@ -774,6 +774,7 @@ int transaction_add_job_and_dependencies( + + assert(tr); + assert(type < _JOB_TYPE_MAX); ++ assert(type < _JOB_TYPE_MAX_IN_TRANSACTION); + assert(unit); + + /* log_debug("Pulling in %s/%s from %s/%s", */ +@@ -824,7 +825,8 @@ int transaction_add_job_and_dependencies( + assert(!tr->anchor_job); + tr->anchor_job = ret; + } +- if (is_new && !ignore_requirements) { ++ ++ if (is_new && !ignore_requirements && type != JOB_NOP) { + Set *following; + + /* If we are following some other unit, make sure we +@@ -844,7 +846,7 @@ int transaction_add_job_and_dependencies( + } + + /* Finally, recursively add in all dependencies. */ +- if (type == JOB_START || type == JOB_RELOAD_OR_START || type == JOB_RESTART) { ++ if (type == JOB_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) { +@@ -934,7 +936,7 @@ int transaction_add_job_and_dependencies( + + } + +- if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) { ++ if (type == JOB_STOP || type == JOB_RESTART) { + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRED_BY], i) { + r = transaction_add_job_and_dependencies(tr, type, dep, ret, true, override, false, false, ignore_order, e); +@@ -959,7 +961,7 @@ int transaction_add_job_and_dependencies( + } + } + +- if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START) { ++ if (type == JOB_RELOAD) { + + SET_FOREACH(dep, ret->unit->dependencies[UNIT_PROPAGATE_RELOAD_TO], i) { + r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, override, false, false, ignore_order, e); +@@ -972,7 +974,7 @@ int transaction_add_job_and_dependencies( + } + } + +- /* JOB_VERIFY_STARTED, JOB_RELOAD require no dependency handling */ ++ /* JOB_VERIFY_STARTED require no dependency handling */ + } + + return 0; +diff --git a/src/unit.c b/src/unit.c +index ff8331c..3625ed2 100644 +--- a/src/unit.c ++++ b/src/unit.c +@@ -249,6 +249,9 @@ bool unit_check_gc(Unit *u) { + if (u->job) + return true; + ++ if (u->nop_job) ++ return true; ++ + if (unit_active_state(u) != UNIT_INACTIVE) + return true; + +@@ -358,6 +361,12 @@ void unit_free(Unit *u) { + job_free(j); + } + ++ if (u->nop_job) { ++ Job *j = u->nop_job; ++ job_uninstall(j); ++ job_free(j); ++ } ++ + for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) + bidi_set_free(u, u->dependencies[d]); + +@@ -501,6 +510,9 @@ int unit_merge(Unit *u, Unit *other) { + if (other->job) + return -EEXIST; + ++ if (other->nop_job) ++ return -EEXIST; ++ + if (!UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) + return -EEXIST; + +@@ -729,6 +741,9 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { + if (u->job) + job_dump(u->job, f, prefix2); + ++ if (u->nop_job) ++ job_dump(u->nop_job, f, prefix2); ++ + free(p2); + } + +@@ -1507,6 +1522,7 @@ bool unit_job_is_applicable(Unit *u, JobType j) { + case JOB_VERIFY_ACTIVE: + case JOB_START: + case JOB_STOP: ++ case JOB_NOP: + return true; + + case JOB_RESTART: +@@ -2293,6 +2309,11 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds) { + job_serialize(u->job, f, fds); + } + ++ if (u->nop_job) { ++ fprintf(f, "job\n"); ++ job_serialize(u->nop_job, f, fds); ++ } ++ + dual_timestamp_serialize(f, "inactive-exit-timestamp", &u->inactive_exit_timestamp); + dual_timestamp_serialize(f, "active-enter-timestamp", &u->active_enter_timestamp); + dual_timestamp_serialize(f, "active-exit-timestamp", &u->active_exit_timestamp); +@@ -2382,12 +2403,18 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { + return r; + } + +- job_install_deserialized(j); + r = hashmap_put(u->manager->jobs, UINT32_TO_PTR(j->id), j); + if (r < 0) { + job_free(j); + return r; + } ++ ++ r = job_install_deserialized(j); ++ if (r < 0) { ++ hashmap_remove(u->manager->jobs, UINT32_TO_PTR(j->id)); ++ job_free(j); ++ return r; ++ } + } else { + /* legacy */ + JobType type = job_type_from_string(v); +diff --git a/src/unit.h b/src/unit.h +index 5fceabb..62b82bc 100644 +--- a/src/unit.h ++++ b/src/unit.h +@@ -158,10 +158,12 @@ struct Unit { + char *fragment_path; /* if loaded from a config file this is the primary path to it */ + usec_t fragment_mtime; + +- /* If there is something to do with this unit, then this is +- * the job for it */ ++ /* If there is something to do with this unit, then this is the installed job for it */ + Job *job; + ++ /* JOB_NOP jobs are special and can be installed without disturbing the real job. */ ++ Job *nop_job; ++ + usec_t job_timeout; + + /* References to this */ +-- +1.7.7 + + +From ad1ece751b204c21424b6bf1de265fec406ba2e4 Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Sat, 12 May 2012 21:06:27 +0200 +Subject: [PATCH 30/30] job: only jobs on the runqueue can be run + +--- + src/job.c | 7 +++---- + 1 files changed, 3 insertions(+), 4 deletions(-) + +diff --git a/src/job.c b/src/job.c +index 2438f39..d6e7cb1 100644 +--- a/src/job.c ++++ b/src/job.c +@@ -470,11 +470,10 @@ int job_run_and_invalidate(Job *j) { + assert(j); + assert(j->installed); + assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); ++ assert(j->in_run_queue); + +- if (j->in_run_queue) { +- LIST_REMOVE(Job, run_queue, j->manager->run_queue, j); +- j->in_run_queue = false; +- } ++ LIST_REMOVE(Job, run_queue, j->manager->run_queue, j); ++ j->in_run_queue = false; + + if (j->state != JOB_WAITING) + return 0; +-- +1.7.7 + diff --git a/journal-bugfixes.patch b/journal-bugfixes.patch new file mode 100644 index 00000000..428d165b --- /dev/null +++ b/journal-bugfixes.patch @@ -0,0 +1,186 @@ +From 911efc97f9bfe2ad4f4d021f5e76d05c8d5d81ac Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Thu, 12 Apr 2012 12:57:41 +0200 +Subject: [PATCH 1/4] journald: add missing flag to open() + +--- + src/journal/journald.c | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/src/journal/journald.c b/src/journal/journald.c +index baad3ab..c8b400a 100644 +--- a/src/journal/journald.c ++++ b/src/journal/journald.c +@@ -2461,7 +2461,7 @@ static int open_proc_kmsg(Server *s) { + return 0; + + +- s->proc_kmsg_fd = open("/proc/kmsg", O_CLOEXEC|O_NONBLOCK|O_NOCTTY); ++ s->proc_kmsg_fd = open("/proc/kmsg", O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); + if (s->proc_kmsg_fd < 0) { + log_warning("Failed to open /proc/kmsg, ignoring: %m"); + return 0; +-- +1.7.7 + + +From 94b8299358fd743137857bc0f28ab62bcf6eec92 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Fri, 13 Apr 2012 13:58:50 +0200 +Subject: [PATCH 2/4] fix a couple of things found with the llvm static + analyzer + +--- + src/journal/journal-file.c | 2 +- + src/journal/journald.c | 2 +- + src/logs-show.c | 2 +- + src/manager.c | 2 +- + 4 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c +index 474dd5c..5255c3b 100644 +--- a/src/journal/journal-file.c ++++ b/src/journal/journal-file.c +@@ -1974,7 +1974,7 @@ int journal_directory_vacuum(const char *directory, uint64_t max_use, uint64_t m + size_t q; + struct stat st; + char *p; +- unsigned long long seqnum, realtime; ++ unsigned long long seqnum = 0, realtime; + sd_id128_t seqnum_id; + bool have_seqnum; + +diff --git a/src/journal/journald.c b/src/journal/journald.c +index c8b400a..1118b7e 100644 +--- a/src/journal/journald.c ++++ b/src/journal/journald.c +@@ -1140,7 +1140,7 @@ static void process_native_message( + char *identifier = NULL, *message = NULL; + + assert(s); +- assert(buffer || n == 0); ++ assert(buffer || buffer_size == 0); + + p = buffer; + remaining = buffer_size; +diff --git a/src/logs-show.c b/src/logs-show.c +index f71c6b0..eb9a902 100644 +--- a/src/logs-show.c ++++ b/src/logs-show.c +@@ -541,7 +541,7 @@ int show_journal_by_unit( + bool follow) { + + char *m = NULL; +- sd_journal *j; ++ sd_journal *j = NULL; + int r; + int fd; + unsigned line = 0; +diff --git a/src/manager.c b/src/manager.c +index 74bd740..3e592b6 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -2979,7 +2979,7 @@ bool manager_unit_pending_inactive(Manager *m, const char *name) { + + void manager_check_finished(Manager *m) { + char userspace[FORMAT_TIMESPAN_MAX], initrd[FORMAT_TIMESPAN_MAX], kernel[FORMAT_TIMESPAN_MAX], sum[FORMAT_TIMESPAN_MAX]; +- usec_t kernel_usec = 0, initrd_usec = 0, userspace_usec = 0, total_usec = 0; ++ usec_t kernel_usec, initrd_usec, userspace_usec, total_usec; + + assert(m); + +-- +1.7.7 + + +From f83fa045b967478a80ca55477dfe6a5be5f0b4a8 Mon Sep 17 00:00:00 2001 +From: Sjoerd Simons +Date: Sat, 14 Apr 2012 14:11:08 +0200 +Subject: [PATCH 3/4] journal: crash when filesystem is low on space + +When space is getting too low on a file system rotating the journal file +will fail after the rotation, as opening the new logfile will fail. + +Recognize this when logging the error and don't try to dereference a +NULL JournalFile pointer. +--- + src/journal/journald.c | 16 +++++++++++++--- + 1 files changed, 13 insertions(+), 3 deletions(-) + +diff --git a/src/journal/journald.c b/src/journal/journald.c +index 1118b7e..9180656 100644 +--- a/src/journal/journald.c ++++ b/src/journal/journald.c +@@ -330,7 +330,10 @@ static void server_rotate(Server *s) { + if (s->runtime_journal) { + r = journal_file_rotate(&s->runtime_journal); + if (r < 0) +- log_error("Failed to rotate %s: %s", s->runtime_journal->path, strerror(-r)); ++ if (s->runtime_journal) ++ log_error("Failed to rotate %s: %s", s->runtime_journal->path, strerror(-r)); ++ else ++ log_error("Failed to create new runtime journal: %s", strerror(-r)); + else + server_fix_perms(s, s->runtime_journal, 0); + } +@@ -338,7 +341,11 @@ static void server_rotate(Server *s) { + if (s->system_journal) { + r = journal_file_rotate(&s->system_journal); + if (r < 0) +- log_error("Failed to rotate %s: %s", s->system_journal->path, strerror(-r)); ++ if (s->system_journal) ++ log_error("Failed to rotate %s: %s", s->system_journal->path, strerror(-r)); ++ else ++ log_error("Failed to create new system journal: %s", strerror(-r)); ++ + else + server_fix_perms(s, s->system_journal, 0); + } +@@ -346,7 +353,10 @@ static void server_rotate(Server *s) { + HASHMAP_FOREACH_KEY(f, k, s->user_journals, i) { + r = journal_file_rotate(&f); + if (r < 0) +- log_error("Failed to rotate %s: %s", f->path, strerror(-r)); ++ if (f->path) ++ log_error("Failed to rotate %s: %s", f->path, strerror(-r)); ++ else ++ log_error("Failed to create user journal: %s", strerror(-r)); + else { + hashmap_replace(s->user_journals, k, f); + server_fix_perms(s, s->system_journal, PTR_TO_UINT32(k)); +-- +1.7.7 + + +From d80e2f5c26aae25c0773042bcd1599d3c583bf6a Mon Sep 17 00:00:00 2001 +From: Michal Schmidt +Date: Tue, 12 Jun 2012 16:45:09 +0200 +Subject: [PATCH 4/4] journal-file: fix mmap leak + +https://bugzilla.redhat.com/show_bug.cgi?id=831132 +--- + src/journal/journal-file.c | 7 +++++-- + 1 files changed, 5 insertions(+), 2 deletions(-) + +diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c +index 5255c3b..e242fa2 100644 +--- a/src/journal/journal-file.c ++++ b/src/journal/journal-file.c +@@ -67,9 +67,12 @@ void journal_file_close(JournalFile *f) { + + assert(f); + +- if (f->header && f->writable) +- f->header->state = STATE_OFFLINE; ++ if (f->header) { ++ if (f->writable) ++ f->header->state = STATE_OFFLINE; + ++ munmap(f->header, PAGE_ALIGN(sizeof(Header))); ++ } + + for (t = 0; t < _WINDOW_MAX; t++) + if (f->windows[t].ptr) +-- +1.7.7 + diff --git a/systemd.changes b/systemd.changes index a6f9884b..4937dd83 100644 --- a/systemd.changes +++ b/systemd.changes @@ -1,3 +1,21 @@ +------------------------------------------------------------------- +Tue Jul 3 17:58:39 CEST 2012 - fcrozat@suse.com + +- Add fix-dir-noatime-tmpfiles.patch: do not modify directory + atime, which was preventing removing empty directories + (bnc#751253, rh#810257). +- Add improve-restart-behaviour.patch: prevent deadlock during + try-restart (bnc#743218). +- Add journal-bugfixes.patch: don't crash when rotating journal + (bnc#768953) and prevent memleak at rotation time too. +- Add ulimit-support.patch: add support for system wide ulimit + (bnc#744818). +- Add change-terminal.patch: use vt102 instead of vt100 as terminal + for non-vc tty. +- Package various .wants directories, which were no longer packaged + due to plymouth units being removed from systemd package. +- Fix buildrequires for manpages build. + ------------------------------------------------------------------- Mon Jul 2 15:44:28 UTC 2012 - fcrozat@suse.com diff --git a/systemd.spec b/systemd.spec index f670c1a5..85c002ec 100644 --- a/systemd.spec +++ b/systemd.spec @@ -35,7 +35,7 @@ BuildRequires: intltool BuildRequires: libacl-devel BuildRequires: libcap-devel BuildRequires: libtool -BuildRequires: libxslt1 +BuildRequires: libxslt-tools BuildRequires: pam-devel BuildRequires: pkg-config BuildRequires: tcpd-devel @@ -93,6 +93,11 @@ Patch42: fixppc.patch Patch43: logind-logout.patch Patch44: fix-getty-isolate.patch Patch45: fix-swap-priority.patch +Patch46: improve-restart-behaviour.patch +Patch47: fix-dir-noatime-tmpfiles.patch +Patch48: journal-bugfixes.patch +Patch49: ulimit-support.patch +Patch50: change-terminal.patch %description Systemd is a system and service manager, compatible with SysV and LSB @@ -148,6 +153,14 @@ Drop-in replacement of System V init tools. %patch43 -p1 %patch44 -p1 %patch45 -p1 +%patch46 -p1 +%patch47 -p1 +%patch48 -p1 +%patch49 -p1 +%patch50 -p1 + +#needed by patch49 +rm man/systemd.conf.5 %build autoreconf -fiv @@ -163,6 +176,7 @@ export V=1 --with-pamlibdir=/%{_lib}/security \ --enable-split-usr \ --disable-gtk \ + --enable-manpages \ --disable-plymouth \ CFLAGS="%{optflags}" make %{?_smp_mflags} @@ -178,7 +192,8 @@ chmod 644 %{buildroot}%{_bindir}/systemd-analyze mkdir -p %{buildroot}%{_sysconfdir}/rpm install -m644 %{S:4} %{buildroot}%{_sysconfdir}/rpm find %{buildroot} -type f -name '*.la' -exec rm -f {} ';' -mkdir -p %{buildroot}/{sbin,var/lib/systemd/sysv-convert,var/lib/systemd/migrated} %{buildroot}/lib/systemd/{system.preset,user.preset} +mkdir -p %{buildroot}/{sbin,var/lib/systemd/sysv-convert,var/lib/systemd/migrated} %{buildroot}/lib/systemd/{system.preset,user.preset,system/halt.target.wants,system/kexec.target.wants,system/poweroff.target.wants,system/reboot.target.wants} + install -m755 %{S:3} -D %{buildroot}%{_sbindir}/systemd-sysv-convert # do not install, code has been fixed, might be useful in the future #install -m755 %{S:5} %{buildroot}/lib/systemd/system-generators diff --git a/ulimit-support.patch b/ulimit-support.patch new file mode 100644 index 00000000..ae6d355c --- /dev/null +++ b/ulimit-support.patch @@ -0,0 +1,253 @@ +From 03854532d39613723dc8b85c424737ecf2e46f74 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 16 Apr 2012 18:54:45 +0200 +Subject: [PATCH 1/3] util: introduce memdup() + +--- + src/util.h | 2 ++ + 1 files changed, 2 insertions(+), 0 deletions(-) + +diff --git a/src/util.h b/src/util.h +index b1af6db..06c9933 100644 +--- a/src/util.h ++++ b/src/util.h +@@ -99,6 +99,8 @@ bool streq_ptr(const char *a, const char *b); + + #define new0(t, n) ((t*) calloc((n), sizeof(t))) + ++#define newdup(t, p, n) ((t*) memdup(p, sizeof(t)*(n)) ++ + #define malloc0(n) (calloc((n), 1)) + + static inline const char* yes_no(bool b) { +-- +1.7.7 + + +From f60b5d436f502152415b08758737f200113ce4bc Mon Sep 17 00:00:00 2001 +From: Frederic Crozat +Date: Mon, 21 May 2012 16:53:18 +0200 +Subject: [PATCH 2/3] util: fix typo in newdup + +Conflicts: + + src/util.h +--- + src/util.h | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/src/util.h b/src/util.h +index 06c9933..41b4c9f 100644 +--- a/src/util.h ++++ b/src/util.h +@@ -99,7 +99,7 @@ bool streq_ptr(const char *a, const char *b); + + #define new0(t, n) ((t*) calloc((n), sizeof(t))) + +-#define newdup(t, p, n) ((t*) memdup(p, sizeof(t)*(n)) ++#define newdup(t, p, n) ((t*) memdup(p, sizeof(t)*(n))) + + #define malloc0(n) (calloc((n), 1)) + +-- +1.7.7 + + +From 8e7fa2b3e68b691c522cf2b60ed920452c146c2e Mon Sep 17 00:00:00 2001 +From: Frederic Crozat +Date: Wed, 27 Jun 2012 14:12:44 +0200 +Subject: [PATCH 3/3] main: allow system wide limits for services + +--- + man/systemd.conf.xml | 27 +++++++++++++++++++++++++++ + src/main.c | 22 ++++++++++++++++++++++ + src/manager.c | 22 ++++++++++++++++++++++ + src/manager.h | 3 +++ + src/service.c | 4 ++++ + 5 files changed, 78 insertions(+), 0 deletions(-) + +diff --git a/man/systemd.conf.xml b/man/systemd.conf.xml +index ba144da..ee461e3 100644 +--- a/man/systemd.conf.xml ++++ b/man/systemd.conf.xml +@@ -149,6 +149,33 @@ + controllers in separate + hierarchies. + ++ ++ ++ DefaultLimitCPU= ++ DefaultLimitFSIZE= ++ DefaultLimitDATA= ++ DefaultLimitSTACK= ++ DefaultLimitCORE= ++ DefaultLimitRSS= ++ DefaultLimitNOFILE= ++ DefaultLimitAS= ++ DefaultLimitNPROC= ++ DefaultLimitMEMLOCK= ++ DefaultLimitLOCKS= ++ DefaultLimitSIGPENDING= ++ DefaultLimitMSGQUEUE= ++ DefaultLimitNICE= ++ DefaultLimitRTPRIO= ++ DefaultLimitRTTIME= ++ These settings control ++ various default resource limits for units. See ++ setrlimit2 ++ for details. Use the string ++ infinity to ++ configure no limit on a specific ++ resource. They can be overriden in units files ++ using corresponding LimitXXXX parameter. ++ + + + +diff --git a/src/main.c b/src/main.c +index ed317b4..3f5f3d7 100644 +--- a/src/main.c ++++ b/src/main.c +@@ -79,6 +79,7 @@ static char **arg_default_controllers = NULL; + static char ***arg_join_controllers = NULL; + static ExecOutput arg_default_std_output = EXEC_OUTPUT_JOURNAL; + static ExecOutput arg_default_std_error = EXEC_OUTPUT_INHERIT; ++static struct rlimit *arg_default_rlimit[RLIMIT_NLIMITS] = {}; + + static FILE* serialization = NULL; + +@@ -659,6 +660,22 @@ static int parse_config_file(void) { + { "Manager", "DefaultStandardOutput", config_parse_output, 0, &arg_default_std_output }, + { "Manager", "DefaultStandardError", config_parse_output, 0, &arg_default_std_error }, + { "Manager", "JoinControllers", config_parse_join_controllers, 0, &arg_join_controllers }, ++ { "Manager", "DefaultLimitCPU", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_CPU]}, ++ { "Manager", "DefaultLimitFSIZE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_FSIZE]}, ++ { "Manager", "DefaultLimitDATA", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_DATA]}, ++ { "Manager", "DefaultLimitSTACK", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_STACK]}, ++ { "Manager", "DefaultLimitCORE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_CORE]}, ++ { "Manager", "DefaultLimitRSS", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_RSS]}, ++ { "Manager", "DefaultLimitNOFILE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_NOFILE]}, ++ { "Manager", "DefaultLimitAS", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_AS]}, ++ { "Manager", "DefaultLimitNPROC", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_NPROC]}, ++ { "Manager", "DefaultLimitMEMLOCK", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_MEMLOCK]}, ++ { "Manager", "DefaultLimitLOCKS", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_LOCKS]}, ++ { "Manager", "DefaultLimitSIGPENDING",config_parse_limit, 0, &arg_default_rlimit[RLIMIT_SIGPENDING]}, ++ { "Manager", "DefaultLimitMSGQUEUE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_MSGQUEUE]}, ++ { "Manager", "DefaultLimitNICE", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_NICE]}, ++ { "Manager", "DefaultLimitRTPRIO", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_RTPRIO]}, ++ { "Manager", "DefaultLimitRTTIME", config_parse_limit, 0, &arg_default_rlimit[RLIMIT_RTTIME]}, + { NULL, NULL, NULL, 0, NULL } + }; + +@@ -1401,6 +1418,8 @@ int main(int argc, char *argv[]) { + m->default_std_output = arg_default_std_output; + m->default_std_error = arg_default_std_error; + ++ manager_set_default_rlimits(m, arg_default_rlimit); ++ + if (dual_timestamp_is_set(&initrd_timestamp)) + m->initrd_timestamp = initrd_timestamp; + +@@ -1539,6 +1558,9 @@ finish: + if (m) + manager_free(m); + ++ for (j = 0; j < RLIMIT_NLIMITS; j++) ++ free (arg_default_rlimit[j]); ++ + free(arg_default_unit); + strv_free(arg_default_controllers); + free_join_controllers(); +diff --git a/src/manager.c b/src/manager.c +index 3e592b6..c6cd06c 100644 +--- a/src/manager.c ++++ b/src/manager.c +@@ -456,6 +456,7 @@ static void manager_clear_jobs_and_units(Manager *m) { + + void manager_free(Manager *m) { + UnitType c; ++ int i; + + assert(m); + +@@ -501,6 +502,9 @@ void manager_free(Manager *m) { + hashmap_free(m->cgroup_bondings); + set_free_free(m->unit_path_cache); + ++ for (i = 0; i < RLIMIT_NLIMITS; i++) ++ free(m->rlimit[i]); ++ + free(m); + } + +@@ -3137,6 +3141,24 @@ int manager_set_default_controllers(Manager *m, char **controllers) { + return 0; + } + ++int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit) { ++ int i; ++ ++ assert(m); ++ ++ for (i = 0; i < RLIMIT_NLIMITS; i++) { ++ if (default_rlimit[i]) { ++ m->rlimit[i] = newdup(struct rlimit, default_rlimit[i], 1); ++ ++ if (!m->rlimit[i]) ++ return -ENOMEM; ++ } ++ } ++ ++ return 0; ++} ++ ++ + void manager_recheck_journal(Manager *m) { + Unit *u; + +diff --git a/src/manager.h b/src/manager.h +index a9d08f0..5f5de8e 100644 +--- a/src/manager.h ++++ b/src/manager.h +@@ -225,6 +225,8 @@ struct Manager { + + ExecOutput default_std_output, default_std_error; + ++ struct rlimit *rlimit[RLIMIT_NLIMITS]; ++ + /* non-zero if we are reloading or reexecuting, */ + int n_reloading; + +@@ -263,6 +265,7 @@ unsigned manager_dispatch_run_queue(Manager *m); + unsigned manager_dispatch_dbus_queue(Manager *m); + + int manager_set_default_controllers(Manager *m, char **controllers); ++int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit); + + int manager_loop(Manager *m); + +diff --git a/src/service.c b/src/service.c +index 8b5c0b0..892392d 100644 +--- a/src/service.c ++++ b/src/service.c +@@ -109,6 +109,7 @@ static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { + + static void service_init(Unit *u) { + Service *s = SERVICE(u); ++ int i; + + assert(u); + assert(u->load_state == UNIT_STUB); +@@ -127,6 +128,9 @@ static void service_init(Unit *u) { + s->guess_main_pid = true; + + exec_context_init(&s->exec_context); ++ for (i = 0; i < RLIMIT_NLIMITS; i++) ++ if (UNIT(s)->manager->rlimit[i]) ++ s->exec_context.rlimit[i] = newdup(struct rlimit, UNIT(s)->manager->rlimit[i], 1); + + RATELIMIT_INIT(s->start_limit, 10*USEC_PER_SEC, 5); + +-- +1.7.7 +