diff --git a/0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch b/0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch new file mode 100644 index 00000000..b1a18d36 --- /dev/null +++ b/0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch @@ -0,0 +1,193 @@ +From d8ccf5fdc91c46ab5d0ae86e38c206bc508d4188 Mon Sep 17 00:00:00 2001 [> v228] +From: Daniel Mack +Date: Fri, 18 Dec 2015 17:28:15 +0100 +Subject: [PATCH] core: fix bus name synchronization after daemon-reload + +During daemon-reload, PID1 temporarly loses its DBus connection, so there's +a small window in which all signals sent by dbus-daemon are lost. + +This is a problem, since we rely on the NameOwnerChanged signals in order to +consider a service with Type=dbus fully started or terminated, respectively. + +In order to fix this, a rewrite of bus_list_names() is necessary. We used +to walk the current list of names on the bus, and blindly triggered the +bus_name_owner_change() callback on each service, providing the actual name +as current owner. This implementation has a number of problems: + +* We cannot detect if the the name was moved from one owner to the other + while we were reloading + +* We don't notify services which missed the name loss signal + +* Providing the actual name as current owner is a hack, as the comment also + admits. + +To fix this, this patch carries the following changes: + +* Track the name of the current bus name owner, and (de-)serialize it + during reload. This way, we can detect changes. + +* In bus_list_names(), walk the list of bus names we're interested in + first, and then see if the name is active on the bus. If it is, + check it it's still the same as it used to be, and synthesize + NameOwnerChanged signals for the name add and/or loss. + +This should fully synchronize the current name list with the internal +state of all services. +--- + src/core/dbus.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--------- + src/core/service.c | 14 ++++++++++++ + src/core/service.h | 1 + + 3 files changed, 69 insertions(+), 10 deletions(-) + +diff --git a/src/core/dbus.c b/src/core/dbus.c +index e7ee216..58069f5 100644 +--- a/src/core/dbus.c ++++ b/src/core/dbus.c +@@ -736,7 +736,9 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void + + static int bus_list_names(Manager *m, sd_bus *bus) { + _cleanup_strv_free_ char **names = NULL; +- char **i; ++ const char *name; ++ Iterator i; ++ Unit *u; + int r; + + assert(m); +@@ -746,15 +748,55 @@ static int bus_list_names(Manager *m, sd_bus *bus) { + if (r < 0) + return log_error_errno(r, "Failed to get initial list of names: %m"); + +- /* This is a bit hacky, we say the owner of the name is the +- * name itself, because we don't want the extra traffic to +- * figure out the real owner. */ +- STRV_FOREACH(i, names) { +- Unit *u; ++ /* We have to synchronize the current bus names with the ++ * list of active services. To do this, walk the list of ++ * all units with bus names. */ ++ HASHMAP_FOREACH_KEY(u, name, m->watch_bus, i) { ++ Service *s = SERVICE(u); ++ ++ assert(s); + +- u = hashmap_get(m->watch_bus, *i); +- if (u) +- UNIT_VTABLE(u)->bus_name_owner_change(u, *i, NULL, *i); ++ if (!streq_ptr(s->bus_name, name)) { ++ log_unit_warning(u, "Bus name has changed from %s → %s, ignoring.", s->bus_name, name); ++ continue; ++ } ++ ++ /* Check if a service's bus name is in the list of currently ++ * active names */ ++ if (strv_contains(names, name)) { ++ _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; ++ const char *unique; ++ ++ /* If it is, determine its current owner */ ++ r = sd_bus_get_name_creds(bus, name, SD_BUS_CREDS_UNIQUE_NAME, &creds); ++ if (r < 0) { ++ log_error_errno(r, "Failed to get bus name owner %s: %m", name); ++ continue; ++ } ++ ++ r = sd_bus_creds_get_unique_name(creds, &unique); ++ if (r < 0) { ++ log_error_errno(r, "Failed to get unique name for %s: %m", name); ++ continue; ++ } ++ ++ /* Now, let's compare that to the previous bus owner, and ++ * if it's still the same, all is fine, so just don't ++ * bother the service. Otherwise, the name has apparently ++ * changed, so synthesize a name owner changed signal. */ ++ ++ if (!streq_ptr(unique, s->bus_name_owner)) ++ UNIT_VTABLE(u)->bus_name_owner_change(u, name, s->bus_name_owner, unique); ++ } else { ++ /* So, the name we're watching is not on the bus. ++ * This either means it simply hasn't appeared yet, ++ * or it was lost during the daemon reload. ++ * Check if the service has a stored name owner, ++ * and synthesize a name loss signal in this case. */ ++ ++ if (s->bus_name_owner) ++ UNIT_VTABLE(u)->bus_name_owner_change(u, name, s->bus_name_owner, NULL); ++ } + } + + return 0; +@@ -808,7 +850,9 @@ static int bus_setup_api(Manager *m, sd_bus *bus) { + if (r < 0) + return log_error_errno(r, "Failed to register name: %m"); + +- bus_list_names(m, bus); ++ r = bus_list_names(m, bus); ++ if (r < 0) ++ return r; + + log_debug("Successfully connected to API bus."); + return 0; +diff --git a/src/core/service.c b/src/core/service.c +index 41a729c..c5b689a 100644 +--- a/src/core/service.c ++++ b/src/core/service.c +@@ -323,6 +323,8 @@ static void service_done(Unit *u) { + s->bus_name = mfree(s->bus_name); + } + ++ s->bus_name_owner = mfree(s->bus_name_owner); ++ + s->bus_endpoint_fd = safe_close(s->bus_endpoint_fd); + service_close_socket_fd(s); + service_connection_unref(s); +@@ -2122,6 +2124,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { + + unit_serialize_item(u, f, "main-pid-known", yes_no(s->main_pid_known)); + unit_serialize_item(u, f, "bus-name-good", yes_no(s->bus_name_good)); ++ unit_serialize_item(u, f, "bus-name-owner", s->bus_name_owner); + + r = unit_serialize_item_escaped(u, f, "status-text", s->status_text); + if (r < 0) +@@ -2249,6 +2252,10 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, + log_unit_debug(u, "Failed to parse bus-name-good value: %s", value); + else + s->bus_name_good = b; ++ } else if (streq(key, "bus-name-owner")) { ++ r = free_and_strdup(&s->bus_name_owner, value); ++ if (r < 0) ++ log_unit_error_errno(u, r, "Unable to deserialize current bus owner %s: %m", value); + } else if (streq(key, "status-text")) { + char *t; + +@@ -3134,6 +3141,13 @@ static void service_bus_name_owner_change( + + s->bus_name_good = !!new_owner; + ++ /* Track the current owner, so we can reconstruct changes after a daemon reload */ ++ r = free_and_strdup(&s->bus_name_owner, new_owner); ++ if (r < 0) { ++ log_unit_error_errno(u, r, "Unable to set new bus name owner %s: %m", new_owner); ++ return; ++ } ++ + if (s->type == SERVICE_DBUS) { + + /* service_enter_running() will figure out what to +diff --git a/src/core/service.h b/src/core/service.h +index d0faad8..19efbcc 100644 +--- a/src/core/service.h ++++ b/src/core/service.h +@@ -172,6 +172,7 @@ struct Service { + bool reset_cpu_usage:1; + + char *bus_name; ++ char *bus_name_owner; /* unique name of the current owner */ + + char *status_text; + int status_errno; +-- +2.6.2 + diff --git a/0001-core-re-sync-bus-name-list-after-deserializing-durin.patch b/0001-core-re-sync-bus-name-list-after-deserializing-durin.patch new file mode 100644 index 00000000..11b6a99f --- /dev/null +++ b/0001-core-re-sync-bus-name-list-after-deserializing-durin.patch @@ -0,0 +1,74 @@ +From 8936a5e34dbfa9274348f3fef99f7c9f9327ddf9 Mon Sep 17 00:00:00 2001 [> v228] +From: Daniel Mack +Date: Tue, 22 Dec 2015 11:37:09 +0100 +Subject: [PATCH] core: re-sync bus name list after deserializing during + daemon-reload + +When the daemon reloads, it doesn not actually give up its DBus connection, +as wrongly stated in an earlier commit. However, even though the bus +connection stays open, the daemon flushes out all its internal state. + +Hence, if there is a NameOwnerChanged signal after the flush and before the +deserialization, it cannot be matched against any pending unit. + +To fix this, rename bus_list_names() to manager_sync_bus_names() and call +it explicitly at the end of the daemon reload operation. +--- + src/core/dbus.c | 4 ++-- + src/core/dbus.h | 2 ++ + src/core/manager.c | 4 ++++ + 3 files changed, 8 insertions(+), 2 deletions(-) + +diff --git a/src/core/dbus.c b/src/core/dbus.c +index 58069f5..1d89b9e 100644 +--- a/src/core/dbus.c ++++ b/src/core/dbus.c +@@ -734,7 +734,7 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void + return 0; + } + +-static int bus_list_names(Manager *m, sd_bus *bus) { ++int manager_sync_bus_names(Manager *m, sd_bus *bus) { + _cleanup_strv_free_ char **names = NULL; + const char *name; + Iterator i; +@@ -850,7 +850,7 @@ static int bus_setup_api(Manager *m, sd_bus *bus) { + if (r < 0) + return log_error_errno(r, "Failed to register name: %m"); + +- r = bus_list_names(m, bus); ++ r = manager_sync_bus_names(m, bus); + if (r < 0) + return r; + +diff --git a/src/core/dbus.h b/src/core/dbus.h +index 4f06ad1..ff76166 100644 +--- a/src/core/dbus.h ++++ b/src/core/dbus.h +@@ -34,6 +34,8 @@ void bus_track_serialize(sd_bus_track *t, FILE *f); + int bus_track_deserialize_item(char ***l, const char *line); + int bus_track_coldplug(Manager *m, sd_bus_track **t, char ***l); + ++int manager_sync_bus_names(Manager *m, sd_bus *bus); ++ + int bus_foreach_bus(Manager *m, sd_bus_track *subscribed2, int (*send_message)(sd_bus *bus, void *userdata), void *userdata); + + int bus_verify_manage_units_async(Manager *m, sd_bus_message *call, sd_bus_error *error); +diff --git a/src/core/manager.c b/src/core/manager.c +index e65616a..ffe27be 100644 +--- a/src/core/manager.c ++++ b/src/core/manager.c +@@ -2574,6 +2574,10 @@ int manager_reload(Manager *m) { + /* Third, fire things up! */ + manager_coldplug(m); + ++ /* Sync current state of bus names with our set of listening units */ ++ if (m->api_bus) ++ manager_sync_bus_names(m, m->api_bus); ++ + assert(m->n_reloading > 0); + m->n_reloading--; + +-- +2.6.2 + diff --git a/systemd-mini.changes b/systemd-mini.changes index 3c123107..9556ac70 100644 --- a/systemd-mini.changes +++ b/systemd-mini.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Sat Mar 26 15:59:36 UTC 2016 - jengelh@inai.de + +- Add two patches which address logind/networkd disappearing from + dbus (and busctl) even while the units and processes continue + running. + 0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch + 0001-core-re-sync-bus-name-list-after-deserializing-durin.patch + ------------------------------------------------------------------- Mon Mar 14 18:04:10 UTC 2016 - fbui@suse.com diff --git a/systemd-mini.spec b/systemd-mini.spec index 87119bc0..f6b00696 100644 --- a/systemd-mini.spec +++ b/systemd-mini.spec @@ -255,6 +255,10 @@ Patch524: 0001-nss-mymachines-do-not-allow-overlong-machine-names.patch Patch525: 0001-core-exclude-.slice-units-from-systemctl-isolate.patch # PATCH-FIX-UPSTREAM -- fixed after 228 Patch526: systemd-228-nspawn-make-journal-linking-non-fatal-in-try-and-auto.diff +# PATCH-FIX-UPSTREAM -- fixed after 228 +Patch527: 0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch +# PATCH-FIX-UPSTREAM -- fixed after 228 +Patch528: 0001-core-re-sync-bus-name-list-after-deserializing-durin.patch # UDEV PATCHES # ============ @@ -597,6 +601,8 @@ cp %{SOURCE7} m4/ %patch524 -p1 %patch525 -p1 %patch526 -p1 +%patch527 -p1 +%patch528 -p1 # udev patches %patch1002 -p1 diff --git a/systemd.changes b/systemd.changes index 3c123107..9556ac70 100644 --- a/systemd.changes +++ b/systemd.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Sat Mar 26 15:59:36 UTC 2016 - jengelh@inai.de + +- Add two patches which address logind/networkd disappearing from + dbus (and busctl) even while the units and processes continue + running. + 0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch + 0001-core-re-sync-bus-name-list-after-deserializing-durin.patch + ------------------------------------------------------------------- Mon Mar 14 18:04:10 UTC 2016 - fbui@suse.com diff --git a/systemd.spec b/systemd.spec index 64a88ce1..c2162ef0 100644 --- a/systemd.spec +++ b/systemd.spec @@ -250,6 +250,10 @@ Patch524: 0001-nss-mymachines-do-not-allow-overlong-machine-names.patch Patch525: 0001-core-exclude-.slice-units-from-systemctl-isolate.patch # PATCH-FIX-UPSTREAM -- fixed after 228 Patch526: systemd-228-nspawn-make-journal-linking-non-fatal-in-try-and-auto.diff +# PATCH-FIX-UPSTREAM -- fixed after 228 +Patch527: 0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch +# PATCH-FIX-UPSTREAM -- fixed after 228 +Patch528: 0001-core-re-sync-bus-name-list-after-deserializing-durin.patch # UDEV PATCHES # ============ @@ -592,6 +596,8 @@ cp %{SOURCE7} m4/ %patch524 -p1 %patch525 -p1 %patch526 -p1 +%patch527 -p1 +%patch528 -p1 # udev patches %patch1002 -p1