Merge pull request #7442 from poettering/scope-fixes

some fixes to the scope unit type

src/core/dbus-scope.c

@@ -61,7 +61,7 @@ static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, scope_result, ScopeResu
 
 const sd_bus_vtable bus_scope_vtable[] = {
         SD_BUS_VTABLE_START(0),
-        SD_BUS_PROPERTY("Controller", "s", NULL, offsetof(Scope, controller), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("Controller", "s", NULL, offsetof(Scope, controller), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
         SD_BUS_PROPERTY("TimeoutStopUSec", "t", bus_property_get_usec, offsetof(Scope, timeout_stop_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Scope, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
         SD_BUS_SIGNAL("RequestStop", NULL, 0),
@@ -119,6 +119,11 @@ static int bus_scope_set_transient_property(
                 const char *controller;
                 char *c;
 
+                /* We can't support direct connections with this, as direct connections know no service or unique name
+                 * concept, but the Controller field stores exactly that. */
+                if (sd_bus_message_get_bus(message) != UNIT(s)->manager->api_bus)
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Sorry, Controller= logic only supported via the bus.");
+
                 r = sd_bus_message_read(message, "s", &controller);
                 if (r < 0)
                         return r;
@@ -228,3 +233,40 @@ int bus_scope_send_request_stop(Scope *s) {
 
         return sd_bus_send_to(UNIT(s)->manager->api_bus, m, s->controller, NULL);
 }
+
+static int on_controller_gone(sd_bus_track *track, void *userdata) {
+        Scope *s = userdata;
+
+        assert(track);
+
+        if (s->controller) {
+                log_unit_debug(UNIT(s), "Controller %s disappeared from bus.", s->controller);
+                unit_add_to_dbus_queue(UNIT(s));
+                s->controller = mfree(s->controller);
+        }
+
+        s->controller_track = sd_bus_track_unref(s->controller_track);
+
+        return 0;
+}
+
+int bus_scope_track_controller(Scope *s) {
+        int r;
+
+        assert(s);
+
+        if (!s->controller || s->controller_track)
+                return 0;
+
+        r = sd_bus_track_new(UNIT(s)->manager->api_bus, &s->controller_track, on_controller_gone, s);
+        if (r < 0)
+                return r;
+
+        r = sd_bus_track_add_name(s->controller_track, s->controller);
+        if (r < 0) {
+                s->controller_track = sd_bus_track_unref(s->controller_track);
+                return r;
+        }
+
+        return 0;
+}

src/core/dbus-scope.h

@@ -30,3 +30,5 @@ int bus_scope_set_property(Unit *u, const char *name, sd_bus_message *i, UnitSet
 int bus_scope_commit_properties(Unit *u);
 
 int bus_scope_send_request_stop(Scope *s);
+
+int bus_scope_track_controller(Scope *s);

src/core/scope.c

@@ -59,7 +59,8 @@ static void scope_done(Unit *u) {
 
         assert(u);
 
-        free(s->controller);
+        s->controller = mfree(s->controller);
+        s->controller_track = sd_bus_track_unref(s->controller_track);
 
         s->timer_event_source = sd_event_source_unref(s->timer_event_source);
 }
@@ -229,6 +230,8 @@ static int scope_coldplug(Unit *u) {
         if (!IN_SET(s->deserialized_state, SCOPE_DEAD, SCOPE_FAILED))
                 unit_watch_all_pids(UNIT(s));
 
+        bus_scope_track_controller(s);
+
         scope_set_state(s, s->deserialized_state);
         return 0;
 }
@@ -272,9 +275,8 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) {
 
         unit_watch_all_pids(UNIT(s));
 
-        /* If we have a controller set let's ask the controller nicely
-         * to terminate the scope, instead of us going directly into
-         * SIGTERM berserk mode */
+        /* If we have a controller set let's ask the controller nicely to terminate the scope, instead of us going
+         * directly into SIGTERM berserk mode */
         if (state == SCOPE_STOP_SIGTERM)
                 skip_signal = bus_scope_send_request_stop(s) > 0;
 
@@ -332,6 +334,8 @@ static int scope_start(Unit *u) {
         if (!u->transient && !MANAGER_IS_RELOADING(u->manager))
                 return -ENOENT;
 
+        (void) bus_scope_track_controller(s);
+
         r = unit_acquire_invocation_id(u);
         if (r < 0)
                 return r;
@@ -411,11 +415,16 @@ static int scope_serialize(Unit *u, FILE *f, FDSet *fds) {
 
         unit_serialize_item(u, f, "state", scope_state_to_string(s->state));
         unit_serialize_item(u, f, "was-abandoned", yes_no(s->was_abandoned));
+
+        if (s->controller)
+                unit_serialize_item(u, f, "controller", s->controller);
+
         return 0;
 }
 
 static int scope_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) {
         Scope *s = SCOPE(u);
+        int r;
 
         assert(u);
         assert(key);
@@ -439,6 +448,12 @@ static int scope_deserialize_item(Unit *u, const char *key, const char *value, F
                         log_unit_debug(u, "Failed to parse boolean value: %s", value);
                 else
                         s->was_abandoned = k;
+        } else if (streq(key, "controller")) {
+
+                r = free_and_strdup(&s->controller, value);
+                if (r < 0)
+                        log_oom();
+
         } else
                 log_unit_debug(u, "Unknown serialization key: %s", key);
 
@@ -525,7 +540,10 @@ int scope_abandon(Scope *s) {
                 return -ESTALE;
 
         s->was_abandoned = true;
+
         s->controller = mfree(s->controller);
+        s->controller_track = sd_bus_track_unref(s->controller_track);
+
         scope_set_state(s, SCOPE_ABANDONED);
 
         /* The client is no longer watching the remaining processes,

src/core/scope.h

@@ -46,6 +46,8 @@ struct Scope {
         usec_t timeout_stop_usec;
 
         char *controller;
+        sd_bus_track *controller_track;
+
         bool was_abandoned;
 
         sd_event_source *timer_event_source;

src/nspawn/nspawn-register.c

@@ -99,7 +99,26 @@ static int append_machine_properties(
         return 0;
 }
 
+static int append_controller_property(sd_bus *bus, sd_bus_message *m) {
+        const char *unique;
+        int r;
+
+        assert(bus);
+        assert(m);
+
+        r = sd_bus_get_unique_name(bus, &unique);
+        if (r < 0)
+                return log_error_errno(r, "Failed to get unique name: %m");
+
+        r = sd_bus_message_append(m, "(sv)", "Controller", "s", unique);
+        if (r < 0)
+                return bus_log_create_error(r);
+
+        return 0;
+}
+
 int register_machine(
+                sd_bus *bus,
                 const char *machine_name,
                 pid_t pid,
                 const char *directory,
@@ -114,12 +133,9 @@ int register_machine(
                 const char *service) {
 
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
         int r;
 
-        r = sd_bus_default_system(&bus);
-        if (r < 0)
-                return log_error_errno(r, "Failed to open system bus: %m");
+        assert(bus);
 
         if (keep_unit) {
                 r = sd_bus_call_method(
@@ -174,6 +190,10 @@ int register_machine(
                                 return bus_log_create_error(r);
                 }
 
+                r = append_controller_property(bus, m);
+                if (r < 0)
+                        return r;
+
                 r = append_machine_properties(
                                 m,
                                 mounts,
@@ -202,16 +222,13 @@ int register_machine(
         return 0;
 }
 
-int terminate_machine(pid_t pid) {
+int terminate_machine(sd_bus *bus, pid_t pid) {
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
         const char *path;
         int r;
 
-        r = sd_bus_default_system(&bus);
-        if (r < 0)
-                return log_error_errno(r, "Failed to open system bus: %m");
+        assert(bus);
 
         r = sd_bus_call_method(
                         bus,
@@ -253,6 +270,7 @@ int terminate_machine(pid_t pid) {
 }
 
 int allocate_scope(
+                sd_bus *bus,
                 const char *machine_name,
                 pid_t pid,
                 const char *slice,
@@ -263,16 +281,13 @@ int allocate_scope(
 
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
         _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL;
         _cleanup_free_ char *scope = NULL;
         const char *description, *object;
         int r;
 
-        r = sd_bus_default_system(&bus);
-        if (r < 0)
-                return log_error_errno(r, "Failed to open system bus: %m");
+        assert(bus);
 
         r = bus_wait_for_jobs_new(bus, &w);
         if (r < 0)
@@ -311,6 +326,10 @@ int allocate_scope(
         if (r < 0)
                 return bus_log_create_error(r);
 
+        r = append_controller_property(bus, m);
+        if (r < 0)
+                return r;
+
         r = append_machine_properties(
                         m,
                         mounts,

src/nspawn/nspawn-register.h

@@ -26,7 +26,7 @@
 
 #include "nspawn-mount.h"
 
-int register_machine(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, bool keep_unit, const char *service);
-int terminate_machine(pid_t pid);
+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, bool keep_unit, const char *service);
+int terminate_machine(sd_bus *bus, pid_t pid);
 
-int allocate_scope(const char *machine_name, pid_t pid, const char *slice, CustomMount *mounts, unsigned n_mounts, int kill_signal, char **properties);
+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);

src/nspawn/nspawn.c

@@ -2039,18 +2039,27 @@ static int on_orderly_shutdown(sd_event_source *s, const struct signalfd_siginfo
 }
 
 static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *ssi, void *userdata) {
+        pid_t pid;
+
+        assert(s);
+        assert(ssi);
+
+        pid = PTR_TO_PID(userdata);
+
         for (;;) {
                 siginfo_t si = {};
+
                 if (waitid(P_ALL, 0, &si, WNOHANG|WNOWAIT|WEXITED) < 0)
                         return log_error_errno(errno, "Failed to waitid(): %m");
                 if (si.si_pid == 0) /* No pending children. */
                         break;
-                if (si.si_pid == PTR_TO_PID(userdata)) {
+                if (si.si_pid == pid) {
                         /* The main process we care for has exited. Return from
                          * signal handler but leave the zombie. */
                         sd_event_exit(sd_event_source_get_event(s), 0);
                         break;
                 }
+
                 /* Reap all other children. */
                 (void) waitid(P_PID, si.si_pid, &si, WNOHANG|WEXITED);
         }
@@ -2058,6 +2067,24 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *ssi, vo
         return 0;
 }
 
+static int on_request_stop(sd_bus_message *m, void *userdata, sd_bus_error *error) {
+        pid_t pid;
+
+        assert(m);
+
+        pid = PTR_TO_PID(userdata);
+
+        if (arg_kill_signal > 0) {
+                log_info("Container termination requested. Attempting to halt container.");
+                (void) kill(pid, arg_kill_signal);
+        } else {
+                log_info("Container termination requested. Exiting.");
+                sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), 0);
+        }
+
+        return 0;
+}
+
 static int determine_names(void) {
         int r;
 
@@ -3214,6 +3241,7 @@ static int run(int master,
         _cleanup_(sd_event_unrefp) sd_event *event = NULL;
         _cleanup_(pty_forward_freep) PTYForward *forward = NULL;
         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;
+        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
         ContainerStatus container_status = 0;
         char last_char = 0;
         int ifi = 0, r;
@@ -3444,8 +3472,31 @@ static int run(int master,
                         return r;
         }
 
+        if (arg_register || !arg_keep_unit) {
+                r = sd_bus_default_system(&bus);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to open system bus: %m");
+        }
+
+        if (!arg_keep_unit) {
+                /* When a new scope is created for this container, then we'll be registered as its controller, in which
+                 * case PID 1 will send us a friendly RequestStop signal, when it is asked to terminate the
+                 * scope. Let's hook into that, and cleanly shut down the container, and print a friendly message. */
+
+                r = sd_bus_add_match(bus, NULL,
+                                     "type='signal',"
+                                     "sender='org.freedesktop.systemd1',"
+                                     "interface='org.freedesktop.systemd1.Scope',"
+                                     "member='RequestStop'",
+                                     on_request_stop, PID_TO_PTR(*pid));
+                if (r < 0)
+                        return log_error_errno(r, "Failed to install request stop match: %m");
+        }
+
         if (arg_register) {
+
                 r = register_machine(
+                                bus,
                                 arg_machine,
                                 *pid,
                                 arg_directory,
@@ -3459,8 +3510,11 @@ static int run(int master,
                                 arg_container_service_name);
                 if (r < 0)
                         return r;
+
         } else if (!arg_keep_unit) {
+
                 r = allocate_scope(
+                                bus,
                                 arg_machine,
                                 *pid,
                                 arg_slice,
@@ -3506,6 +3560,12 @@ static int run(int master,
         if (r < 0)
                 return log_error_errno(r, "Failed to get default event source: %m");
 
+        if (bus) {
+                r = sd_bus_attach_event(bus, event, 0);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to attach bus to event loop: %m");
+        }
+
         r = setup_sd_notify_parent(event, notify_socket, PID_TO_PTR(*pid), &notify_event_source);
         if (r < 0)
                 return r;
@@ -3567,8 +3627,8 @@ static int run(int master,
                 putc('\n', stdout);
 
         /* Kill if it is not dead yet anyway */
-        if (arg_register && !arg_keep_unit)
-                terminate_machine(*pid);
+        if (arg_register && !arg_keep_unit && bus)
+                terminate_machine(bus, *pid);
 
         /* Normally redundant, but better safe than sorry */
         (void) kill(*pid, SIGKILL);