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

keszybz · web-flow · commit 2d479ff1cc04 · 2018-12-06T16:42:21.000+01:00
force PropertiesChanged bus signal on all unit state changes
diff --git a/TODO b/TODO
@@ -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
diff --git a/src/core/automount.c b/src/core/automount.c
@@ -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;
 
diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
@@ -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);
diff --git a/src/core/dbus-job.h b/src/core/dbus-job.h
@@ -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);
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
@@ -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);
 }
 
diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h
@@ -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);
diff --git a/src/core/device.c b/src/core/device.c
@@ -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;
 
diff --git a/src/core/job.c b/src/core/job.c
@@ -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;
 }
 
diff --git a/src/core/mount.c b/src/core/mount.c
@@ -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;
 
diff --git a/src/core/path.c b/src/core/path.c
@@ -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;
 
diff --git a/src/core/scope.c b/src/core/scope.c
@@ -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;
 
diff --git a/src/core/service.c b/src/core/service.c
@@ -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;
diff --git a/src/core/slice.c b/src/core/slice.c
@@ -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;
 
diff --git a/src/core/socket.c b/src/core/socket.c
@@ -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;
 
diff --git a/src/core/swap.c b/src/core/swap.c
@@ -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;
 
diff --git a/src/core/target.c b/src/core/target.c
@@ -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;
 
diff --git a/src/core/timer.c b/src/core/timer.c
@@ -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;
 
diff --git a/src/core/unit.c b/src/core/unit.c
@@ -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;
 }
 
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c

Original file line number	Diff line number	Diff line change
`@@ -236,6 +236,9 @@ Job* job_install(Job *j) {`
`236`	`236`
`237`	`237`	`job_add_to_gc_queue(j);`
`238`	`238`
	`239`	`+ job_add_to_dbus_queue(j); /* announce this job to clients */`
	`240`	`+ unit_add_to_dbus_queue(j->unit); /* The Job property of the unit has changed now */`
	`241`	`+`
`239`	`242`	`return j;`
`240`	`243`	`}`
`241`	`244`