Accepting request 380337 from home:jengelh:branches:Base:System
- 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 OBS-URL: https://build.opensuse.org/request/show/380337 OBS-URL: https://build.opensuse.org/package/show/Base:System/systemd?expand=0&rev=934
This commit is contained in:
parent
023ef9eec7
commit
527ef37082
193
0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch
Normal file
193
0001-core-fix-bus-name-synchronization-after-daemon-reloa.patch
Normal file
@ -0,0 +1,193 @@
|
||||
From d8ccf5fdc91c46ab5d0ae86e38c206bc508d4188 Mon Sep 17 00:00:00 2001 [> v228]
|
||||
From: Daniel Mack <daniel@zonque.org>
|
||||
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
|
||||
|
@ -0,0 +1,74 @@
|
||||
From 8936a5e34dbfa9274348f3fef99f7c9f9327ddf9 Mon Sep 17 00:00:00 2001 [> v228]
|
||||
From: Daniel Mack <daniel@zonque.org>
|
||||
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
|
||||
|
@ -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
|
||||
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user