From 6797c324a653f119a3d7133122648aaa4878ddd6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Jul 2013 23:31:40 +0200 Subject: [PATCH] logind: don't misunderstand UnitRemoved signals during reloading When PID 1 reloads the units logind/machined will see UnitRemoved signals for all units. Instead of trusting these immediately, let's check the actual unit state before considering a unit gone, so that reloading PID 1 is not mistaken as the end of all sessions. --- src/login/logind-dbus.c | 72 ++++++++++++++++++++++++++----------- src/login/logind.c | 13 ++++++- src/machine/machined-dbus.c | 51 ++++++++++++++++++++++---- src/machine/machined.c | 12 +++++++ 4 files changed, 120 insertions(+), 28 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 29a196323..ec46fdc23 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2427,28 +2427,32 @@ DBusHandlerResult bus_message_filter( } session = hashmap_get(m->session_units, unit); - if (session) { - hashmap_remove(m->session_units, session->scope); - free(session->scope); - session->scope = NULL; + if (session) + session_add_to_gc_queue(session); - session_add_to_gc_queue(session); + user = hashmap_get(m->user_units, unit); + if (user) + user_add_to_gc_queue(user); + + } else if (dbus_message_is_signal(message, "org.freedesktop.systemd1.Manager", "Reloading")) { + dbus_bool_t b; + + if (!dbus_message_get_args(message, &error, + DBUS_TYPE_BOOLEAN, &b, + DBUS_TYPE_INVALID)) { + log_error("Failed to parse Reloading message: %s", bus_error_message(&error)); + goto finish; } - user = hashmap_get(m->user_units, unit); - if (user) { - - if (streq_ptr(unit, user->service)) { - hashmap_remove(m->user_units, user->service); - free(user->service); - user->service = NULL; - } else if (streq_ptr(unit, user->slice)) { - hashmap_remove(m->user_units, user->slice); - free(user->slice); - user->slice = NULL; - } + /* systemd finished reloading, let's recheck all our sessions */ + if (!b) { + Session *session; + Iterator i; - user_add_to_gc_queue(user); + log_debug("System manager has been reloaded, rechecking sessions..."); + + HASHMAP_FOREACH(session, m->sessions, i) + session_add_to_gc_queue(session); } } @@ -2682,6 +2686,16 @@ int manager_stop_unit(Manager *manager, const char *unit, DBusError *error, char DBUS_TYPE_STRING, &fail, DBUS_TYPE_INVALID); if (r < 0) { + if (dbus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) || + dbus_error_has_name(error, BUS_ERROR_LOAD_FAILED)) { + + if (job) + *job = NULL; + + dbus_error_free(error); + return 0; + } + log_error("Failed to stop unit %s: %s", unit, bus_error(error, r)); return r; } @@ -2704,7 +2718,7 @@ int manager_stop_unit(Manager *manager, const char *unit, DBusError *error, char *job = copy; } - return 0; + return 1; } int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, DBusError *error) { @@ -2769,8 +2783,26 @@ int manager_unit_is_active(Manager *manager, const char *unit) { DBUS_TYPE_STRING, &interface, DBUS_TYPE_STRING, &property, DBUS_TYPE_INVALID); - if (r < 0) { + if (dbus_error_has_name(&error, DBUS_ERROR_NO_REPLY) || + dbus_error_has_name(&error, DBUS_ERROR_DISCONNECTED)) { + /* systemd might have droppped off + * momentarily, let's not make this an + * error */ + + dbus_error_free(&error); + return true; + } + + if (dbus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT) || + dbus_error_has_name(&error, BUS_ERROR_LOAD_FAILED)) { + /* If the unit is already unloaded then it's + * not active */ + + dbus_error_free(&error); + return false; + } + log_error("Failed to query ActiveState: %s", bus_error(&error, r)); dbus_error_free(&error); return r; diff --git a/src/login/logind.c b/src/login/logind.c index fcb3ccf4a..a79ba333d 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -1038,12 +1038,23 @@ static int manager_connect_bus(Manager *m) { "interface='org.freedesktop.DBus.Properties'," "member='PropertiesChanged'", &error); - if (dbus_error_is_set(&error)) { log_error("Failed to add match for PropertiesChanged: %s", bus_error_message(&error)); dbus_error_free(&error); } + dbus_bus_add_match(m->bus, + "type='signal'," + "sender='org.freedesktop.systemd1'," + "interface='org.freedesktop.systemd1.Manager'," + "member='Reloading'," + "path='/org/freedesktop/systemd1'", + &error); + if (dbus_error_is_set(&error)) { + log_error("Failed to add match for Reloading: %s", bus_error_message(&error)); + dbus_error_free(&error); + } + r = bus_method_call_with_reply( m->bus, "org.freedesktop.systemd1", diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 1b1eb3a33..4664c73a9 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -583,12 +583,28 @@ DBusHandlerResult bus_message_filter( } mm = hashmap_get(m->machine_units, unit); - if (mm) { - hashmap_remove(m->machine_units, mm->scope); - free(mm->scope); - mm->scope = NULL; - + if (mm) machine_add_to_gc_queue(mm); + + } else if (dbus_message_is_signal(message, "org.freedesktop.systemd1.Manager", "Reloading")) { + dbus_bool_t b; + + if (!dbus_message_get_args(message, &error, + DBUS_TYPE_BOOLEAN, &b, + DBUS_TYPE_INVALID)) { + log_error("Failed to parse Reloading message: %s", bus_error_message(&error)); + goto finish; + } + + /* systemd finished reloading, let's recheck all our machines */ + if (!b) { + Machine *mm; + Iterator i; + + log_debug("System manager has been reloaded, rechecking machines..."); + + HASHMAP_FOREACH(mm, m->machines, i) + machine_add_to_gc_queue(mm); } } @@ -727,6 +743,16 @@ int manager_stop_unit(Manager *manager, const char *unit, DBusError *error, char DBUS_TYPE_STRING, &fail, DBUS_TYPE_INVALID); if (r < 0) { + if (dbus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) || + dbus_error_has_name(error, BUS_ERROR_LOAD_FAILED)) { + + if (job) + *job = NULL; + + dbus_error_free(error); + return 0; + } + log_error("Failed to stop unit %s: %s", unit, bus_error(error, r)); return r; } @@ -749,7 +775,7 @@ int manager_stop_unit(Manager *manager, const char *unit, DBusError *error, char *job = copy; } - return 0; + return 1; } int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, DBusError *error) { @@ -814,8 +840,19 @@ int manager_unit_is_active(Manager *manager, const char *unit) { DBUS_TYPE_STRING, &interface, DBUS_TYPE_STRING, &property, DBUS_TYPE_INVALID); - if (r < 0) { + if (dbus_error_has_name(&error, DBUS_ERROR_NO_REPLY) || + dbus_error_has_name(&error, DBUS_ERROR_DISCONNECTED)) { + dbus_error_free(&error); + return true; + } + + if (dbus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT) || + dbus_error_has_name(&error, BUS_ERROR_LOAD_FAILED)) { + dbus_error_free(&error); + return false; + } + log_error("Failed to query ActiveState: %s", bus_error(&error, r)); dbus_error_free(&error); return r; diff --git a/src/machine/machined.c b/src/machine/machined.c index f2803a18c..c7c4ecc37 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -233,6 +233,18 @@ static int manager_connect_bus(Manager *m) { dbus_error_free(&error); } + dbus_bus_add_match(m->bus, + "type='signal'," + "sender='org.freedesktop.systemd1'," + "interface='org.freedesktop.systemd1.Manager'," + "member='Reloading'," + "path='/org/freedesktop/systemd1'", + &error); + if (dbus_error_is_set(&error)) { + log_error("Failed to add match for Reloading: %s", bus_error_message(&error)); + dbus_error_free(&error); + } + r = bus_method_call_with_reply( m->bus, "org.freedesktop.systemd1", -- 2.30.2