From 7693661a8406e004dc20b696850c9acb8c024566 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 12 Oct 2023 12:03:03 +0200 Subject: [PATCH 1/3] run: output invocation ID when starting service and scope units --- src/run/run.c | 109 +++++++++++++++++++++++-------------- test/units/testsuite-65.sh | 4 +- 2 files changed, 70 insertions(+), 43 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 6f110117ab..58974370ef 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1224,6 +1224,50 @@ static int bus_call_with_hint( return r; } +static int acquire_invocation_id(sd_bus *bus, const char *unit, sd_id128_t *ret) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_free_ char *object = NULL; + const void *p; + size_t l; + int r; + + assert(bus); + assert(ret); + + if (unit) { + object = unit_dbus_path_from_name(unit); + if (!object) + return log_oom(); + } + + r = sd_bus_get_property(bus, + "org.freedesktop.systemd1", + object ?: "/org/freedesktop/systemd1/unit/self", + "org.freedesktop.systemd1.Unit", + "InvocationID", + &error, + &reply, + "ay"); + if (r < 0) + return log_error_errno(r, "Failed to request invocation ID for unit: %s", bus_error_message(&error, r)); + + r = sd_bus_message_read_array(reply, 'y', &p, &l); + if (r < 0) + return bus_log_parse_error(r); + + if (l == 0) { + *ret = SD_ID128_NULL; + return 0; /* no uuid set */ + } + + if (l != sizeof(sd_id128_t)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid UUID size, %zu != %zu.", l, sizeof(sd_id128_t)); + + memcpy(ret, p, l); + return !sd_id128_is_null(*ret); +} + static int start_transient_service(sd_bus *bus) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -1325,8 +1369,17 @@ static int start_transient_service(sd_bus *bus) { return r; } - if (!arg_quiet) - log_info("Running as unit: %s", service); + if (!arg_quiet) { + sd_id128_t invocation_id; + + r = acquire_invocation_id(bus, service, &invocation_id); + if (r < 0) + return r; + if (r == 0) /* No invocation UUID set */ + log_info("Running as unit: %s", service); + else + log_info("Running as unit: %s; invocation ID: " SD_ID128_FORMAT_STR, service, SD_ID128_FORMAT_VAL(invocation_id)); + } if (arg_wait || arg_stdio != ARG_STDIO_NONE) { _cleanup_(run_context_free) RunContext c = { @@ -1453,38 +1506,6 @@ static int start_transient_service(sd_bus *bus) { return EXIT_SUCCESS; } -static int acquire_invocation_id(sd_bus *bus, sd_id128_t *ret) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - const void *p; - size_t l; - int r; - - assert(bus); - assert(ret); - - r = sd_bus_get_property(bus, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1/unit/self", - "org.freedesktop.systemd1.Unit", - "InvocationID", - &error, - &reply, - "ay"); - if (r < 0) - return log_error_errno(r, "Failed to request invocation ID for scope: %s", bus_error_message(&error, r)); - - r = sd_bus_message_read_array(reply, 'y', &p, &l); - if (r < 0) - return bus_log_parse_error(r); - - if (l != sizeof(sd_id128_t)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid UUID size, %zu != %zu.", l, sizeof(sd_id128_t)); - - memcpy(ret, p, l); - return 0; -} - static int start_transient_scope(sd_bus *bus) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; @@ -1573,13 +1594,15 @@ static int start_transient_scope(sd_bus *bus) { if (r < 0) return r; - r = acquire_invocation_id(bus, &invocation_id); + r = acquire_invocation_id(bus, NULL, &invocation_id); if (r < 0) return r; - - r = strv_extendf(&user_env, "INVOCATION_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(invocation_id)); - if (r < 0) - return log_oom(); + if (r == 0) + log_debug("No invocation ID set."); + else { + if (strv_extendf(&user_env, "INVOCATION_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(invocation_id)) < 0) + return log_oom(); + } if (arg_nice_set) { if (setpriority(PRIO_PROCESS, 0, arg_nice) < 0) @@ -1642,8 +1665,12 @@ static int start_transient_scope(sd_bus *bus) { if (!env) return log_oom(); - if (!arg_quiet) - log_info("Running scope as unit: %s", scope); + if (!arg_quiet) { + if (sd_id128_is_null(invocation_id)) + log_info("Running as unit: %s", scope); + else + log_info("Running as unit: %s; invocation ID: " SD_ID128_FORMAT_STR, scope, SD_ID128_FORMAT_VAL(invocation_id)); + } if (arg_expand_environment > 0) { _cleanup_strv_free_ char **expanded_cmdline = NULL, **unset_variables = NULL, **bad_variables = NULL; diff --git a/test/units/testsuite-65.sh b/test/units/testsuite-65.sh index 66db42d759..6202b38660 100755 --- a/test/units/testsuite-65.sh +++ b/test/units/testsuite-65.sh @@ -827,13 +827,13 @@ check deny yes /run/systemd/system/deny-list.service check deny no deny-list.service output=$(systemd-run -p "SystemCallFilter=@system-service" -p "SystemCallFilter=~@resources:ENOANO @privileged" -p "SystemCallFilter=@clock" sleep 60 2>&1) -name=$(echo "$output" | awk '{ print $4 }') +name=$(echo "$output" | awk '{ print $4 }' | cut -d';' -f1) check allow yes /run/systemd/transient/"$name" check allow no "$name" output=$(systemd-run -p "SystemCallFilter=~@known" -p "SystemCallFilter=@system-service" -p "SystemCallFilter=~@resources:ENOANO @privileged" -p "SystemCallFilter=@clock" sleep 60 2>&1) -name=$(echo "$output" | awk '{ print $4 }') +name=$(echo "$output" | awk '{ print $4 }' | cut -d';' -f1) check deny yes /run/systemd/transient/"$name" check deny no "$name" From 49a510eba29c78f4b7dc1c39391314a48eb8833b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 16 Oct 2023 11:31:38 +0200 Subject: [PATCH 2/3] run: pin the unit we invoke continously while we are running We read properties of the unit, hence it shouldn't be GC'ed as long as we run. Hence, let's just set AddRef unconditionally for the units we create. --- src/run/run.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 58974370ef..edafd49567 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -653,6 +653,8 @@ static int parse_argv(int argc, char *argv[]) { static int transient_unit_set_properties(sd_bus_message *m, UnitType t, char **properties) { int r; + assert(m); + r = sd_bus_message_append(m, "(sv)", "Description", "s", arg_description); if (r < 0) return bus_log_create_error(r); @@ -663,6 +665,17 @@ static int transient_unit_set_properties(sd_bus_message *m, UnitType t, char **p return bus_log_create_error(r); } + r = sd_bus_is_bus_client(sd_bus_message_get_bus(m)); + if (r < 0) + return log_error_errno(r, "Can't determine if bus connection is direct or to broker: %m"); + if (r > 0) { + /* Pin the object as least as long as we are around. Note that AddRef (currently) only works + * if we talk via the bus though. */ + r = sd_bus_message_append(m, "(sv)", "AddRef", "b", 1); + if (r < 0) + return bus_log_create_error(r); + } + return bus_append_unit_property_assignment_many(m, t, properties); } @@ -754,12 +767,6 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p if (r < 0) return r; - if (arg_wait || arg_stdio != ARG_STDIO_NONE) { - r = sd_bus_message_append(m, "(sv)", "AddRef", "b", 1); - if (r < 0) - return bus_log_create_error(r); - } - if (arg_remain_after_exit) { r = sd_bus_message_append(m, "(sv)", "RemainAfterExit", "b", arg_remain_after_exit); if (r < 0) From 4d4c4ae9cbf540a0c75a1e1f9ebd37f37f30c898 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 16 Oct 2023 13:48:40 +0200 Subject: [PATCH 3/3] update TODO --- TODO | 2 -- 1 file changed, 2 deletions(-) diff --git a/TODO b/TODO index e72240a3d0..c115759229 100644 --- a/TODO +++ b/TODO @@ -1581,8 +1581,6 @@ Features: /etc/resolv.conf. Should be smart and do something useful on read-only images, for example fall back to read-only bind mounting the file instead. -* show invocation ID in systemd-run output - * bypass SIGTERM state in unit files if KillSignal is SIGKILL * add proper dbus APIs for the various sd_notify() commands, such as MAINPID=1