From 7eda208ffec7b2cc7b3177cd72fefbe6b32cf2e0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Oct 2023 09:55:28 +0200 Subject: [PATCH] tree-wide: prefer sending pifds over pids when creating scope units --- src/login/logind-dbus.c | 6 +++--- src/login/logind-dbus.h | 2 +- src/login/logind-session.c | 2 +- src/machine/machine.c | 8 ++++++-- src/nspawn/nspawn-register.c | 28 +++++++++++++++++++++++----- src/nspawn/nspawn-register.h | 2 +- src/nspawn/nspawn.c | 3 ++- src/run/run.c | 7 ++++++- src/shared/bus-unit-util.c | 16 ++++++++++++++++ src/shared/bus-unit-util.h | 3 +++ 10 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 1d3bdac53d..ca1c485a24 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -3946,7 +3946,7 @@ static int strdup_job(sd_bus_message *reply, char **job) { int manager_start_scope( Manager *manager, const char *scope, - pid_t pid, + const PidRef *pidref, const char *slice, const char *description, char **wants, @@ -3961,7 +3961,7 @@ int manager_start_scope( assert(manager); assert(scope); - assert(pid > 1); + assert(pidref_is_set(pidref)); assert(job); r = bus_message_new_method_call(manager->bus, &m, bus_systemd_mgr, "StartTransientUnit"); @@ -4012,7 +4012,7 @@ int manager_start_scope( if (r < 0) return r; - r = sd_bus_message_append(m, "(sv)", "PIDs", "au", 1, pid); + r = bus_append_scope_pidref(m, pidref); if (r < 0) return r; diff --git a/src/login/logind-dbus.h b/src/login/logind-dbus.h index 11eb170d17..c9d59231d4 100644 --- a/src/login/logind-dbus.h +++ b/src/login/logind-dbus.h @@ -24,7 +24,7 @@ int match_reloading(sd_bus_message *message, void *userdata, sd_bus_error *error int manager_send_changed(Manager *manager, const char *property, ...) _sentinel_; -int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, char **wants, char **after, const char *requires_mounts_for, sd_bus_message *more_properties, sd_bus_error *error, char **job); +int manager_start_scope(Manager *manager, const char *scope, const PidRef *pidref, const char *slice, const char *description, char **wants, char **after, const char *requires_mounts_for, sd_bus_message *more_properties, sd_bus_error *error, char **job); int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_stop_unit(Manager *manager, const char *unit, const char *job_mode, sd_bus_error *error, char **job); int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index c957c93a46..91361a8f6f 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -684,7 +684,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er r = manager_start_scope( s->manager, scope, - s->leader.pid, + &s->leader, s->user->slice, description, /* These two have StopWhenUnneeded= set, hence add a dep towards them */ diff --git a/src/machine/machine.c b/src/machine/machine.c index 6f0ef3da4a..ae64b430cf 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -9,6 +9,7 @@ #include "alloc-util.h" #include "bus-error.h" #include "bus-locator.h" +#include "bus-unit-util.h" #include "bus-util.h" #include "env-file.h" #include "errno-util.h" @@ -383,8 +384,11 @@ static int machine_start_scope( if (r < 0) return r; - r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", - "PIDs", "au", 1, machine->leader.pid, + r = bus_append_scope_pidref(m, &machine->leader); + if (r < 0) + return r; + + r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)", "Delegate", "b", 1, "CollectMode", "s", "inactive-or-failed", "AddRef", "b", 1, diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c index 8995e0f7b3..2c9ebda61a 100644 --- a/src/nspawn/nspawn-register.c +++ b/src/nspawn/nspawn-register.c @@ -194,7 +194,6 @@ int register_machine( r = sd_bus_call(bus, m, 0, &error, NULL); } - if (r < 0) return log_error_errno(r, "Failed to register machine: %s", bus_error_message(&error, r)); @@ -226,7 +225,8 @@ int allocate_scope( unsigned n_mounts, int kill_signal, char **properties, - sd_bus_message *properties_message) { + sd_bus_message *properties_message, + bool allow_pidfd) { _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; @@ -260,8 +260,19 @@ int allocate_scope( description = strjoina("Container ", machine_name); - r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)(sv)", - "PIDs", "au", 1, pid, + if (allow_pidfd) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = pidref_set_pid(&pidref, pid); + if (r < 0) + return log_error_errno(r, "Failed to allocate PID reference: %m"); + + r = bus_append_scope_pidref(m, &pidref); + } else + r = sd_bus_message_append(m, "(sv)", "PIDs", "au", 1, pid); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", "Description", "s", description, "Delegate", "b", 1, "CollectMode", "s", "inactive-or-failed", @@ -305,8 +316,15 @@ int allocate_scope( return bus_log_create_error(r); r = sd_bus_call(bus, m, 0, &error, &reply); - if (r < 0) + if (r < 0) { + /* If this failed with a property we couldn't write, this is quite likely because the server + * doesn't support PIDFDs yet, let's try without. */ + if (allow_pidfd && + sd_bus_error_has_names(&error, SD_BUS_ERROR_UNKNOWN_PROPERTY, SD_BUS_ERROR_PROPERTY_READ_ONLY)) + return allocate_scope(bus, machine_name, pid, slice, mounts, n_mounts, kill_signal, properties, properties_message, /* allow_pidfd= */ false); + return log_error_errno(r, "Failed to allocate scope: %s", bus_error_message(&error, r)); + } r = sd_bus_message_read(reply, "o", &object); if (r < 0) diff --git a/src/nspawn/nspawn-register.h b/src/nspawn/nspawn-register.h index 59fdd1bd61..be65d2b230 100644 --- a/src/nspawn/nspawn-register.h +++ b/src/nspawn/nspawn-register.h @@ -10,5 +10,5 @@ int register_machine(sd_bus *bus, const char *machine_name, pid_t pid, const char *directory, sd_id128_t uuid, int local_ifindex, const char *slice, CustomMount *mounts, unsigned n_mounts, int kill_signal, char **properties, sd_bus_message *properties_message, bool keep_unit, const char *service); int unregister_machine(sd_bus *bus, const char *machine_name); -int allocate_scope(sd_bus *bus, const char *machine_name, pid_t pid, const char *slice, CustomMount *mounts, unsigned n_mounts, int kill_signal, char **properties, sd_bus_message *properties_message); +int allocate_scope(sd_bus *bus, const char *machine_name, pid_t pid, const char *slice, CustomMount *mounts, unsigned n_mounts, int kill_signal, char **properties, sd_bus_message *properties_message, bool allow_pidfds); int terminate_scope(sd_bus *bus, const char *machine_name); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 05d04dce13..07234df85b 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5074,7 +5074,8 @@ static int run_container( arg_custom_mounts, arg_n_custom_mounts, arg_kill_signal, arg_property, - arg_property_message); + arg_property_message, + /* allow_pidfds= */ true); if (r < 0) return r; diff --git a/src/run/run.c b/src/run/run.c index f6a2ea53de..77853298e0 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -945,7 +945,12 @@ static int transient_scope_set_properties(sd_bus_message *m) { if (r < 0) return r; - r = sd_bus_message_append(m, "(sv)", "PIDs", "au", 1, (uint32_t) getpid_cached()); + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = pidref_set_self(&pidref); + if (r < 0) + return r; + + r = bus_append_scope_pidref(m, &pidref); if (r < 0) return bus_log_create_error(r); diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index f88a4f5aab..eeeabc66b1 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2817,6 +2817,22 @@ int bus_append_unit_property_assignment_many(sd_bus_message *m, UnitType t, char return 0; } +int bus_append_scope_pidref(sd_bus_message *m, const PidRef *pidref) { + assert(m); + + if (!pidref_is_set(pidref)) + return -ESRCH; + + if (pidref->fd >= 0) + return sd_bus_message_append( + m, "(sv)", + "PIDFDs", "ah", 1, pidref->fd); + + return sd_bus_message_append( + m, "(sv)", + "PIDs", "au", 1, pidref->pid); +} + int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet) { const char *type, *path, *source; InstallChange *changes = NULL; diff --git a/src/shared/bus-unit-util.h b/src/shared/bus-unit-util.h index 267d516cbf..d52c8475ca 100644 --- a/src/shared/bus-unit-util.h +++ b/src/shared/bus-unit-util.h @@ -4,6 +4,7 @@ #include "sd-bus.h" #include "install.h" +#include "pidref.h" #include "unit-def.h" typedef struct UnitInfo { @@ -25,6 +26,8 @@ int bus_parse_unit_info(sd_bus_message *message, UnitInfo *u); int bus_append_unit_property_assignment(sd_bus_message *m, UnitType t, const char *assignment); int bus_append_unit_property_assignment_many(sd_bus_message *m, UnitType t, char **l); +int bus_append_scope_pidref(sd_bus_message *m, const PidRef *pidref); + int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet); int unit_load_state(sd_bus *bus, const char *name, char **load_state); -- 2.25.1