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