From da81a108653e2ef19102698dbc0184bd18b084d9 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Thu, 10 Oct 2024 21:16:05 +0200
Subject: [PATCH 1/4] core/manager: still send out STATUS=Ready for user
manager
This effectively reverts 37d15cd132f3a8a0bf42fb252c1604e804171ff2.
The offending commit wrongly assumed that the second READY=1
notification is for system scope only, but it also serves the purpose
of flushing out previous STATUS= containing user unit job status.
---
src/core/manager.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/core/manager.c b/src/core/manager.c
index 2789f0e3d0c9c..456ad46135b72 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3885,7 +3885,7 @@ static void manager_notify_finished(Manager *m) {
log_taint_string(m);
}
-static void manager_send_ready_user_scope(Manager *m) {
+static void manager_send_ready_on_basic_target(Manager *m) {
int r;
assert(m);
@@ -3904,18 +3904,18 @@ static void manager_send_ready_user_scope(Manager *m) {
m->status_ready = false;
}
-static void manager_send_ready_system_scope(Manager *m) {
+static void manager_send_ready_on_idle(Manager *m) {
int r;
assert(m);
- if (!MANAGER_IS_SYSTEM(m))
- return;
-
/* Skip the notification if nothing changed. */
if (m->ready_sent && m->status_ready)
return;
+ /* Note that for user managers, we might have already sent READY=1 in manager_send_ready_user_scope().
+ * But we still need to flush STATUS=. The second READY=1 will be treated as a noop so it doesn't
+ * hurt to send it twice. */
r = sd_notify(/* unset_environment= */ false,
"READY=1\n"
"STATUS=Ready.");
@@ -3940,7 +3940,7 @@ static void manager_check_basic_target(Manager *m) {
return;
/* For user managers, send out READY=1 as soon as we reach basic.target */
- manager_send_ready_user_scope(m);
+ manager_send_ready_on_basic_target(m);
/* Log the taint string as soon as we reach basic.target */
log_taint_string(m);
@@ -3971,7 +3971,7 @@ void manager_check_finished(Manager *m) {
if (hashmap_buckets(m->jobs) > hashmap_size(m->units) / 10)
m->jobs = hashmap_free(m->jobs);
- manager_send_ready_system_scope(m);
+ manager_send_ready_on_idle(m);
/* Notify Type=idle units that we are done now */
manager_close_idle_pipe(m);
From 155098a702c4f6de6b1dca534661492625773fed Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Thu, 10 Oct 2024 21:06:35 +0200
Subject: [PATCH 2/4] core/manager-serialize: drop serialization for
Manager.ready_sent
This field indicates whether READY=1 has been sent to
the service manager/supervisor. Whenever we reload/reexec/soft-reboot,
manager_send_reloading() always resets it to false first,
so that READY=1 is sent after reloading finishes. Hence
we utterly get "false" at all times. Kill it.
---
src/core/manager-serialize.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c
index 62dfce93a0a85..3f624619dfd19 100644
--- a/src/core/manager-serialize.c
+++ b/src/core/manager-serialize.c
@@ -92,7 +92,6 @@ int manager_serialize(
(void) serialize_item_format(f, "current-job-id", "%" PRIu32, m->current_job_id);
(void) serialize_item_format(f, "n-installed-jobs", "%u", m->n_installed_jobs);
(void) serialize_item_format(f, "n-failed-jobs", "%u", m->n_failed_jobs);
- (void) serialize_bool(f, "ready-sent", m->ready_sent);
(void) serialize_bool(f, "taint-logged", m->taint_logged);
(void) serialize_bool(f, "service-watchdogs", m->service_watchdogs);
@@ -356,15 +355,6 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
else
m->n_failed_jobs += n;
- } else if ((val = startswith(l, "ready-sent="))) {
- int b;
-
- b = parse_boolean(val);
- if (b < 0)
- log_notice("Failed to parse ready-sent flag '%s', ignoring.", val);
- else
- m->ready_sent = m->ready_sent || b;
-
} else if ((val = startswith(l, "taint-logged="))) {
int b;
@@ -558,7 +548,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
if (q < _MANAGER_TIMESTAMP_MAX) /* found it */
(void) deserialize_dual_timestamp(val, m->timestamps + q);
- else if (!STARTSWITH_SET(l, "kdbus-fd=", "honor-device-enumeration=")) /* ignore deprecated values */
+ else if (!STARTSWITH_SET(l, "kdbus-fd=", "honor-device-enumeration=", "ready-sent=")) /* ignore deprecated values */
log_notice("Unknown serialization item '%s', ignoring.", l);
}
}
From a375e145190482e8a2f0971bffb332e31211622f Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Thu, 10 Oct 2024 21:32:17 +0200
Subject: [PATCH 3/4] units/{user,capsule}@.service: issue daemon-reexec when
notify-reloading
Closes #28367 (but not really in the exact form, see below)
We have the problem of restarting all user manager instances
after upgrade. Current approaches involve systemctl kill
with SIGRTMIN+25, which is async and feels rather ugly [1][2];
or systemctl --machine=user@ --user, which requires entering
each user session. Neither is particularly elegant.
Instead, let's just signal daemon-reexec when user@.service
is reloaded from system manager. Our long goal of dropping
daemon-reload in favor of reexec (see TODO) is unlikely to happen
due to user dbus restrictions, but here the synchronization
is done via READY=1.
[1] https://gitlab.archlinux.org/archlinux/packaging/packages/systemd/-/blob/main/systemd.install?ref_type=heads#L37
[2] https://salsa.debian.org/systemd-team/systemd/-/blob/debian/master/debian/systemd.postinst#L24
#28367 would not really work for us now I come to think about it,
because all processes will be reparented to pid1 as soon as
original user manager process exits. This alternative approach
seems good enough for our use case.
---
units/capsule@.service.in | 4 ++++
units/user@.service.in | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/units/capsule@.service.in b/units/capsule@.service.in
index f2bb9e3a45a83..a64298786e490 100644
--- a/units/capsule@.service.in
+++ b/units/capsule@.service.in
@@ -23,6 +23,10 @@ StateDirectory=capsules/%i
RuntimeDirectory=capsules/%i
LogExtraFields=CAPSULE=%i
Slice=capsule.slice
+# Reexecute the manager on service reload, instead of reloading.
+# This provides a synchronous method for restarting all user manager
+# instances after upgrade.
+ReloadSignal=RTMIN+25
KillMode=mixed
Delegate=pids memory cpu
DelegateSubgroup=init.scope
diff --git a/units/user@.service.in b/units/user@.service.in
index 5695465747217..381ab2a0db54e 100644
--- a/units/user@.service.in
+++ b/units/user@.service.in
@@ -20,6 +20,10 @@ PAMName=systemd-user
Type=notify-reload
ExecStart={{LIBEXECDIR}}/systemd --user
Slice=user-%i.slice
+# Reexecute the manager on service reload, instead of reloading.
+# This provides a synchronous method for restarting all user manager
+# instances after upgrade.
+ReloadSignal=RTMIN+25
KillMode=mixed
Delegate=pids memory cpu
DelegateSubgroup=init.scope
From 2d0af8bc354f4a1429cebedfb387af72c88720a0 Mon Sep 17 00:00:00 2001
From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Thu, 10 Oct 2024 22:37:39 +0200
Subject: [PATCH 4/4] rpm/systemd-update-helper: Use systemctl reload to
reexec/reload user managers
Let's always use systemctl reload to reexec and reload user managers
now that it always implies a reexec. This moves all the job management
logic to pid 1 instead of bash and reduces the complexity of the logic
as we remove systemd-run, pam and systemd-stdio-bridge from the equation.
---
src/rpm/systemd-update-helper.in | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/rpm/systemd-update-helper.in b/src/rpm/systemd-update-helper.in
index c81e16c3d3ffb..8af914935261a 100755
--- a/src/rpm/systemd-update-helper.in
+++ b/src/rpm/systemd-update-helper.in
@@ -107,25 +107,13 @@ case "$command" in
[ -d /run/systemd/system ] || exit 0
- users=$(systemctl list-units 'user@*' --legend=no | sed -n -r 's/.*user@([0-9]+).service.*/\1/p')
-
- if [[ "$command" =~ reexec ]]; then
- for user in $users; do
- SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \
- systemctl --user -M "$user@" daemon-reexec &
- done
- wait
- fi
-
- if [[ "$command" =~ reload ]]; then
- for user in $users; do
- SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \
- systemctl --user -M "$user@" daemon-reload &
- done
- wait
+ if [[ "$command" =~ reexec|reload ]]; then
+ SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s systemctl reload "user@*.service"
fi
if [[ "$command" =~ restart ]]; then
+ users=$(systemctl list-units 'user@*' --legend=no | sed -n -r 's/.*user@([0-9]+).service.*/\1/p')
+
for user in $users; do
SYSTEMD_BUS_TIMEOUT={{UPDATE_HELPER_USER_TIMEOUT_SEC}}s \
systemctl --user -M "$user@" reload-or-restart --marked &