Merge pull request #10963 from poettering/bus-force-state-change-signal

force PropertiesChanged bus signal on all unit state changes
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2018-12-06 16:42:21 +01:00 committed by GitHub
commit 2d479ff1cc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 159 additions and 28 deletions

4
TODO
View file

@ -23,6 +23,10 @@ Janitorial Clean-ups:
Features:
* when importing an fs tree with machined, optionally apply userns-rec-chown
* when importing an fs tree with machined, complain if image is not an OS
* when we fork off generators and such, lower LIMIT_NOFILE soft limit to 1K
* rework seccomp/nnp logic that that even if User= is used in combination with

View file

@ -16,6 +16,7 @@
#include "bus-error.h"
#include "bus-util.h"
#include "dbus-automount.h"
#include "dbus-unit.h"
#include "fd-util.h"
#include "format-util.h"
#include "io-util.h"
@ -237,6 +238,9 @@ static void automount_set_state(Automount *a, AutomountState state) {
AutomountState old_state;
assert(a);
if (a->state != state)
bus_unit_send_pending_change_signal(UNIT(a), false);
old_state = a->state;
a->state = state;

View file

@ -4,6 +4,7 @@
#include "alloc-util.h"
#include "dbus-job.h"
#include "dbus-unit.h"
#include "dbus.h"
#include "job.h"
#include "log.h"
@ -173,6 +174,9 @@ void bus_job_send_change_signal(Job *j) {
assert(j);
/* Make sure that any change signal on the unit is reflected before we send out the change signal on the job */
bus_unit_send_pending_change_signal(j->unit, true);
if (j->in_dbus_queue) {
LIST_REMOVE(dbus_queue, j->manager->dbus_job_queue, j);
j->in_dbus_queue = false;
@ -185,6 +189,21 @@ void bus_job_send_change_signal(Job *j) {
j->sent_dbus_new_signal = true;
}
void bus_job_send_pending_change_signal(Job *j, bool including_new) {
assert(j);
if (!j->in_dbus_queue)
return;
if (!j->sent_dbus_new_signal && !including_new)
return;
if (MANAGER_IS_RELOADING(j->unit->manager))
return;
bus_job_send_change_signal(j);
}
static int send_removed_signal(sd_bus *bus, void *userdata) {
_cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
_cleanup_free_ char *p = NULL;
@ -222,6 +241,9 @@ void bus_job_send_removed_signal(Job *j) {
if (!j->sent_dbus_new_signal)
bus_job_send_change_signal(j);
/* Make sure that any change signal on the unit is reflected before we send out the change signal on the job */
bus_unit_send_pending_change_signal(j->unit, true);
r = bus_foreach_bus(j->manager, j->bus_track, send_removed_signal, j);
if (r < 0)
log_debug_errno(r, "Failed to send job remove signal for %u: %m", j->id);

View file

@ -12,6 +12,7 @@ int bus_job_method_cancel(sd_bus_message *message, void *job, sd_bus_error *erro
int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_bus_error *error);
void bus_job_send_change_signal(Job *j);
void bus_job_send_pending_change_signal(Job *j, bool including_new);
void bus_job_send_removed_signal(Job *j);
int bus_job_coldplug_bus_track(Job *j);

View file

@ -662,8 +662,8 @@ const sd_bus_vtable bus_unit_vtable[] = {
SD_BUS_PROPERTY("AssertResult", "b", bus_property_get_bool, offsetof(Unit, assert_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
BUS_PROPERTY_DUAL_TIMESTAMP("ConditionTimestamp", offsetof(Unit, condition_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
BUS_PROPERTY_DUAL_TIMESTAMP("AssertTimestamp", offsetof(Unit, assert_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY("Conditions", "a(sbbsi)", property_get_conditions, offsetof(Unit, conditions), 0),
SD_BUS_PROPERTY("Asserts", "a(sbbsi)", property_get_conditions, offsetof(Unit, asserts), 0),
SD_BUS_PROPERTY("Conditions", "a(sbbsi)", property_get_conditions, offsetof(Unit, conditions), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION),
SD_BUS_PROPERTY("Asserts", "a(sbbsi)", property_get_conditions, offsetof(Unit, asserts), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION),
SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, 0, SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("Transient", "b", bus_property_get_bool, offsetof(Unit, transient), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("Perpetual", "b", bus_property_get_bool, offsetof(Unit, perpetual), SD_BUS_VTABLE_PROPERTY_CONST),
@ -675,8 +675,8 @@ const sd_bus_vtable bus_unit_vtable[] = {
SD_BUS_PROPERTY("SuccessAction", "s", property_get_emergency_action, offsetof(Unit, success_action), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("SuccessActionExitStatus", "i", bus_property_get_int, offsetof(Unit, success_action_exit_status), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Unit, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("InvocationID", "ay", bus_property_get_id128, offsetof(Unit, invocation_id), 0),
SD_BUS_PROPERTY("CollectMode", "s", property_get_collect_mode, offsetof(Unit, collect_mode), 0),
SD_BUS_PROPERTY("InvocationID", "ay", bus_property_get_id128, offsetof(Unit, invocation_id), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY("CollectMode", "s", property_get_collect_mode, offsetof(Unit, collect_mode), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("Refs", "as", property_get_refs, 0, 0),
SD_BUS_METHOD("Start", "s", "o", method_start, SD_BUS_VTABLE_UNPRIVILEGED),
@ -1202,6 +1202,27 @@ void bus_unit_send_change_signal(Unit *u) {
u->sent_dbus_new_signal = true;
}
void bus_unit_send_pending_change_signal(Unit *u, bool including_new) {
/* Sends out any pending change signals, but only if they really are pending. This call is used when we are
* about to change state in order to force out a PropertiesChanged signal beforehand if there was one pending
* so that clients can follow the full state transition */
if (!u->in_dbus_queue) /* If not enqueued, don't bother */
return;
if (!u->sent_dbus_new_signal && !including_new) /* If the unit was never announced, don't bother, it's fine if
* the unit appears in the new state right-away (except if the
* caller explicitly asked us to send it anyway) */
return;
if (MANAGER_IS_RELOADING(u->manager)) /* Don't generate unnecessary PropertiesChanged signals for the same unit
* when we are reloading. */
return;
bus_unit_send_change_signal(u);
}
static int send_removed_signal(sd_bus *bus, void *userdata) {
_cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
_cleanup_free_ char *p = NULL;
@ -1300,6 +1321,9 @@ int bus_unit_queue_job(
if (!path)
return -ENOMEM;
/* Before we send the method reply, force out the announcement JobNew for this job */
bus_job_send_pending_change_signal(j, true);
return sd_bus_reply_method_return(message, "o", path);
}

View file

@ -11,6 +11,7 @@ extern const sd_bus_vtable bus_unit_vtable[];
extern const sd_bus_vtable bus_unit_cgroup_vtable[];
void bus_unit_send_change_signal(Unit *u);
void bus_unit_send_pending_change_signal(Unit *u, bool including_new);
void bus_unit_send_removed_signal(Unit *u);
int bus_unit_method_start_generic(sd_bus_message *message, Unit *u, JobType job_type, bool reload_if_possible, sd_bus_error *error);

View file

@ -6,6 +6,7 @@
#include "alloc-util.h"
#include "bus-error.h"
#include "dbus-device.h"
#include "dbus-unit.h"
#include "device-private.h"
#include "device-util.h"
#include "device.h"
@ -115,6 +116,9 @@ static void device_set_state(Device *d, DeviceState state) {
DeviceState old_state;
assert(d);
if (d->state != state)
bus_unit_send_pending_change_signal(UNIT(d), false);
old_state = d->state;
d->state = state;

View file

@ -236,6 +236,9 @@ Job* job_install(Job *j) {
job_add_to_gc_queue(j);
job_add_to_dbus_queue(j); /* announce this job to clients */
unit_add_to_dbus_queue(j->unit); /* The Job property of the unit has changed now */
return j;
}

View file

@ -11,6 +11,7 @@
#include "alloc-util.h"
#include "dbus-mount.h"
#include "dbus-unit.h"
#include "device.h"
#include "escape.h"
#include "exit-status.h"
@ -640,6 +641,9 @@ static void mount_set_state(Mount *m, MountState state) {
MountState old_state;
assert(m);
if (m->state != state)
bus_unit_send_pending_change_signal(UNIT(m), false);
old_state = m->state;
m->state = state;

View file

@ -8,6 +8,7 @@
#include "bus-error.h"
#include "bus-util.h"
#include "dbus-path.h"
#include "dbus-unit.h"
#include "fd-util.h"
#include "fs-util.h"
#include "glob-util.h"
@ -410,6 +411,9 @@ static void path_set_state(Path *p, PathState state) {
PathState old_state;
assert(p);
if (p->state != state)
bus_unit_send_pending_change_signal(UNIT(p), false);
old_state = p->state;
p->state = state;

View file

@ -5,6 +5,7 @@
#include "alloc-util.h"
#include "dbus-scope.h"
#include "dbus-unit.h"
#include "load-dropin.h"
#include "log.h"
#include "scope.h"
@ -82,6 +83,9 @@ static void scope_set_state(Scope *s, ScopeState state) {
ScopeState old_state;
assert(s);
if (s->state != state)
bus_unit_send_pending_change_signal(UNIT(s), false);
old_state = s->state;
s->state = state;

View file

@ -12,6 +12,7 @@
#include "bus-kernel.h"
#include "bus-util.h"
#include "dbus-service.h"
#include "dbus-unit.h"
#include "def.h"
#include "env-util.h"
#include "escape.h"
@ -1035,6 +1036,9 @@ static void service_set_state(Service *s, ServiceState state) {
assert(s);
if (s->state != state)
bus_unit_send_pending_change_signal(UNIT(s), false);
table = s->type == SERVICE_IDLE ? state_translation_table_idle : state_translation_table;
old_state = s->state;

View file

@ -4,6 +4,7 @@
#include "alloc-util.h"
#include "dbus-slice.h"
#include "dbus-unit.h"
#include "log.h"
#include "serialize.h"
#include "slice.h"
@ -29,6 +30,9 @@ static void slice_set_state(Slice *t, SliceState state) {
SliceState old_state;
assert(t);
if (t->state != state)
bus_unit_send_pending_change_signal(UNIT(t), false);
old_state = t->state;
t->state = state;

View file

@ -17,6 +17,7 @@
#include "bus-util.h"
#include "copy.h"
#include "dbus-socket.h"
#include "dbus-unit.h"
#include "def.h"
#include "exit-status.h"
#include "fd-util.h"
@ -1742,6 +1743,9 @@ static void socket_set_state(Socket *s, SocketState state) {
SocketState old_state;
assert(s);
if (s->state != state)
bus_unit_send_pending_change_signal(UNIT(s), false);
old_state = s->state;
s->state = state;

View file

@ -9,6 +9,7 @@
#include "alloc-util.h"
#include "dbus-swap.h"
#include "dbus-unit.h"
#include "device-private.h"
#include "device-util.h"
#include "device.h"
@ -480,6 +481,9 @@ static void swap_set_state(Swap *s, SwapState state) {
assert(s);
if (s->state != state)
bus_unit_send_pending_change_signal(UNIT(s), false);
old_state = s->state;
s->state = state;

View file

@ -1,6 +1,7 @@
/* SPDX-License-Identifier: LGPL-2.1+ */
#include "dbus-target.h"
#include "dbus-unit.h"
#include "log.h"
#include "serialize.h"
#include "special.h"
@ -18,6 +19,9 @@ static void target_set_state(Target *t, TargetState state) {
TargetState old_state;
assert(t);
if (t->state != state)
bus_unit_send_pending_change_signal(UNIT(t), false);
old_state = t->state;
t->state = state;

View file

@ -6,6 +6,7 @@
#include "bus-error.h"
#include "bus-util.h"
#include "dbus-timer.h"
#include "dbus-unit.h"
#include "fs-util.h"
#include "parse-util.h"
#include "random-util.h"
@ -247,6 +248,9 @@ static void timer_set_state(Timer *t, TimerState state) {
TimerState old_state;
assert(t);
if (t->state != state)
bus_unit_send_pending_change_signal(UNIT(t), false);
old_state = t->state;
t->state = state;

View file

@ -1639,6 +1639,8 @@ static bool unit_condition_test(Unit *u) {
dual_timestamp_get(&u->condition_timestamp);
u->condition_result = unit_condition_test_list(u, u->conditions, condition_type_to_string);
unit_add_to_dbus_queue(u);
return u->condition_result;
}
@ -1648,6 +1650,8 @@ static bool unit_assert_test(Unit *u) {
dual_timestamp_get(&u->assert_timestamp);
u->assert_result = unit_condition_test_list(u, u->asserts, assert_type_to_string);
unit_add_to_dbus_queue(u);
return u->assert_result;
}
@ -2339,6 +2343,10 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag
m = u->manager;
/* Let's enqueue the change signal early. In case this unit has a job associated we want that this unit is in
* the bus queue, so that any job change signal queued will force out the unit change signal first. */
unit_add_to_dbus_queue(u);
/* Update timestamps for state changes */
if (!MANAGER_IS_RELOADING(m)) {
dual_timestamp_get(&u->state_change_timestamp);
@ -2497,7 +2505,6 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag
}
}
unit_add_to_dbus_queue(u);
unit_add_to_gc_queue(u);
}
@ -4930,7 +4937,7 @@ void unit_notify_user_lookup(Unit *u, uid_t uid, gid_t gid) {
r = unit_ref_uid_gid(u, uid, gid);
if (r > 0)
bus_unit_send_change_signal(u);
unit_add_to_dbus_queue(u);
}
int unit_set_invocation_id(Unit *u, sd_id128_t id) {
@ -4984,6 +4991,7 @@ int unit_acquire_invocation_id(Unit *u) {
if (r < 0)
return log_unit_error_errno(u, r, "Failed to set invocation ID for unit: %m");
unit_add_to_dbus_queue(u);
return 0;
}

View file

@ -2799,63 +2799,87 @@ static void wait_context_free(WaitContext *c) {
}
static int on_properties_changed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
const char *path, *interface, *active_state = NULL, *job_path = NULL;
WaitContext *c = userdata;
const char *path;
bool is_failed;
int r;
/* Called whenever we get a PropertiesChanged signal. Checks if ActiveState changed to inactive/failed.
*
* Signal parameters: (s interface, a{sv} changed_properties, as invalidated_properties) */
path = sd_bus_message_get_path(m);
if (!set_contains(c->unit_paths, path))
return 0;
/* Check if ActiveState changed to inactive/failed */
/* (s interface, a{sv} changed_properties, as invalidated_properties) */
r = sd_bus_message_skip(m, "s");
r = sd_bus_message_read(m, "s", &interface);
if (r < 0)
return bus_log_parse_error(r);
if (!streq(interface, "org.freedesktop.systemd1.Unit")) /* ActiveState is on the Unit interface */
return 0;
r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, "{sv}");
if (r < 0)
return bus_log_parse_error(r);
while ((r = sd_bus_message_enter_container(m, SD_BUS_TYPE_DICT_ENTRY, "sv")) > 0) {
for (;;) {
const char *s;
r = sd_bus_message_read(m, "s", &s);
r = sd_bus_message_enter_container(m, SD_BUS_TYPE_DICT_ENTRY, "sv");
if (r < 0)
return bus_log_parse_error(r);
if (r == 0) /* end of array */
break;
r = sd_bus_message_read(m, "s", &s); /* Property name */
if (r < 0)
return bus_log_parse_error(r);
if (streq(s, "ActiveState")) {
bool is_failed;
r = sd_bus_message_enter_container(m, SD_BUS_TYPE_VARIANT, "s");
r = sd_bus_message_read(m, "v", "s", &active_state);
if (r < 0)
return bus_log_parse_error(r);
r = sd_bus_message_read(m, "s", &s);
if (job_path) /* Found everything we need */
break;
} else if (streq(s, "Job")) {
uint32_t job_id;
r = sd_bus_message_read(m, "v", "(uo)", &job_id, &job_path);
if (r < 0)
return bus_log_parse_error(r);
is_failed = streq(s, "failed");
if (streq(s, "inactive") || is_failed) {
log_debug("%s became %s, dropping from --wait tracking", path, s);
free(set_remove(c->unit_paths, path));
c->any_failed = c->any_failed || is_failed;
} else
log_debug("ActiveState on %s changed to %s", path, s);
/* There's still a job pending for this unit, let's ignore this for now, and return right-away. */
if (job_id != 0)
return 0;
if (active_state) /* Found everything we need */
break;
break; /* no need to dissect the rest of the message */
} else {
/* other property */
r = sd_bus_message_skip(m, "v");
r = sd_bus_message_skip(m, "v"); /* Other property */
if (r < 0)
return bus_log_parse_error(r);
}
r = sd_bus_message_exit_container(m);
if (r < 0)
return bus_log_parse_error(r);
}
if (r < 0)
return bus_log_parse_error(r);
/* If this didn't contain the ActiveState property we can't do anything */
if (!active_state)
return 0;
is_failed = streq(active_state, "failed");
if (streq(active_state, "inactive") || is_failed) {
log_debug("%s became %s, dropping from --wait tracking", path, active_state);
free(set_remove(c->unit_paths, path));
c->any_failed = c->any_failed || is_failed;
} else
log_debug("ActiveState on %s changed to %s", path, active_state);
if (set_isempty(c->unit_paths))
sd_event_exit(c->event, EXIT_SUCCESS);