diff options
| -rw-r--r-- | .gitignore | 1 | ||||
| -rw-r--r-- | Makefile | 5 | ||||
| -rw-r--r-- | job.c | 100 | ||||
| -rw-r--r-- | job.h | 5 | ||||
| -rw-r--r-- | manager.c | 259 | ||||
| -rw-r--r-- | test-engine.c | 21 | ||||
| -rw-r--r-- | test-job-type.c | 65 | ||||
| -rw-r--r-- | test2/g.service | 3 |
8 files changed, 371 insertions, 88 deletions
diff --git a/.gitignore b/.gitignore index ff976ff9a9..94e3036c97 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ systemd *.o test-engine +test-job-type @@ -3,7 +3,7 @@ LIBS=-lrt COMMON=name.o util.o set.o hashmap.o strv.o job.o manager.o conf-parser.o load-fragment.o socket-util.o log.o -all: systemd test-engine +all: systemd test-engine test-job-type systemd: main.o $(COMMON) $(CC) $(CFLAGS) -o $@ $^ $(LIBS) @@ -11,5 +11,8 @@ systemd: main.o $(COMMON) test-engine: test-engine.o $(COMMON) $(CC) $(CFLAGS) -o $@ $^ $(LIBS) +test-job-type: test-job-type.o $(COMMON) + $(CC) $(CFLAGS) -o $@ $^ $(LIBS) + clean: rm -f *.o systemd test-engine @@ -1,6 +1,7 @@ /*-*- Mode: C; c-basic-offset: 8 -*-*/ #include <assert.h> +#include <errno.h> #include "macro.h" #include "job.h" @@ -127,7 +128,7 @@ void job_dependency_delete(Job *subject, Job *object, bool *matters) { job_dependency_free(l); } -void job_dump(Job *j, FILE*f, const char *prefix) { +const char* job_type_to_string(JobType t) { static const char* const job_type_table[_JOB_TYPE_MAX] = { [JOB_START] = "start", @@ -139,6 +140,15 @@ void job_dump(Job *j, FILE*f, const char *prefix) { [JOB_TRY_RESTART] = "try-restart", }; + + if (t < 0 || t >= _JOB_TYPE_MAX) + return "n/a"; + + return job_type_table[t]; +} + +void job_dump(Job *j, FILE*f, const char *prefix) { + static const char* const job_state_table[_JOB_STATE_MAX] = { [JOB_WAITING] = "waiting", [JOB_RUNNING] = "running", @@ -153,7 +163,7 @@ void job_dump(Job *j, FILE*f, const char *prefix) { "%s\tAction: %s → %s\n" "%s\tState: %s\n", prefix, j->id, - prefix, name_id(j->name), job_type_table[j->type], + prefix, name_id(j->name), job_type_to_string(j->type), prefix, job_state_table[j->state]); } @@ -168,3 +178,89 @@ 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_STARTED)) + *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_STARTED, 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_STARTED, 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_STARTED, JOB_RELOAD)) + *a = JOB_RELOAD; + else if (types_match(*a, b, JOB_VERIFY_STARTED, JOB_TRY_RESTART) || + types_match(*a, b, JOB_RELOAD, JOB_TRY_RESTART)) + *a = JOB_TRY_RESTART; + else + return -EEXIST; + + return 0; +} + +bool job_type_mergeable(JobType a, JobType b) { + return job_type_merge(&a, b) >= 0; +} + +bool job_type_is_superset(JobType a, JobType b) { + + /* Checks whether operation a is a "superset" of b */ + + if (a == b) + return true; + + switch (a) { + case JOB_START: + return b == JOB_VERIFY_STARTED; + + case JOB_RELOAD: + return b == JOB_VERIFY_STARTED; + + case JOB_RELOAD_OR_START: + return + b == JOB_RELOAD || + b == JOB_START; + + case JOB_RESTART: + return + b == JOB_START || + b == JOB_VERIFY_STARTED || + b == JOB_RELOAD || + b == JOB_RELOAD_OR_START || + b == JOB_TRY_RESTART; + + case JOB_TRY_RESTART: + return + b == JOB_VERIFY_STARTED || + b == JOB_RELOAD; + default: + return false; + + } +} @@ -90,4 +90,9 @@ bool job_is_anchor(Job *j); int job_merge(Job *j, Job *other); +const char* job_type_to_string(JobType t); +int job_type_merge(JobType *a, JobType b); +bool job_type_mergeable(JobType a, JobType b); +bool job_type_is_superset(JobType a, JobType b); + #endif @@ -55,12 +55,24 @@ static void transaction_delete_job(Manager *m, Job *j) { assert(m); assert(j); + /* Deletes one job from the transaction */ + manager_transaction_unlink_job(m, j); if (!j->linked) job_free(j); } +static void transaction_delete_name(Manager *m, Name *n) { + Job *j; + + /* Deletes all jobs associated with a certain name from the + * transaction */ + + while ((j = hashmap_get(m->transaction_jobs, n))) + transaction_delete_job(m, j); +} + static void transaction_abort(Manager *m) { Job *j; @@ -81,6 +93,11 @@ static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsi assert(m); + /* A recursive sweep through the graph that marks all names + * 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. */ + for (l = j ? j->subject_list : m->transaction_anchor; l; l = l->subject_next) { /* This link does not matter */ @@ -98,40 +115,6 @@ static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsi } } -static bool types_match(JobType a, JobType b, JobType c, JobType d) { - return - (a == c && b == d) || - (a == d && b == c); -} - -static int types_merge(JobType *a, JobType b) { - if (*a == b) - return 0; - - if (types_match(*a, b, JOB_START, JOB_VERIFY_STARTED)) - *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_STARTED, 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_STARTED, 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_STARTED, JOB_RELOAD)) - *a = JOB_RELOAD; - else if (types_match(*a, b, JOB_VERIFY_STARTED, JOB_TRY_RESTART) || - types_match(*a, b, JOB_RELOAD, JOB_TRY_RESTART)) - *a = JOB_TRY_RESTART; - - return -EEXIST; -} - static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, JobType t) { JobDependency *l, *last; @@ -140,6 +123,8 @@ static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, Job assert(j->name == other->name); assert(!j->linked); + /* Merges 'other' into 'j' and then deletes j. */ + j->type = t; j->state = JOB_WAITING; @@ -183,6 +168,44 @@ static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, Job transaction_delete_job(m, other); } +static int delete_one_unmergable_job(Manager *m, Job *j) { + Job *k; + + assert(j); + + /* Tries to delete one item in the linked list + * j->transaction_next->transaction_next->... that conflicts + * whith 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 */ + for (; j; j = j->transaction_next) + for (k = j->transaction_next; k; k = k->transaction_next) { + Job *d; + + /* Is this one mergeable? Then skip it */ + if (job_type_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) + 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("Try to fix job merging by deleting job %s", name_id(d->name)); + transaction_delete_job(m, d); + return 0; + } + + return -EINVAL; +} + static int transaction_merge_jobs(Manager *m) { Job *j; void *state; @@ -190,13 +213,39 @@ static int transaction_merge_jobs(Manager *m) { assert(m); + /* 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, state) { + JobType t; + Job *k; + + t = j->type; + for (k = j->transaction_next; k; k = k->transaction_next) { + if ((r = 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 */ + + if ((r = delete_one_unmergable_job(m, j)) >= 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 */ + return r; + } + } + + /* Second step, merge the jobs. */ HASHMAP_FOREACH(j, m->transaction_jobs, state) { JobType t = j->type; Job *k; for (k = j->transaction_next; k; k = k->transaction_next) - if ((r = types_merge(&t, k->type)) < 0) - return r; + assert_se(job_type_merge(&t, k->type) == 0); while ((k = j->transaction_next)) { if (j->linked) { @@ -213,6 +262,20 @@ static int transaction_merge_jobs(Manager *m) { return 0; } +static bool name_matters_to_anchor(Name *n, Job *j) { + assert(n); + assert(!j->transaction_prev); + + /* Checks whether at least one of the jobs for this name + * matters to the anchor. */ + + for (; j; j = j->transaction_next) + if (j->matters_to_anchor) + return true; + + return false; +} + static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned generation) { void *state; Name *n; @@ -220,19 +283,30 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned assert(m); 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. */ /* Did we find a cycle? */ if (j->marker && j->generation == generation) { Job *k; /* So, 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. */ + * 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. */ for (k = from; k; k = (k->generation == generation ? k->marker : NULL)) { - if (!k->matters_to_anchor) { + + if (!k->linked && + !name_matters_to_anchor(k->name, k)) { + /* Ok, we can drop this one, so let's + * do so. */ log_debug("Breaking order cycle by deleting job %s", name_id(k->name)); - transaction_delete_job(m, k); + transaction_delete_name(m, k->name); return -EAGAIN; } @@ -242,19 +316,25 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned break; } - return -ELOOP; + 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 */ j->marker = from; j->generation = generation; - /* We assume that the the dependencies are both-ways, and + /* We assume that the the dependencies are bidirectional, and * hence can ignore NAME_AFTER */ - SET_FOREACH(n, j->name->meta.dependencies[NAME_BEFORE], state) { Job *o; + /* Is there a job for this name? */ if (!(o = hashmap_get(m->transaction_jobs, n))) + + /* Ok, there is no job for this in the + * transaction, but maybe there is already one + * running? */ if (!(o = n->meta.job)) continue; @@ -266,36 +346,19 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned } static int transaction_verify_order(Manager *m, unsigned *generation) { - bool again; + Job *j; + int r; + void *state; + assert(m); assert(generation); - do { - Job *j; - int r; - void *state; - - again = false; - - HASHMAP_FOREACH(j, m->transaction_jobs, state) { + /* Check if the ordering graph is cyclic. If it is, try to fix + * that up by dropping one of the jobs. */ - /* Assume merged */ - assert(!j->transaction_next); - assert(!j->transaction_prev); - - if ((r = transaction_verify_order_one(m, j, NULL, (*generation)++)) < 0) { - - /* There was a cycleq, but it was fixed, - * we need to restart our algorithm */ - if (r == -EAGAIN) { - again = true; - break; - } - - return r; - } - } - } while (again); + HASHMAP_FOREACH(j, m->transaction_jobs, state) + if ((r = transaction_verify_order_one(m, j, NULL, (*generation)++)) < 0) + return r; return 0; } @@ -305,6 +368,8 @@ static void transaction_collect_garbage(Manager *m) { assert(m); + /* Drop jobs that are not required by any other job */ + do { void *state; Job *j; @@ -335,7 +400,9 @@ static int transaction_is_destructive(Manager *m, JobMode mode) { * existing jobs would be replaced */ HASHMAP_FOREACH(j, m->transaction_jobs, state) - if (j->name->meta.job && j->name->meta.job != j) + if (j->name->meta.job && + j->name->meta.job != j && + !job_type_is_superset(j->type, j->name->meta.job->type)) return -EEXIST; return 0; @@ -346,6 +413,8 @@ static int transaction_apply(Manager *m, JobMode mode) { Job *j; int r; + /* Moves the transaction jobs to the set of active jobs */ + HASHMAP_FOREACH(j, m->transaction_jobs, state) { if (j->linked) continue; @@ -376,6 +445,8 @@ static int transaction_apply(Manager *m, JobMode mode) { job_dependency_free(j->object_list); } + m->transaction_anchor = NULL; + return 0; rollback: @@ -403,23 +474,47 @@ static int transaction_activate(Manager *m, JobMode mode) { /* First step: figure out which jobs matter */ transaction_find_jobs_that_matter_to_anchor(m, NULL, generation++); - /* Second step: let's merge entries we can merge */ - if ((r = transaction_merge_jobs(m)) < 0) - goto rollback; + for (;;) { + /* Second step: Let's remove unneeded jobs that might + * be lurking. */ + transaction_collect_garbage(m); - /* Third step: verify order makes sense */ - if ((r = transaction_verify_order(m, &generation)) < 0) - goto rollback; + /* Third step: verify order makes sense and correct + * cycles if necessary and possible */ + if ((r = transaction_verify_order(m, &generation)) >= 0) + break; - /* Third step: do garbage colletion */ - transaction_collect_garbage(m); + if (r != -EAGAIN) + goto rollback; - /* Fourth step: check whether we can actually apply this */ + /* Let's see if the resulting transaction ordering + * graph is still cyclic... */ + } + + for (;;) { + /* Fourth step: let's drop unmergable entries if + * necessary and possible, merge entries we can + * merge */ + if ((r = transaction_merge_jobs(m)) >= 0) + break; + + if (r != -EAGAIN) + goto rollback; + + /* Fifth step: an entry got dropped, let's garbage + * collect its dependencies. */ + transaction_collect_garbage(m); + + /* Let's see if the resulting transaction still has + * unmergable entries ... */ + } + + /* Sith step: check whether we can actually apply this */ if (mode == JOB_FAIL) if ((r = transaction_is_destructive(m, mode)) < 0) goto rollback; - /* Fifth step: apply changes */ + /* Seventh step: apply changes */ if ((r = transaction_apply(m, mode)) < 0) goto rollback; @@ -664,10 +759,10 @@ int manager_load_name(Manager *m, const char *name, Name **_ret) { return -ENOMEM; } - if (set_put(ret->meta.names, n) < 0) { + if ((r = set_put(ret->meta.names, n)) < 0) { name_free(ret); free(n); - return -ENOMEM; + return r; } if ((r = name_link(ret)) < 0) { diff --git a/test-engine.c b/test-engine.c index 15ce70f1ac..f92b05a7dd 100644 --- a/test-engine.c +++ b/test-engine.c @@ -9,7 +9,7 @@ int main(int argc, char *argv[]) { Manager *m = NULL; - Name *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e; + Name *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL; Job *j; assert_se(chdir("test2") == 0); @@ -33,13 +33,28 @@ int main(int argc, char *argv[]) { manager_dump_names(m, stdout, "\t"); printf("Test2: (Cyclic Order, Unfixable)\n"); - assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, false, &j) == -ELOOP); + assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, false, &j) == -ENOEXEC); manager_dump_jobs(m, stdout, "\t"); - printf("Test3: (Cyclic Order, Fixable)\n"); + printf("Test3: (Cyclic Order, Fixable, Garbage Collector)\n"); assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, false, &j) == 0); manager_dump_jobs(m, stdout, "\t"); + printf("Test4: (Identical transaction)\n"); + assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, false, &j) == 0); + manager_dump_jobs(m, stdout, "\t"); + + printf("Loaded3:\n"); + assert_se(manager_load_name(m, "g.service", &g) == 0); + manager_dump_names(m, stdout, "\t"); + + printf("Test5: (Colliding transaction, fail)\n"); + assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, false, &j) == -EEXIST); + + printf("Test6: (Colliding transaction, replace)\n"); + assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, false, &j) == 0); + manager_dump_jobs(m, stdout, "\t"); + manager_free(m); return 0; diff --git a/test-job-type.c b/test-job-type.c new file mode 100644 index 0000000000..db5c329e13 --- /dev/null +++ b/test-job-type.c @@ -0,0 +1,65 @@ +/*-*- Mode: C; c-basic-offset: 8 -*-*/ + +#include <stdio.h> +#include <errno.h> +#include <string.h> +#include <unistd.h> + +#include "job.h" + +int main(int argc, char*argv[]) { + JobType a, b, c, d, e, f, g; + + for (a = 0; a < _JOB_TYPE_MAX; a++) + for (b = 0; b < _JOB_TYPE_MAX; b++) { + + if (!job_type_mergeable(a, b)) + printf("Not mergeable: %s + %s\n", job_type_to_string(a), job_type_to_string(b)); + + for (c = 0; c < _JOB_TYPE_MAX; c++) { + + /* Verify transitivity of mergeability + * of job types */ + assert(!job_type_mergeable(a, b) || + !job_type_mergeable(b, c) || + job_type_mergeable(a, c)); + + d = a; + if (job_type_merge(&d, b) >= 0) { + + printf("%s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(d)); + + /* Verify that merged entries can be + * merged with the same entries they + * can be merged with seperately */ + assert(!job_type_mergeable(a, c) || job_type_mergeable(d, c)); + assert(!job_type_mergeable(b, c) || job_type_mergeable(d, c)); + + /* Verify that if a merged + * with b is not mergable with + * c then either a or b is not + * mergeable with c either. */ + assert(job_type_mergeable(d, c) || !job_type_mergeable(a, c) || !job_type_mergeable(b, c)); + + e = b; + if (job_type_merge(&e, c) >= 0) { + + /* Verify associativity */ + + f = d; + assert(job_type_merge(&f, c) == 0); + + g = e; + assert(job_type_merge(&g, a) == 0); + + assert(f == g); + + 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)); + } + } + } + } + + + return 0; +} diff --git a/test2/g.service b/test2/g.service new file mode 100644 index 0000000000..e811e6083d --- /dev/null +++ b/test2/g.service @@ -0,0 +1,3 @@ +[Meta] +Description=G +Conflicts=e.service |