From d79200e26ee39d4b451f95e876fc4595df51fe51 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 17:36:39 +0200 Subject: 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. --- src/core/dbus-unit.c | 37 +++++++++++++++++++------------------ src/core/load-fragment.c | 22 ++++++++-------------- src/core/mount.c | 2 +- src/core/scope.c | 2 +- src/core/service.c | 2 +- src/core/socket.c | 2 +- src/core/swap.c | 2 +- src/core/unit.c | 35 +++++++++++++++++++++++++++++++---- src/core/unit.h | 3 ++- 9 files changed, 65 insertions(+), 42 deletions(-) (limited to 'src/core') diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 91c31987fe..1e6291e762 100644 --- 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 721da3470e..745291c5c6 100644 --- 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 7e19e66a51..2b81d17b9c 100644 --- 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 bf89936153..c594ab5294 100644 --- 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 097e7c710c..3c4232417d 100644 --- 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 aa65c5b7a0..1014fad626 100644 --- 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 349fd6f7b9..4f3ddc9f04 100644 --- 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 2ad49fd50b..9afb5736d7 100644 --- 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 8da12356f8..bc26653247 100644 --- 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_; -- cgit v1.2.3-54-g00ecf