From 8eb668b9ab2f7627a89c95ffd61350ee9d416da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 25 May 2023 12:26:37 +0200 Subject: [PATCH 1/3] firstboot: synchronously wait for systemd-vconsole-setup.service/restart job Requested in https://github.com/systemd/systemd/pull/27755#pullrequestreview-1443489520. I dropped the info message about the job being requested, because we get fairly verbose logs from starting the unit, and the additional message isn't useful. In the unit, the ordering before systemd-vconsole-setup.service is dropped, because now it needs to happen in parallel, while systemd-firstboot.service is running. This means that we may potentially execute vconsole-setup twice, but it's fairly quick, so this doesn't matter much. --- src/firstboot/firstboot.c | 20 ++++++++++++++++++-- units/systemd-firstboot.service | 4 ++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index c423aff5dd..65859eaa9f 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -13,6 +13,7 @@ #include "bus-error.h" #include "bus-locator.h" #include "bus-util.h" +#include "bus-wait-for-jobs.h" #include "chase.h" #include "copy.h" #include "creds-util.h" @@ -1588,6 +1589,9 @@ static int reload_system_manager(sd_bus **bus) { static int reload_vconsole(sd_bus **bus) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; + const char *object; int r; assert(bus); @@ -1598,10 +1602,22 @@ static int reload_vconsole(sd_bus **bus) { return bus_log_connect_error(r, BUS_TRANSPORT_LOCAL); } - r = bus_call_method(*bus, bus_systemd_mgr, "RestartUnit", &error, NULL, "ss", "systemd-vconsole-setup.service", "replace"); + r = bus_wait_for_jobs_new(*bus, &w); + if (r < 0) + return log_error_errno(r, "Could not watch jobs: %m"); + + r = bus_call_method(*bus, bus_systemd_mgr, "RestartUnit", &error, &reply, + "ss", "systemd-vconsole-setup.service", "replace"); if (r < 0) return log_error_errno(r, "Failed to issue method call: %s", bus_error_message(&error, r)); - log_info("Requested vconsole setup to apply key map configuration."); + + r = sd_bus_message_read(reply, "o", &object); + if (r < 0) + return bus_log_parse_error(r); + + r = bus_wait_for_jobs_one(w, object, false, NULL); + if (r < 0) + return log_error_errno(r, "Failed to wait for systemd-vconsole-setup.service/restart: %m"); return 0; } diff --git a/units/systemd-firstboot.service b/units/systemd-firstboot.service index 5fee85a287..234e4e7572 100644 --- a/units/systemd-firstboot.service +++ b/units/systemd-firstboot.service @@ -15,9 +15,9 @@ ConditionPathIsReadWrite=/etc ConditionFirstBoot=yes DefaultDependencies=no -After=systemd-remount-fs.service -Before=systemd-vconsole-setup.service sysinit.target first-boot-complete.target +After=systemd-remount-fs.service systemd-sysusers.service Wants=first-boot-complete.target +Before=first-boot-complete.target sysinit.target Conflicts=shutdown.target Before=shutdown.target From b2ce20aa0cd632978fc7e1971fe83757eb8e7f4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 25 May 2023 13:36:17 +0200 Subject: [PATCH 2/3] units: order systemd-firstboot after systemd-tmpfiles-setup We may copy files from factory to /etc. The default mkosi config has factory/etc/vconsole.conf. systemd-firstboot would race with tmpfiles-setup, and sometimes ask for the keymap, and sometimes not. I guess that if there are files in factory, we shouldn't ask the user for the same configuration. --- units/systemd-firstboot.service | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/units/systemd-firstboot.service b/units/systemd-firstboot.service index 234e4e7572..984ebf282a 100644 --- a/units/systemd-firstboot.service +++ b/units/systemd-firstboot.service @@ -15,7 +15,7 @@ ConditionPathIsReadWrite=/etc ConditionFirstBoot=yes DefaultDependencies=no -After=systemd-remount-fs.service systemd-sysusers.service +After=systemd-remount-fs.service systemd-sysusers.service systemd-tmpfiles-setup.service Wants=first-boot-complete.target Before=first-boot-complete.target sysinit.target Conflicts=shutdown.target From f3f8bd6a38eeac3abd703ecc77ccc00609875b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 26 May 2023 15:09:03 +0200 Subject: [PATCH 3/3] various: fix error message for bus_wait_for_jobs_new() As pointed out by Mike Yuan in review of grandparent commit, bus_wait_for_jobs_new() can also fail for non-oom reasons. --- src/run/run.c | 4 ++-- src/shared/tests.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 5124436a05..7dcd37cedd 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1483,7 +1483,7 @@ static int start_transient_scope(sd_bus *bus) { r = bus_wait_for_jobs_new(bus, &w); if (r < 0) - return log_oom(); + return log_error_errno(r, "Could not watch jobs: %m"); if (arg_unit) { r = unit_name_mangle_with_suffix(arg_unit, "as unit", @@ -1725,7 +1725,7 @@ static int start_transient_trigger(sd_bus *bus, const char *suffix) { r = bus_wait_for_jobs_new(bus, &w); if (r < 0) - return log_oom(); + return log_error_errno(r, "Could not watch jobs: %m"); if (arg_unit) { switch (unit_name_to_type(arg_unit)) { diff --git a/src/shared/tests.c b/src/shared/tests.c index 8884c8e90e..ce72be1776 100644 --- a/src/shared/tests.c +++ b/src/shared/tests.c @@ -213,7 +213,7 @@ static int allocate_scope(void) { r = bus_wait_for_jobs_new(bus, &w); if (r < 0) - return log_oom(); + return log_error_errno(r, "Could not watch jobs: %m"); if (asprintf(&scope, "%s-%" PRIx64 ".scope", program_invocation_short_name, random_u64()) < 0) return log_oom();