unit: unify how we assing slices to units

This adds a new call unit_set_slice(), and simplifies
unit_add_default_slice(). THis should make our code a bit more robust
and simpler.

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 91c31987fe85bfee0a909e37825879f406193bd6..1e6291e762f4afb2966870e9c84c0ba98a96ac21 100644 (file)
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -965,38 +965,39 @@ static int bus_unit_set_transient_property(
 
                 return 1;
 
-        } else if (streq(name, "Slice") && UNIT_HAS_CGROUP_CONTEXT(u)) {
+        } else if (streq(name, "Slice")) {
+                Unit *slice;
                 const char *s;
 
+                if (!UNIT_HAS_CGROUP_CONTEXT(u))
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "The slice property is only available for units with control groups.");
+                if (u->type == UNIT_SLICE)
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Slice may not be set for slice units.");
+
                 r = sd_bus_message_read(message, "s", &s);
                 if (r < 0)
                         return r;
 
-                if (!unit_name_is_valid(s, UNIT_NAME_PLAIN) || !endswith(s, ".slice"))
-                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid slice name %s", s);
+                if (!unit_name_is_valid(s, UNIT_NAME_PLAIN))
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name '%s'", s);
 
-                if (isempty(s)) {
-                        if (mode != UNIT_CHECK) {
-                                unit_ref_unset(&u->slice);
-                                unit_remove_drop_in(u, mode, name);
-                        }
-                } else {
-                        Unit *slice;
+                r = manager_load_unit(u->manager, s, NULL, error, &slice);
+                if (r < 0)
+                        return r;
+
+                if (slice->type != UNIT_SLICE)
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit name '%s' is not a slice", s);
 
-                        r = manager_load_unit(u->manager, s, NULL, error, &slice);
+                if (mode != UNIT_CHECK) {
+                        r = unit_set_slice(u, slice);
                         if (r < 0)
                                 return r;
 
-                        if (slice->type != UNIT_SLICE)
-                                return -EINVAL;
-
-                        if (mode != UNIT_CHECK) {
-                                unit_ref_set(&u->slice, slice);
-                                unit_write_drop_in_private_format(u, mode, name, "Slice=%s\n", s);
-                        }
+                        unit_write_drop_in_private_format(u, mode, name, "Slice=%s\n", s);
                 }
 
                 return 1;
+
         } else if (STR_IN_SET(name,
                               "Requires", "RequiresOverridable",
                               "Requisite", "RequisiteOverridable",

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 721da3470e21a0398ba0fb15f54404796d2c88f1..745291c5c6b657638285f3cfaab7304bfc86b0b7 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2602,7 +2602,7 @@ int config_parse_unit_slice(
                 void *userdata) {
 
         _cleanup_free_ char *k = NULL;
-        Unit *u = userdata, *slice;
+        Unit *u = userdata, *slice = NULL;
         int r;
 
         assert(filename);
@@ -2611,29 +2611,23 @@ int config_parse_unit_slice(
         assert(u);
 
         r = unit_name_printf(u, rvalue, &k);
-        if (r < 0)
-                log_syntax(unit, LOG_ERR, filename, line, -r,
-                           "Failed to resolve unit specifiers on %s. Ignoring.", rvalue);
-        if (!k) {
-                k = strdup(rvalue);
-                if (!k)
-                        return log_oom();
+        if (r < 0) {
+                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s. Ignoring.", rvalue);
+                return 0;
         }
 
         r = manager_load_unit(u->manager, k, NULL, NULL, &slice);
         if (r < 0) {
-                log_syntax(unit, LOG_ERR, filename, line, -r,
-                           "Failed to load slice unit %s. Ignoring.", k);
+                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to load slice unit %s. Ignoring.", k);
                 return 0;
         }
 
-        if (slice->type != UNIT_SLICE) {
-                log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                           "Slice unit %s is not a slice. Ignoring.", k);
+        r = unit_set_slice(u, slice);
+        if (r < 0) {
+                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to assign slice %s to unit %s. Ignoring.", slice->id, u->id);
                 return 0;
         }
 
-        unit_ref_set(&u->slice, slice);
         return 0;
 }
 

diff --git a/src/core/mount.c b/src/core/mount.c
index 7e19e66a51edbed1e74dea64f522a090791d00de..2b81d17b9c92ec525067ac6a5b84ef3e95c10492 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -521,7 +521,7 @@ static int mount_add_extras(Mount *m) {
         if (r < 0)
                 return r;
 
-        r = unit_add_default_slice(u, &m->cgroup_context);
+        r = unit_set_default_slice(u);
         if (r < 0)
                 return r;
 

diff --git a/src/core/scope.c b/src/core/scope.c
index bf899361537ee4ec7d4ee46550385662cd39b380..c594ab529439f2c5d86642f2450ee9977b679813 100644 (file)
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -164,7 +164,7 @@ static int scope_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        r = unit_add_default_slice(u, &s->cgroup_context);
+        r = unit_set_default_slice(u);
         if (r < 0)
                 return r;
 

diff --git a/src/core/service.c b/src/core/service.c
index 097e7c710c4905f3f69c38a4f53676d91edeeb45..3c4232417da597fecf7923e6448aed3fb0f9855a 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -556,7 +556,7 @@ static int service_add_extras(Service *s) {
         if (r < 0)
                 return r;
 
-        r = unit_add_default_slice(UNIT(s), &s->cgroup_context);
+        r = unit_set_default_slice(UNIT(s));
         if (r < 0)
                 return r;
 

diff --git a/src/core/socket.c b/src/core/socket.c
index aa65c5b7a0b2569c3c0a4a9be33e1771b70de7e6..1014fad626dcaef7f40fafbca302823a2edb7bf7 100644 (file)
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -345,7 +345,7 @@ static int socket_add_extras(Socket *s) {
                 if (r < 0)
                         return r;
 
-                r = unit_add_default_slice(u, &s->cgroup_context);
+                r = unit_set_default_slice(u);
                 if (r < 0)
                         return r;
         }

diff --git a/src/core/swap.c b/src/core/swap.c
index 349fd6f7b9e8ea2d115b5e8e19629219b3c65f8a..4f3ddc9f04e7ba20a37c9c4dd45b0898984e4da6 100644 (file)
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -326,7 +326,7 @@ static int swap_load(Unit *u) {
                 if (r < 0)
                         return r;
 
-                r = unit_add_default_slice(u, &s->cgroup_context);
+                r = unit_set_default_slice(u);
                 if (r < 0)
                         return r;
 

diff --git a/src/core/unit.c b/src/core/unit.c
index 2ad49fd50b45f1c4d3ba3edd091f71bc84dbc195..9afb5736d7a6e7271d3299eac82197b6c8f1bd42 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2424,14 +2424,42 @@ char *unit_default_cgroup_path(Unit *u) {
                 return strjoin(u->manager->cgroup_root, "/", escaped, NULL);
 }
 
-int unit_add_default_slice(Unit *u, CGroupContext *c) {
+int unit_set_slice(Unit *u, Unit *slice) {
+        assert(u);
+        assert(slice);
+
+        /* Sets the unit slice if it has not been set before. Is extra
+         * careful, to only allow this for units that actually have a
+         * cgroup context. Also, we don't allow to set this for slices
+         * (since the parent slice is derived from the name). Make
+         * sure the unit we set is actually a slice. */
+
+        if (!UNIT_HAS_CGROUP_CONTEXT(u))
+                return -EOPNOTSUPP;
+
+        if (u->type == UNIT_SLICE)
+                return -EINVAL;
+
+        if (slice->type != UNIT_SLICE)
+                return -EINVAL;
+
+        if (UNIT_DEREF(u->slice) == slice)
+                return 0;
+
+        if (UNIT_ISSET(u->slice))
+                return -EBUSY;
+
+        unit_ref_set(&u->slice, slice);
+        return 1;
+}
+
+int unit_set_default_slice(Unit *u) {
         _cleanup_free_ char *b = NULL;
         const char *slice_name;
         Unit *slice;
         int r;
 
         assert(u);
-        assert(c);
 
         if (UNIT_ISSET(u->slice))
                 return 0;
@@ -2471,8 +2499,7 @@ int unit_add_default_slice(Unit *u, CGroupContext *c) {
         if (r < 0)
                 return r;
 
-        unit_ref_set(&u->slice, slice);
-        return 0;
+        return unit_set_slice(u, slice);
 }
 
 const char *unit_slice_name(Unit *u) {

diff --git a/src/core/unit.h b/src/core/unit.h
index 8da12356f89f11d691732002440287c3a5d0c22c..bc26653247bd33d24750b48637758dce92c66dfc 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -494,7 +494,8 @@ int unit_load_fragment_and_dropin(Unit *u);
 int unit_load_fragment_and_dropin_optional(Unit *u);
 int unit_load(Unit *unit);
 
-int unit_add_default_slice(Unit *u, CGroupContext *c);
+int unit_set_slice(Unit *u, Unit *slice);
+int unit_set_default_slice(Unit *u);
 
 const char *unit_description(Unit *u) _pure_;