Marcus Meissner
55aeaf2355
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/package/show/Base:System/systemd?expand=0&rev=934
194 lines
8.0 KiB
Diff
194 lines
8.0 KiB
Diff
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
|
|
|