Merge pull request #1111 from poettering/more-cgroup-fixes

More cgroup fixes

8 files changed, 101 insertions, 48 deletions

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index c26807ba2b..e92d2cc850 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1005,6 +1005,7 @@ Unit* manager_get_unit_by_cgroup(Manager *m, const char *cgroup) {
 
 Unit *manager_get_unit_by_pid(Manager *m, pid_t pid) {
         _cleanup_free_ char *cgroup = NULL;
+        Unit *u;
         int r;
 
         assert(m);
@@ -1012,6 +1013,14 @@ Unit *manager_get_unit_by_pid(Manager *m, pid_t pid) {
         if (pid <= 1)
                 return NULL;
 
+        u = hashmap_get(m->watch_pids1, LONG_TO_PTR(pid));
+        if (u)
+                return u;
+
+        u = hashmap_get(m->watch_pids2, LONG_TO_PTR(pid));
+        if (u)
+                return u;
+
         r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, pid, &cgroup);
         if (r < 0)
                 return NULL;
@@ -1030,7 +1039,7 @@ int manager_notify_cgroup_empty(Manager *m, const char *cgroup) {
         if (!u)
                 return 0;
 
-        r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, true);
+        r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path);
         if (r <= 0)
                 return r;
 
@@ -1124,6 +1133,18 @@ int unit_reset_cpu_usage(Unit *u) {
         return 0;
 }
 
+bool unit_cgroup_delegate(Unit *u) {
+        CGroupContext *c;
+
+        assert(u);
+
+        c = unit_get_cgroup_context(u);
+        if (!c)
+                return false;
+
+        return c->delegate;
+}
+
 static const char* const cgroup_device_policy_table[_CGROUP_DEVICE_POLICY_MAX] = {
         [CGROUP_AUTO] = "auto",
         [CGROUP_CLOSED] = "closed",
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
index 869ddae8c4..7b38d210fb 100644
--- a/src/core/cgroup.h
+++ b/src/core/cgroup.h
@@ -130,5 +130,7 @@ int unit_get_memory_current(Unit *u, uint64_t *ret);
 int unit_get_cpu_usage(Unit *u, nsec_t *ret);
 int unit_reset_cpu_usage(Unit *u);
 
+bool unit_cgroup_delegate(Unit *u);
+
 const char* cgroup_device_policy_to_string(CGroupDevicePolicy i) _const_;
 CGroupDevicePolicy cgroup_device_policy_from_string(const char *s) _pure_;
diff --git a/src/core/main.c b/src/core/main.c
index 833fd74d87..2736b272dc 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -2048,7 +2048,7 @@ finish:
                  * kernel; at this point, we will not listen to the
                  * signals anyway */
                 if (detect_container(NULL) <= 0)
-                        cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER);
+                        (void) cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER);
 
                 execve(SYSTEMD_SHUTDOWN_BINARY_PATH, (char **) command_line, env_block);
                 log_error_errno(errno, "Failed to execute shutdown binary, %s: %m",
diff --git a/src/core/manager.c b/src/core/manager.c
index ede2a9910d..14f069ba97 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -573,6 +573,7 @@ int manager_new(ManagerRunningAs running_as, bool test_run, Manager **_m) {
 
         m->ask_password_inotify_fd = -1;
         m->have_ask_password = -EINVAL; /* we don't know */
+        m->first_boot = -1;
 
         m->test_run = test_run;
 
@@ -2998,12 +2999,14 @@ void manager_set_first_boot(Manager *m, bool b) {
         if (m->running_as != MANAGER_SYSTEM)
                 return;
 
-        m->first_boot = b;
+        if (m->first_boot != (int) b) {
+                if (b)
+                        (void) touch("/run/systemd/first-boot");
+                else
+                        (void) unlink("/run/systemd/first-boot");
+        }
 
-        if (m->first_boot)
-                touch("/run/systemd/first-boot");
-        else
-                unlink("/run/systemd/first-boot");
+        m->first_boot = b;
 }
 
 void manager_status_printf(Manager *m, StatusType type, const char *status, const char *format, ...) {
diff --git a/src/core/manager.h b/src/core/manager.h
index 1e01f2bdef..3f7fa24e58 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -233,7 +233,6 @@ struct Manager {
         bool dispatching_dbus_queue:1;
 
         bool taint_usr:1;
-        bool first_boot:1;
 
         bool test_run:1;
 
@@ -295,6 +294,8 @@ struct Manager {
 
         const char *unit_log_field;
         const char *unit_log_format_string;
+
+        int first_boot;
 };
 
 int manager_new(ManagerRunningAs running_as, bool test_run, Manager **m);
diff --git a/src/core/scope.c b/src/core/scope.c
index c594ab5294..1e94d63561 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -396,7 +396,7 @@ static bool scope_check_gc(Unit *u) {
         if (u->cgroup_path) {
                 int r;
 
-                r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, true);
+                r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path);
                 if (r <= 0)
                         return true;
         }
diff --git a/src/core/service.c b/src/core/service.c
index 3c4232417d..5a0a3aa867 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1269,7 +1269,7 @@ static int cgroup_good(Service *s) {
         if (!UNIT(s)->cgroup_path)
                 return 0;
 
-        r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, UNIT(s)->cgroup_path, true);
+        r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, UNIT(s)->cgroup_path);
         if (r < 0)
                 return r;
 
@@ -1520,18 +1520,33 @@ fail:
         service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES);
 }
 
+static bool service_good(Service *s) {
+        int main_pid_ok;
+        assert(s);
+
+        if (s->type == SERVICE_DBUS && !s->bus_name_good)
+                return false;
+
+        main_pid_ok = main_pid_good(s);
+        if (main_pid_ok > 0) /* It's alive */
+                return true;
+        if (main_pid_ok == 0) /* It's dead */
+                return false;
+
+        /* OK, we don't know anything about the main PID, maybe
+         * because there is none. Let's check the control group
+         * instead. */
+
+        return cgroup_good(s) != 0;
+}
+
 static void service_enter_running(Service *s, ServiceResult f) {
-        int main_pid_ok, cgroup_ok;
         assert(s);
 
         if (f != SERVICE_SUCCESS)
                 s->result = f;
 
-        main_pid_ok = main_pid_good(s);
-        cgroup_ok = cgroup_good(s);
-
-        if ((main_pid_ok > 0 || (main_pid_ok < 0 && cgroup_ok != 0)) &&
-            (s->bus_name_good || s->type != SERVICE_DBUS)) {
+        if (service_good(s)) {
 
                 /* If there are any queued up sd_notify()
                  * notifications, process them now */
diff --git a/src/core/unit.c b/src/core/unit.c
index 5f602bdf5f..34d3adcd3b 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -28,27 +28,28 @@
 #include "sd-id128.h"
 #include "sd-messages.h"
 #include "set.h"
-#include "unit.h"
 #include "macro.h"
 #include "strv.h"
 #include "path-util.h"
-#include "load-fragment.h"
-#include "load-dropin.h"
 #include "log.h"
-#include "unit-name.h"
-#include "dbus-unit.h"
-#include "special.h"
 #include "cgroup-util.h"
 #include "missing.h"
 #include "mkdir.h"
 #include "fileio-label.h"
-#include "bus-common-errors.h"
-#include "dbus.h"
-#include "execute.h"
-#include "dropin.h"
 #include "formats-util.h"
 #include "process-util.h"
+#include "virt.h"
+#include "bus-common-errors.h"
 #include "bus-util.h"
+#include "dropin.h"
+#include "unit-name.h"
+#include "special.h"
+#include "unit.h"
+#include "load-fragment.h"
+#include "load-dropin.h"
+#include "dbus.h"
+#include "dbus-unit.h"
+#include "execute.h"
 
 const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX] = {
         [UNIT_SERVICE] = &service_vtable,
@@ -2016,9 +2017,9 @@ void unit_unwatch_pid(Unit *u, pid_t pid) {
         assert(u);
         assert(pid >= 1);
 
-        hashmap_remove_value(u->manager->watch_pids1, LONG_TO_PTR(pid), u);
-        hashmap_remove_value(u->manager->watch_pids2, LONG_TO_PTR(pid), u);
-        set_remove(u->pids, LONG_TO_PTR(pid));
+        (void) hashmap_remove_value(u->manager->watch_pids1, LONG_TO_PTR(pid), u);
+        (void) hashmap_remove_value(u->manager->watch_pids2, LONG_TO_PTR(pid), u);
+        (void) set_remove(u->pids, LONG_TO_PTR(pid));
 }
 
 void unit_unwatch_all_pids(Unit *u) {
@@ -2440,6 +2441,9 @@ int unit_set_slice(Unit *u, Unit *slice) {
         if (u->type == UNIT_SLICE)
                 return -EINVAL;
 
+        if (unit_active_state(u) != UNIT_INACTIVE)
+                return -EBUSY;
+
         if (slice->type != UNIT_SLICE)
                 return -EINVAL;
 
@@ -3168,7 +3172,7 @@ int unit_kill_common(
                 if (!pid_set)
                         return -ENOMEM;
 
-                q = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, signo, false, true, false, pid_set);
+                q = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, signo, false, false, false, pid_set);
                 if (q < 0 && q != -EAGAIN && q != -ESRCH && q != -ENOENT)
                         r = q;
         }
@@ -3524,7 +3528,8 @@ int unit_kill_context(
                 pid_t control_pid,
                 bool main_pid_alien) {
 
-        int sig, wait_for_exit = false, r;
+        bool wait_for_exit = false;
+        int sig, r;
 
         assert(u);
         assert(c);
@@ -3553,13 +3558,13 @@ int unit_kill_context(
                         _cleanup_free_ char *comm = NULL;
                         get_process_comm(main_pid, &comm);
 
-                        log_unit_warning_errno(u, r, "Failed to kill main process " PID_FMT " (%s): %m", main_pid, strna(comm));
+                        log_unit_warning_errno(u, r, "Failed to kill main process " PID_FMT " (%s), ignoring: %m", main_pid, strna(comm));
                 } else {
                         if (!main_pid_alien)
                                 wait_for_exit = true;
 
-                        if (c->send_sighup && k != KILL_KILL)
-                                kill(main_pid, SIGHUP);
+                        if (c->send_sighup && k == KILL_TERMINATE)
+                                (void) kill(main_pid, SIGHUP);
                 }
         }
 
@@ -3570,16 +3575,17 @@ int unit_kill_context(
                         _cleanup_free_ char *comm = NULL;
                         get_process_comm(control_pid, &comm);
 
-                        log_unit_warning_errno(u, r, "Failed to kill control process " PID_FMT " (%s): %m", control_pid, strna(comm));
+                        log_unit_warning_errno(u, r, "Failed to kill control process " PID_FMT " (%s), ignoring: %m", control_pid, strna(comm));
                 } else {
                         wait_for_exit = true;
 
-                        if (c->send_sighup && k != KILL_KILL)
-                                kill(control_pid, SIGHUP);
+                        if (c->send_sighup && k == KILL_TERMINATE)
+                                (void) kill(control_pid, SIGHUP);
                 }
         }
 
-        if ((c->kill_mode == KILL_CONTROL_GROUP || (c->kill_mode == KILL_MIXED && k == KILL_KILL)) && u->cgroup_path) {
+        if (u->cgroup_path &&
+            (c->kill_mode == KILL_CONTROL_GROUP || (c->kill_mode == KILL_MIXED && k == KILL_KILL))) {
                 _cleanup_set_free_ Set *pid_set = NULL;
 
                 /* Exclude the main/control pids from being killed via the cgroup */
@@ -3587,21 +3593,26 @@ int unit_kill_context(
                 if (!pid_set)
                         return -ENOMEM;
 
-                r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, sig, true, true, false, pid_set);
+                r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, sig, true, k != KILL_TERMINATE, false, pid_set);
                 if (r < 0) {
                         if (r != -EAGAIN && r != -ESRCH && r != -ENOENT)
-                                log_unit_warning_errno(u, r, "Failed to kill control group: %m");
+                                log_unit_warning_errno(u, r, "Failed to kill control group %s, ignoring: %m", u->cgroup_path);
+
                 } else if (r > 0) {
 
                         /* FIXME: For now, we will not wait for the
-                         * cgroup members to die, simply because
-                         * cgroup notification is unreliable. It
-                         * doesn't work at all in containers, and
-                         * outside of containers it can be confused
-                         * easily by leaving directories in the
-                         * cgroup. */
-
-                        /* wait_for_exit = true; */
+                         * cgroup members to die if we are running in
+                         * a container or if this is a delegation
+                         * unit, simply because cgroup notification is
+                         * unreliable in these cases. It doesn't work
+                         * at all in containers, and outside of
+                         * containers it can be confused easily by
+                         * left-over directories in the cgroup --
+                         * which however should not exist in
+                         * non-delegated units. */
+
+                        if  (detect_container(NULL) == 0 && !unit_cgroup_delegate(u))
+                                wait_for_exit = true;
 
                         if (c->send_sighup && k != KILL_KILL) {
                                 set_free(pid_set);