From c20076a8c170101fb9e6c77e4f7f84c5525dd7cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Oct 2018 22:50:25 +0200 Subject: [PATCH 01/10] pid1: add a new AbandonScope() method call on the Manager object This is the same as Abandon() on the Scope object, but saves clients from first translating a unit name into a unit object path. This logic matches how all the other unit methods have counterparts on the Manager object too (e.g. StopUnit() on the Manager object matching Stop() on the Unit object), this one was simply forgotten so far. --- src/core/dbus-manager.c | 25 +++++++++++++++++++++++++ src/core/dbus-scope.c | 4 ++-- src/core/dbus-scope.h | 2 ++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 677d9c3226..2e38088d0f 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -12,6 +12,7 @@ #include "dbus-execute.h" #include "dbus-job.h" #include "dbus-manager.h" +#include "dbus-scope.h" #include "dbus-unit.h" #include "dbus.h" #include "env-util.h" @@ -2422,6 +2423,29 @@ static int method_get_job_waiting(sd_bus_message *message, void *userdata, sd_bu return bus_job_method_get_waiting_jobs(message, j, error); } +static int method_abandon_scope(sd_bus_message *message, void *userdata, sd_bus_error *error) { + Manager *m = userdata; + const char *name; + Unit *u; + int r; + + assert(message); + assert(m); + + r = sd_bus_message_read(message, "s", &name); + if (r < 0) + return r; + + r = bus_get_unit_by_name(m, message, name, &u, error); + if (r < 0) + return r; + + if (u->type != UNIT_SCOPE) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit '%s' is not a scope unit, refusing.", name); + + return bus_scope_method_abandon(message, u, error); +} + const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_VTABLE_START(0), @@ -2537,6 +2561,7 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_METHOD("StartTransientUnit", "ssa(sv)a(sa(sv))", "o", method_start_transient_unit, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetUnitProcesses", "s", "a(sus)", method_get_unit_processes, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("AttachProcessesToUnit", "ssau", NULL, method_attach_processes_to_unit, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("AbandonScope", "s", NULL, method_abandon_scope, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetJob", "u", "o", method_get_job, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetJobAfter", "u", "a(usssoo)", method_get_job_waiting, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetJobBefore", "u", "a(usssoo)", method_get_job_waiting, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/core/dbus-scope.c b/src/core/dbus-scope.c index 6725f62794..5d9fe98857 100644 --- a/src/core/dbus-scope.c +++ b/src/core/dbus-scope.c @@ -14,7 +14,7 @@ #include "selinux-access.h" #include "unit.h" -static int bus_scope_abandon(sd_bus_message *message, void *userdata, sd_bus_error *error) { +int bus_scope_method_abandon(sd_bus_message *message, void *userdata, sd_bus_error *error) { Scope *s = userdata; int r; @@ -48,7 +48,7 @@ const sd_bus_vtable bus_scope_vtable[] = { 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), - SD_BUS_METHOD("Abandon", NULL, NULL, bus_scope_abandon, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("Abandon", NULL, NULL, bus_scope_method_abandon, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_VTABLE_END }; diff --git a/src/core/dbus-scope.h b/src/core/dbus-scope.h index 7c080dbcf7..702f55898d 100644 --- a/src/core/dbus-scope.h +++ b/src/core/dbus-scope.h @@ -14,4 +14,6 @@ int bus_scope_commit_properties(Unit *u); int bus_scope_send_request_stop(Scope *s); +int bus_scope_method_abandon(sd_bus_message *message, void *userdata, sd_bus_error *error); + int bus_scope_track_controller(Scope *s); From e5c36295d81971ef75d9c6f98f0890b92a4a353f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Oct 2018 23:04:51 +0200 Subject: [PATCH 02/10] unit: enqueue cgroup empty check event if the last ref on a unit is dropped --- src/core/dbus-unit.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 97bceb19dd..7ef9baf905 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1781,7 +1781,13 @@ static int bus_unit_track_handler(sd_bus_track *t, void *userdata) { u->bus_track = sd_bus_track_unref(u->bus_track); /* make sure we aren't called again */ + /* If the client that tracks us disappeared, then there's reason to believe that the cgroup is empty now too, + * let's see */ + unit_add_to_cgroup_empty_queue(u); + + /* Also add the unit to the GC queue, after all if the client left it might be time to GC this unit */ unit_add_to_gc_queue(u); + return 0; } From c4e48030cf299b56668b1cd360c4ec93e7f97b20 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 6 Oct 2018 18:43:28 +0200 Subject: [PATCH 03/10] sd-bus: make "close+flush-on-exit" optional when using sd-event with sd-bus This adds a new pair of API calls sd_bus_set_close_on_exit() and sd_bus_get_close_on_exit(). They control whether an sd_bus object attached to a an sd-event loop shall automatically be flushed/closed when the event loop goes down. Usually that's a good thing, except for very few cases where the bus connection is longer living than the event loop it is attached on. Specifically, this is the case for nspawn, where we run the event loop only while the container is up, but afterwards still want to be able to use the bus connection. --- man/rules/meson.build | 1 + man/sd-bus.xml | 1 + man/sd_bus_set_close_on_exit.xml | 105 +++++++++++++++++++++++++++ src/libsystemd/libsystemd.sym | 3 + src/libsystemd/sd-bus/bus-internal.h | 1 + src/libsystemd/sd-bus/sd-bus.c | 43 ++++++++--- src/systemd/sd-bus.h | 2 + 7 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 man/sd_bus_set_close_on_exit.xml diff --git a/man/rules/meson.build b/man/rules/meson.build index b3011c5f04..33301d4b6d 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -289,6 +289,7 @@ manpages = [ 'sd_bus_release_name_async', 'sd_bus_request_name_async'], ''], + ['sd_bus_set_close_on_exit', '3', ['sd_bus_get_close_on_exit'], ''], ['sd_bus_set_connected_signal', '3', ['sd_bus_get_connected_signal'], ''], ['sd_bus_set_description', '3', diff --git a/man/sd-bus.xml b/man/sd-bus.xml index bf2f37a86d..1df988cf7a 100644 --- a/man/sd-bus.xml +++ b/man/sd-bus.xml @@ -84,6 +84,7 @@ sd_bus_set_description3, sd_bus_set_sender3, sd_bus_set_watch_bind3 +sd_bus_set_close_on_exit3 sd_bus_slot_set_description3, sd_bus_slot_set_destroy_callback3, sd_bus_slot_set_floating3, diff --git a/man/sd_bus_set_close_on_exit.xml b/man/sd_bus_set_close_on_exit.xml new file mode 100644 index 0000000000..dc4f6a3e15 --- /dev/null +++ b/man/sd_bus_set_close_on_exit.xml @@ -0,0 +1,105 @@ + + + + + + + + + sd_bus_set_close_on_exit + systemd + + + + sd_bus_set_close_on_exit + 3 + + + + sd_bus_set_close_on_exit + sd_bus_get_close_on_exit + + Control whether to close the bus connection during the event loop exit phase + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_set_close_on_exit + sd_bus *bus + int b + + + + int sd_bus_get_close_on_exit + sd_bus *bus + + + + + + + Description + + sd_bus_set_close_on_exit() may be used to enable or disable whether the bus connection + is automatically flushed (as in + sd_bus_flush3) and closed (as in + sd_bus_close3) during the exit + phase of the event loop. This logic only applies to bus connections that are attached to an + sd-event3 event loop, see + sd_bus_attach_event3. By default + this mechanism is enabled and makes sure that any pending messages that have not been written to the bus connection + are written out when the event loop is shutting down. In some cases this behaviour is not desirable, for example + when the bus connection shall remain usable until after the event loop exited. If b is + true, the feature is enabled (which is the default), otherwise disabled. + + sd_bus_get_close_on_exit() may be used to query the current setting of this feature. It + returns zero when the feature is disabled, and positive if enabled. + + + + Return Value + + On success, sd_bus_set_close_on_exit() returns 0 or a positive integer. On failure, it returns a negative errno-style + error code. + + sd_bus_get_close_on_exit() returns 0 if the feature is currently turned off or a + positive integer if it is on. On failure, it returns a negative errno-style error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -ECHILD + + The bus connection has been created in a different process. + + + + + + + + See Also + + + systemd1, + sd-bus3, + sd_bus_flush3, + sd_bus_attach_event3, + sd-event3, + sd_event_add_exit3 + + + + diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 1f2238ca37..6a64b929dd 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -579,6 +579,9 @@ global: sd_bus_error_move; + sd_bus_set_close_on_exit; + sd_bus_get_close_on_exit; + sd_device_ref; sd_device_unref; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 55f2bc36fb..4bc945d9ee 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -211,6 +211,7 @@ struct sd_bus { bool accept_fd:1; bool attach_timestamp:1; bool connected_signal:1; + bool close_on_exit:1; int use_memfd; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index bc7d00c3d0..f086d13898 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -232,18 +232,22 @@ _public_ int sd_bus_new(sd_bus **ret) { assert_return(ret, -EINVAL); - b = new0(sd_bus, 1); + b = new(sd_bus, 1); if (!b) return -ENOMEM; - b->n_ref = REFCNT_INIT; - b->input_fd = b->output_fd = -1; - b->inotify_fd = -1; - b->message_version = 1; - b->creds_mask |= SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME; - b->accept_fd = true; - b->original_pid = getpid_cached(); - b->n_groups = (size_t) -1; + *b = (sd_bus) { + .n_ref = REFCNT_INIT, + .input_fd = -1, + .output_fd = -1, + .inotify_fd = -1, + .message_version = 1, + .creds_mask = SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME, + .accept_fd = true, + .original_pid = getpid_cached(), + .n_groups = (size_t) -1, + .close_on_exit = true, + }; assert_se(pthread_mutex_init(&b->memfd_cache_mutex, NULL) == 0); @@ -3409,8 +3413,10 @@ static int quit_callback(sd_event_source *event, void *userdata) { assert(event); - sd_bus_flush(bus); - sd_bus_close(bus); + if (bus->close_on_exit) { + sd_bus_flush(bus); + sd_bus_close(bus); + } return 1; } @@ -4135,3 +4141,18 @@ _public_ int sd_bus_get_method_call_timeout(sd_bus *bus, uint64_t *ret) { *ret = bus->method_call_timeout = BUS_DEFAULT_TIMEOUT; return 0; } + +_public_ int sd_bus_set_close_on_exit(sd_bus *bus, int b) { + assert_return(bus, -EINVAL); + assert_return(bus = bus_resolve(bus), -ENOPKG); + + bus->close_on_exit = b; + return 0; +} + +_public_ int sd_bus_get_close_on_exit(sd_bus *bus) { + assert_return(bus, -EINVAL); + assert_return(bus = bus_resolve(bus), -ENOPKG); + + return bus->close_on_exit; +} diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 9c4bbed9dc..220ddb99ec 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -154,6 +154,8 @@ int sd_bus_set_allow_interactive_authorization(sd_bus *bus, int b); int sd_bus_get_allow_interactive_authorization(sd_bus *bus); int sd_bus_set_exit_on_disconnect(sd_bus *bus, int b); int sd_bus_get_exit_on_disconnect(sd_bus *bus); +int sd_bus_set_close_on_exit(sd_bus *bus, int b); +int sd_bus_get_close_on_exit(sd_bus *bus); int sd_bus_set_watch_bind(sd_bus *bus, int b); int sd_bus_get_watch_bind(sd_bus *bus); int sd_bus_set_connected_signal(sd_bus *bus, int b); From e5a2d8b5b56305bbb6dfc5a148ab1b73925c72db Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 6 Oct 2018 18:45:58 +0200 Subject: [PATCH 04/10] nspawn: make use of the new sd_bus_set_close_on_exit() call in nspawn --- src/nspawn/nspawn.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index ab19d73c27..97d9c7c7b7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3974,6 +3974,10 @@ static int run(int master, r = sd_bus_default_system(&bus); if (r < 0) return log_error_errno(r, "Failed to open system bus: %m"); + + r = sd_bus_set_close_on_exit(bus, false); + if (r < 0) + return log_error_errno(r, "Failed to disable close-on-exit behaviour: %m"); } if (!arg_keep_unit) { From 11d81e506ed68c6c5cebe319dc57a9a2fc4319c5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Oct 2018 22:54:57 +0200 Subject: [PATCH 05/10] nspawn: simplify machine terminate bus call We have the machine name anyway, let's use TerminateMachine() on machined's Manager object directly with it. That way it's a single method call only, instead of two, to terminate the machine. --- src/nspawn/nspawn-register.c | 34 +++++++--------------------------- src/nspawn/nspawn-register.h | 2 +- src/nspawn/nspawn.c | 2 +- 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c index 85f3cf1c01..e459cb63ec 100644 --- a/src/nspawn/nspawn-register.c +++ b/src/nspawn/nspawn-register.c @@ -201,10 +201,11 @@ int register_machine( return 0; } -int terminate_machine(sd_bus *bus, pid_t pid) { +int terminate_machine( + sd_bus *bus, + const char *machine_name) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - const char *path; int r; assert(bus); @@ -214,32 +215,11 @@ int terminate_machine(sd_bus *bus, pid_t pid) { "org.freedesktop.machine1", "/org/freedesktop/machine1", "org.freedesktop.machine1.Manager", - "GetMachineByPID", - &error, - &reply, - "u", - (uint32_t) pid); - if (r < 0) { - /* Note that the machine might already have been - * cleaned up automatically, hence don't consider it a - * failure if we cannot get the machine object. */ - log_debug("Failed to get machine: %s", bus_error_message(&error, r)); - return 0; - } - - r = sd_bus_message_read(reply, "o", &path); - if (r < 0) - return bus_log_parse_error(r); - - r = sd_bus_call_method( - bus, - "org.freedesktop.machine1", - path, - "org.freedesktop.machine1.Machine", - "Terminate", + "TerminateMachine", &error, NULL, - NULL); + "s", + machine_name); if (r < 0) log_debug("Failed to terminate machine: %s", bus_error_message(&error, r)); diff --git a/src/nspawn/nspawn-register.h b/src/nspawn/nspawn-register.h index 30807b9687..ddd8b053a3 100644 --- a/src/nspawn/nspawn-register.h +++ b/src/nspawn/nspawn-register.h @@ -8,6 +8,6 @@ #include "nspawn-mount.h" 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 terminate_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); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 97d9c7c7b7..11e02822d7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4130,7 +4130,7 @@ static int run(int master, /* Kill if it is not dead yet anyway */ if (arg_register && !arg_keep_unit && bus) - terminate_machine(bus, *pid); + terminate_machine(bus, arg_machine); /* Normally redundant, but better safe than sorry */ (void) kill(*pid, SIGKILL); From df61bc5e4aa19f9b211dbe8414343b44361e442c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Oct 2018 22:56:20 +0200 Subject: [PATCH 06/10] nspawn: merge two variable declaration lines --- src/nspawn/nspawn-register.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c index e459cb63ec..0d45cce66e 100644 --- a/src/nspawn/nspawn-register.c +++ b/src/nspawn/nspawn-register.c @@ -236,9 +236,8 @@ int allocate_scope( int kill_signal, char **properties) { + _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; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = 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; From 1d78fea2d6230e0aafa2603abc8f1f51966ef134 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Oct 2018 22:56:40 +0200 Subject: [PATCH 07/10] nspawn: rework how we allocate/kill scopes Fixes: #6347 --- src/nspawn/nspawn-register.c | 64 +++++++++++++++++++++++++++++++++++- src/nspawn/nspawn-register.h | 1 + src/nspawn/nspawn.c | 8 +++-- 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c index 0d45cce66e..a7cdfc1c7d 100644 --- a/src/nspawn/nspawn-register.c +++ b/src/nspawn/nspawn-register.c @@ -274,10 +274,12 @@ int allocate_scope( description = strjoina("Container ", machine_name); - r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)", + r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)(sv)", "PIDs", "au", 1, pid, "Description", "s", description, "Delegate", "b", 1, + "CollectMode", "s", "inactive-or-failed", + "AddRef", "b", 1, "Slice", "s", isempty(slice) ? SPECIAL_MACHINE_SLICE : slice); if (r < 0) return bus_log_create_error(r); @@ -324,3 +326,63 @@ int allocate_scope( return 0; } + +int terminate_scope( + sd_bus *bus, + const char *machine_name) { + + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_free_ char *scope = NULL; + int r; + + r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope); + if (r < 0) + return log_error_errno(r, "Failed to mangle scope name: %m"); + + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "AbandonScope", + &error, + NULL, + "s", + scope); + if (r < 0) { + log_debug_errno(r, "Failed to abandon scope '%s', ignoring: %s", scope, bus_error_message(&error, r)); + sd_bus_error_free(&error); + } + + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "KillUnit", + &error, + NULL, + "ssi", + scope, + "all", + (int32_t) SIGKILL); + if (r < 0) { + log_debug_errno(r, "Failed to SIGKILL scope '%s', ignoring: %s", scope, bus_error_message(&error, r)); + sd_bus_error_free(&error); + } + + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "UnrefUnit", + &error, + NULL, + "s", + scope); + if (r < 0) + log_debug_errno(r, "Failed to drop reference to scope '%s', ignoring: %s", scope, bus_error_message(&error, r)); + + return 0; +} diff --git a/src/nspawn/nspawn-register.h b/src/nspawn/nspawn-register.h index ddd8b053a3..05f5776f23 100644 --- a/src/nspawn/nspawn-register.h +++ b/src/nspawn/nspawn-register.h @@ -11,3 +11,4 @@ int register_machine(sd_bus *bus, const char *machine_name, pid_t pid, const cha int terminate_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); +int terminate_scope(sd_bus *bus, const char *machine_name); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 11e02822d7..9f69ee10a8 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4129,8 +4129,12 @@ static int run(int master, putc('\n', stdout); /* Kill if it is not dead yet anyway */ - if (arg_register && !arg_keep_unit && bus) - terminate_machine(bus, arg_machine); + if (bus) { + if (arg_register) + terminate_machine(bus, arg_machine); + else if (!arg_keep_unit) + terminate_scope(bus, arg_machine); + } /* Normally redundant, but better safe than sorry */ (void) kill(*pid, SIGKILL); From b92d0b4c5adef37e9de8f6cc22a0e27b97fcf3ad Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 7 Oct 2018 14:50:11 +0200 Subject: [PATCH 08/10] machined: rework referencing of machine scopes from machined, too When a machine scope is registered by machined, let's add a reference to it, and change the GC mode so that the unit is cleaned up as soon as machined drops the reference, regardless of the fail state. Fixes: #2809 --- src/machine/machine.c | 15 +++++++++++---- src/machine/machined-dbus.c | 35 ++++++++++++++++++++++++++--------- src/machine/machined.h | 1 + 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 239228b2d2..d5e0d4953f 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -403,7 +403,7 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { static int machine_stop_scope(Machine *m) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *job = NULL; - int r; + int r, q; assert(m); assert(m->class != MACHINE_HOST); @@ -412,10 +412,17 @@ static int machine_stop_scope(Machine *m) { return 0; r = manager_stop_unit(m->manager, m->unit, &error, &job); - if (r < 0) - return log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); + if (r < 0) { + log_error_errno(r, "Failed to stop machine scope: %s", bus_error_message(&error, r)); + sd_bus_error_free(&error); + } else + free_and_replace(m->scope_job, job); - return free_and_replace(m->scope_job, job); + q = manager_unref_unit(m->manager, m->unit, &error); + if (q < 0) + log_warning_errno(q, "Failed to drop reference to machine scope, ignoring: %s", bus_error_message(&error, r)); + + return r; } int machine_stop(Machine *m) { diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 32c0b04283..87e6298c78 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1363,18 +1363,15 @@ int manager_start_scope( return r; } - r = sd_bus_message_append(m, "(sv)", "PIDs", "au", 1, pid); + r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", + "PIDs", "au", 1, pid, + "Delegate", "b", 1, + "CollectMode", "s", "inactive-or-failed", + "AddRef", "b", 1, + "TasksMax", "t", UINT64_C(16384)); if (r < 0) return r; - r = sd_bus_message_append(m, "(sv)", "Delegate", "b", 1); - if (r < 0) - return r; - - r = sd_bus_message_append(m, "(sv)", "TasksMax", "t", UINT64_C(16384)); - if (r < 0) - return bus_log_create_error(r); - if (more_properties) { r = sd_bus_message_copy(m, more_properties, true); if (r < 0) @@ -1411,6 +1408,26 @@ int manager_start_scope( return 1; } +int manager_unref_unit( + Manager *m, + const char *unit, + sd_bus_error *error) { + + assert(m); + assert(unit); + + return sd_bus_call_method( + m->bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "UnrefUnit", + error, + NULL, + "s", + unit); +} + int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; int r; diff --git a/src/machine/machined.h b/src/machine/machined.h index 3197c1aade..ef63f96e97 100644 --- a/src/machine/machined.h +++ b/src/machine/machined.h @@ -50,5 +50,6 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, sd_bus_message *more_properties, sd_bus_error *error, char **job); int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_kill_unit(Manager *manager, const char *unit, int signo, sd_bus_error *error); +int manager_unref_unit(Manager *m, const char *unit, sd_bus_error *error); int manager_unit_is_active(Manager *manager, const char *unit); int manager_job_is_active(Manager *manager, const char *path); From bedea99dce3dd2c69a99a09efd6a0405769e759a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Oct 2018 15:44:22 +0200 Subject: [PATCH 09/10] core: expose bus client names currently reffing a unit as property This is useful for debugging client-side ref counting of units: for each ref taken on a unit the client's sender name is listed. If a client has multiple refs on the same unit it is listed multiple times. --- src/core/dbus-unit.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 7ef9baf905..18e5b8d2df 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -561,6 +561,44 @@ int bus_unit_method_unref(sd_bus_message *message, void *userdata, sd_bus_error return sd_bus_reply_method_return(message, NULL); } +static int property_get_refs( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + Unit *u = userdata; + const char *i; + int r; + + assert(bus); + assert(reply); + + r = sd_bus_message_open_container(reply, 'a', "s"); + if (r < 0) + return r; + + for (i = sd_bus_track_first(u->bus_track); i; i = sd_bus_track_next(u->bus_track)) { + int c, k; + + c = sd_bus_track_count_name(u->bus_track, i); + if (c < 0) + return c; + + /* Add the item multiple times if the ref count for each is above 1 */ + for (k = 0; k < c; k++) { + r = sd_bus_message_append(reply, "s", i); + if (r < 0) + return r; + } + } + + return sd_bus_message_close_container(reply); +} + const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_VTABLE_START(0), @@ -637,6 +675,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Unit, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("InvocationID", "ay", bus_property_get_id128, offsetof(Unit, invocation_id), 0), SD_BUS_PROPERTY("CollectMode", "s", property_get_collect_mode, offsetof(Unit, collect_mode), 0), + SD_BUS_PROPERTY("Refs", "as", property_get_refs, 0, 0), SD_BUS_METHOD("Start", "s", "o", method_start, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("Stop", "s", "o", method_stop, SD_BUS_VTABLE_UNPRIVILEGED), From 7685329311d8e8747403de30c394dd4ce4760de6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Oct 2018 15:46:04 +0200 Subject: [PATCH 10/10] TODO --- TODO | 3 +++ 1 file changed, 3 insertions(+) diff --git a/TODO b/TODO index 1ac6ee4bfe..dba4fb6843 100644 --- a/TODO +++ b/TODO @@ -43,6 +43,9 @@ Features: * support the bind/connect/sendmsg cgroup stuff for sandboxing, and possibly patching around +* maybe implicitly attach monotonic+realtime timestamps to outgoing messages in + log.c and sd-journal-send + * chown() tty a service is attached to after the service goes down * optionally: turn on cgroup delegation for per-session scope units