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 &