diff --git a/TODO b/TODO index cafd75a01d..41ad24d3bb 100644 --- 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 index 3d8348e0b7..de8010bf2e 100644 --- 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 index 20d890b36c..d11e58b51d 100644 --- 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 index 3cc60f22ee..c9f6fc7187 100644 --- 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 index 6d9b559d2c..968166ee60 100644 --- 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 index 68eb621836..345345e3eb 100644 --- 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 index 8b6126c4cf..5acd9b7a70 100644 --- 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 index 40be7213eb..af5070b8cf 100644 --- 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 index 99b2aa0904..afdbaa1d9d 100644 --- 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 index 01019b0cf7..831e49df29 100644 --- 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 index 151b8989a6..e478661f94 100644 --- 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 index 964a7fd057..76f1e16069 100644 --- 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 index dc087680e1..15b18bcad3 100644 --- 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 index 6697f05fbf..dd126a7f21 100644 --- 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 index db806fe0bb..90207a48fa 100644 --- 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 index b8b8e32805..421a304c73 100644 --- 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 index 1527aab158..d9ba2f76b3 100644 --- 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 index 122b399d66..e1b6e9f11c 100644 --- 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 index 8f8e9ace1e..5bc2cb9fb7 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -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);