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 4fd56a11ae9d86ef3ea71dc69976fde2312fa829..f045159d41dbcc92062850bc77b4ed32b60a8903 100644 (file)
--- 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 b1cd26662f7a0e087fd99ab172cc93084ededb5e..0132b65a979343c77ba435946070a7835cba8bc1 100644 (file)
--- 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 49f41c62d7a49f183408f0975df51cbdf470d290..08a7f58ef57fe513d933efb7dc174a410a5fc197 100644 (file)
--- 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 9bfe2add54718905eb4bd0b25e74b4c8d4be5f93..1eeeaf17a50420260df7869d1e265f0d5fb709c8 100644 (file)
--- 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);
                 }
         }