From: Lennart Poettering Date: Thu, 6 Aug 2015 13:50:54 +0000 (+0300) Subject: machined: rework state tracking logic for machines X-Git-Tag: v225~88^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F898%2Fhead;p=thirdparty%2Fsystemd.git machined: rework state tracking logic for machines This splits up the stopping logic for machines into two steps: first on machine_stop() we begin with the shutdown of a machine by queuing the stop method call for it. Then, in machine_finalize() we actually remove the rest of its runtime context. This mimics closely how sessions are handled in logind. This also reworks the GC logic to strictly check the current state of the machine unit, rather than shortcutting a few cases, like for example assuming that UnitRemoved really means a machine is gone (which it isn't since Reloading might trigger it, see #376). Fixes #376. --- diff --git a/src/machine/machine.c b/src/machine/machine.c index 4fd56a11ae9..f045159d41d 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -419,7 +419,19 @@ static int machine_stop_scope(Machine *m) { } int machine_stop(Machine *m) { - int r = 0, k; + int r; + assert(m); + + r = machine_stop_scope(m); + + m->stopping = true; + + machine_save(m); + + return r; +} + +int machine_finalize(Machine *m) { assert(m); if (m->started) @@ -430,20 +442,15 @@ int machine_stop(Machine *m) { LOG_MESSAGE("Machine %s terminated.", m->name), NULL); - /* Kill cgroup */ - k = machine_stop_scope(m); - if (k < 0) - r = k; - machine_unlink(m); machine_add_to_gc_queue(m); - if (m->started) + if (m->started) { machine_send_signal(m, false); + m->started = false; + } - m->started = false; - - return r; + return 0; } bool machine_check_gc(Machine *m, bool drop_not_started) { @@ -474,8 +481,11 @@ void machine_add_to_gc_queue(Machine *m) { MachineState machine_get_state(Machine *s) { assert(s); + if (s->stopping) + return MACHINE_CLOSING; + if (s->scope_job) - return s->started ? MACHINE_OPENING : MACHINE_CLOSING; + return MACHINE_OPENING; return MACHINE_RUNNING; } diff --git a/src/machine/machine.h b/src/machine/machine.h index b1cd26662f7..0132b65a979 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -82,6 +82,7 @@ struct Machine { bool in_gc_queue:1; bool started:1; + bool stopping:1; sd_bus_message *create_message; @@ -100,6 +101,7 @@ bool machine_check_gc(Machine *m, bool drop_not_started); void machine_add_to_gc_queue(Machine *m); int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error); int machine_stop(Machine *m); +int machine_finalize(Machine *m); int machine_save(Machine *m); int machine_load(Machine *m); int machine_kill(Machine *m, KillWho who, int signo); diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 49f41c62d7a..08a7f58ef57 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1136,8 +1136,9 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err machine_send_create_reply(machine, &e); } - } else - machine_save(machine); + } + + machine_save(machine); } machine_add_to_gc_queue(machine); @@ -1146,7 +1147,7 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err int match_properties_changed(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_free_ char *unit = NULL; - const char *path, *interface; + const char *path; Manager *m = userdata; Machine *machine; int r; @@ -1170,36 +1171,6 @@ int match_properties_changed(sd_bus_message *message, void *userdata, sd_bus_err if (!machine) return 0; - r = sd_bus_message_read(message, "s", &interface); - if (r < 0) { - bus_log_parse_error(r); - return 0; - } - - if (streq(interface, "org.freedesktop.systemd1.Unit")) { - struct properties { - char *active_state; - char *sub_state; - } properties = {}; - - const struct bus_properties_map map[] = { - { "ActiveState", "s", NULL, offsetof(struct properties, active_state) }, - { "SubState", "s", NULL, offsetof(struct properties, sub_state) }, - {} - }; - - r = bus_message_map_properties_changed(message, map, &properties); - if (r < 0) - bus_log_parse_error(r); - else if (streq_ptr(properties.active_state, "inactive") || - streq_ptr(properties.active_state, "failed") || - streq_ptr(properties.sub_state, "auto-restart")) - machine_release_unit(machine); - - free(properties.active_state); - free(properties.sub_state); - } - machine_add_to_gc_queue(machine); return 0; } @@ -1223,9 +1194,7 @@ int match_unit_removed(sd_bus_message *message, void *userdata, sd_bus_error *er if (!machine) return 0; - machine_release_unit(machine); machine_add_to_gc_queue(machine); - return 0; } diff --git a/src/machine/machined.c b/src/machine/machined.c index 9bfe2add547..1eeeaf17a50 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -247,8 +247,16 @@ void manager_gc(Manager *m, bool drop_not_started) { LIST_REMOVE(gc_queue, m->machine_gc_queue, machine); machine->in_gc_queue = false; - if (!machine_check_gc(machine, drop_not_started)) { + /* First, if we are not closing yet, initiate stopping */ + if (!machine_check_gc(machine, drop_not_started) && + machine_get_state(machine) != MACHINE_CLOSING) machine_stop(machine); + + /* Now, the stop stop probably made this referenced + * again, but if it didn't, then it's time to let it + * go entirely. */ + if (!machine_check_gc(machine, drop_not_started)) { + machine_finalize(machine); machine_free(machine); } }