Merge pull request #29455 from poettering/scope-pidref

pid1: allow creating scope units based on pidfds (as opposed to numeric pids)

TODO

@@ -214,7 +214,6 @@ Features:
   - exec_spawn()
   - cg_pid_get_unit()
   - openpt_allocate_in_namespace()
-  - scope dbus PIDs property needs to gain PIDFDs companion
   - sd_bus_creds
   - unit_attach_pid_to_cgroup_via_bus()
   - cg_attach() – requires new kernel feature

src/basic/pidref.h

@@ -39,6 +39,10 @@ int pidref_set_pidfd(PidRef *pidref, int fd);
 int pidref_set_pidfd_take(PidRef *pidref, int fd); /* takes ownership of the passed pidfd on success*/
 int pidref_set_pidfd_consume(PidRef *pidref, int fd); /* takes ownership of the passed pidfd in both success and failure */
 
+static inline int pidref_set_self(PidRef *pidref) {
+        return pidref_set_pid(pidref, 0);
+}
+
 void pidref_done(PidRef *pidref);
 PidRef *pidref_free(PidRef *pidref);
 DEFINE_TRIVIAL_CLEANUP_FUNC(PidRef*, pidref_free);

src/core/dbus-scope.c

@@ -136,12 +136,51 @@ static int bus_scope_set_transient_property(
                 if (r < 0)
                         return r;
 
-                if (n <= 0)
-                        return -EINVAL;
+                return n <= 0 ? -EINVAL : 1;
+        }
 
-                return 1;
+        if (streq(name, "PIDFDs")) {
+                unsigned n = 0;
 
-        } else if (streq(name, "Controller")) {
+                r = sd_bus_message_enter_container(message, 'a', "h");
+                if (r < 0)
+                        return r;
+
+                for (;;) {
+                        _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
+                        int fd;
+
+                        r = sd_bus_message_read(message, "h", &fd);
+                        if (r < 0)
+                                return r;
+                        if (r == 0)
+                                break;
+
+                        r = pidref_set_pidfd(&pidref, fd);
+                        if (r < 0)
+                                return r;
+
+                        r = unit_pid_attachable(u, &pidref, error);
+                        if (r < 0)
+                                return r;
+
+                        if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
+                                r = unit_watch_pidref(u, &pidref, /* exclusive= */ false);
+                                if (r < 0 && r != -EEXIST)
+                                        return r;
+                        }
+
+                        n++;
+                }
+
+                r = sd_bus_message_exit_container(message);
+                if (r < 0)
+                        return r;
+
+                return n <= 0 ? -EINVAL : 1;
+        }
+
+        if (streq(name, "Controller")) {
                 const char *controller;
 
                 /* We can't support direct connections with this, as direct connections know no service or unique name

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;
 

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);

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 */

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,

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)

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);

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;
 

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);
 

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;

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);