]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
Merge pull request #10963 from poettering/bus-force-state-change-signal
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Thu, 6 Dec 2018 15:42:21 +0000 (16:42 +0100)
committerGitHub <noreply@github.com>
Thu, 6 Dec 2018 15:42:21 +0000 (16:42 +0100)
force PropertiesChanged bus signal on all unit state changes

19 files changed:
TODO
src/core/automount.c
src/core/dbus-job.c
src/core/dbus-job.h
src/core/dbus-unit.c
src/core/dbus-unit.h
src/core/device.c
src/core/job.c
src/core/mount.c
src/core/path.c
src/core/scope.c
src/core/service.c
src/core/slice.c
src/core/socket.c
src/core/swap.c
src/core/target.c
src/core/timer.c
src/core/unit.c
src/systemctl/systemctl.c

diff --git a/TODO b/TODO
index cafd75a01daa5c3952b7ad0b183062d9be530c4c..41ad24d3bbd20fd493c3833542eb74708d941e0f 100644 (file)
--- 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
index 3d8348e0b7fc0229b90a66ccae87b1b9d5eb4c85..de8010bf2e5d86865cb7fff98033423713959e77 100644 (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;
 
index 20d890b36c37685201a28e3ba5e680ade9094101..d11e58b51ddd5cde5a0a23ed8f6842416984634f 100644 (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);
index 3cc60f22ee069199bb545b6b8767c477389929f0..c9f6fc718719861cd6e8fae308843fcb17fc1003 100644 (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);
index 6d9b559d2c7d522ff63f561edb41b66f9fc007b6..968166ee60409dca2970490cbfa2bbfd1c85af3e 100644 (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);
 }
 
index 68eb621836b1a77d79e8f46e4d0e2c4f1c44aceb..345345e3ebadea254c0ab0bd1697b40b15fea79c 100644 (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);
index 8b6126c4cfe8e95e9b0e4fb06421d6fc8e177e17..5acd9b7a7036de7652f2255de69e92f6f3eb4109 100644 (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;
 
index 40be7213ebd56e5647403c66b3751d62b7da3117..af5070b8cf9acb3c3802b4fe0bdf0ef5e4be9ab5 100644 (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;
 }
 
index 99b2aa0904c39a78782b498802ca143cb9f441f3..afdbaa1d9d0389c6f4b299f42e897c8e612eb72c 100644 (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;
 
index 01019b0cf77f9e7027ab4d4e9ba87baa45598ed9..831e49df29f2623b0f9e0a49ffaeae66fcf198a8 100644 (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;
 
index 151b8989a6416bb38bd7aada60a2ffdb6874636c..e478661f9486fb52849da49f4ebe745b27e3ce16 100644 (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;
 
index 964a7fd05725d62e057c5c4a1e292d9bb5992be1..76f1e160697f7d22f53c4554c9d4645f67368505 100644 (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;
index dc087680e1931dcc74feb701c9b55d7bd9b05e93..15b18bcad3562fb9876a80db1a452e73a1eca0ff 100644 (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;
 
index 6697f05fbfd9daa546afd85b34d83d13ea5fb522..dd126a7f21b7d06fe27f2ea3c9fde6452c63ef17 100644 (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;
 
index db806fe0bb3bc8a2928f7bf1872ab8ce35273adb..90207a48fa6c76f52d506346341b6e495ceabdd4 100644 (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;
 
index b8b8e32805eb0988b4a90150457c93dc2063abe6..421a304c73da421b14a962ab2cb35e962e23a248 100644 (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;
 
index 1527aab158279b9ce18c9e04658e261e09df15b2..d9ba2f76b3d3984d6745824ee74cc4abdea9185c 100644 (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;
 
index 122b399d66805c4123268a83ca4918dd0f1ef75b..e1b6e9f11cc67a8f0ecc84672d575a811709caae 100644 (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;
 }
 
index 8f8e9ace1e8b8950b66e687911295de9ff4455ab..5bc2cb9fb7310e3ead7e142041a6703ece12ae03 100644 (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);