From fbdc87679cc4f3c9fc3653636e94be20f06d18e4 Mon Sep 17 00:00:00 2001
From: Anita Zhang <the.anitazha@gmail.com>
Date: Tue, 9 Nov 2021 15:26:28 -0800
Subject: [PATCH] core: replace slice dependencies as they get added
Defines a "UNIT_DEPENDENCY_SLICE_PROPERTY" UnitDependencyMask type that
is used when adding slices to the dependencies hashmap. This type is
used to remove slice dependencies when they get overridden by new ones.
Fixes #20182
---
src/core/dbus-unit.c | 2 +-
src/core/load-fragment.c | 2 +-
src/core/unit-serialize.c | 1 +
src/core/unit.c | 10 +++++++---
src/core/unit.h | 7 +++++--
src/test/test-engine.c | 31 ++++++++++++++++++++++++++++++-
6 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index fe320f1b05a8..d4ec789a7c11 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -2273,7 +2273,7 @@ static int bus_unit_set_transient_property(
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit name '%s' is not a slice", s);
if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
- r = unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);
+ r = unit_set_slice(u, slice);
if (r < 0)
return r;
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 62cadaf2286f..830048ae1915 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3792,7 +3792,7 @@ int config_parse_unit_slice(
return 0;
}
- r = unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);
+ r = unit_set_slice(u, slice);
if (r < 0) {
log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to assign slice %s to unit %s, ignoring: %m", slice->id, u->id);
return 0;
diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c
index 3458d7017bd5..7d2e6bc130de 100644
--- a/src/core/unit-serialize.c
+++ b/src/core/unit-serialize.c
@@ -593,6 +593,7 @@ static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependency
{ UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT, "mountinfo-implicit" },
{ UNIT_DEPENDENCY_MOUNTINFO_DEFAULT, "mountinfo-default" },
{ UNIT_DEPENDENCY_PROC_SWAP, "proc-swap" },
+ { UNIT_DEPENDENCY_SLICE_PROPERTY, "slice-property" },
};
assert(f);
diff --git a/src/core/unit.c b/src/core/unit.c
index 4c55827a6511..a3bca43566e0 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -3284,7 +3284,7 @@ int unit_set_invocation_id(Unit *u, sd_id128_t id) {
return r;
}
-int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) {
+int unit_set_slice(Unit *u, Unit *slice) {
int r;
assert(u);
@@ -3317,7 +3317,11 @@ int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) {
if (UNIT_GET_SLICE(u) && u->cgroup_realized)
return -EBUSY;
- r = unit_add_dependency(u, UNIT_IN_SLICE, slice, true, mask);
+ /* Remove any slices assigned prior; we should only have one UNIT_IN_SLICE dependency */
+ if (UNIT_GET_SLICE(u))
+ unit_remove_dependencies(u, UNIT_DEPENDENCY_SLICE_PROPERTY);
+
+ r = unit_add_dependency(u, UNIT_IN_SLICE, slice, true, UNIT_DEPENDENCY_SLICE_PROPERTY);
if (r < 0)
return r;
@@ -3373,7 +3377,7 @@ int unit_set_default_slice(Unit *u) {
if (r < 0)
return r;
- return unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);
+ 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 0dd6a9591d96..ba12fe4ac1ef 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -89,7 +89,10 @@ typedef enum UnitDependencyMask {
/* A dependency created because of data read from /proc/swaps and no other configuration source */
UNIT_DEPENDENCY_PROC_SWAP = 1 << 7,
- _UNIT_DEPENDENCY_MASK_FULL = (1 << 8) - 1,
+ /* A dependency for units in slices assigned by directly setting Slice= */
+ UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 8,
+
+ _UNIT_DEPENDENCY_MASK_FULL = (1 << 9) - 1,
} UnitDependencyMask;
/* The Unit's dependencies[] hashmaps use this structure as value. It has the same size as a void pointer, and thus can
@@ -782,7 +785,7 @@ Unit *unit_follow_merge(Unit *u) _pure_;
int unit_load_fragment_and_dropin(Unit *u, bool fragment_required);
int unit_load(Unit *unit);
-int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask);
+int unit_set_slice(Unit *u, Unit *slice);
int unit_set_default_slice(Unit *u);
const char *unit_description(Unit *u) _pure_;
diff --git a/src/test/test-engine.c b/src/test/test-engine.c
index 880af36fb523..673c66561240 100644
--- a/src/test/test-engine.c
+++ b/src/test/test-engine.c
@@ -8,6 +8,7 @@
#include "manager-dump.h"
#include "rm-rf.h"
#include "service.h"
+#include "slice.h"
#include "special.h"
#include "strv.h"
#include "tests.h"
@@ -75,7 +76,8 @@ int main(int argc, char *argv[]) {
_cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL;
_cleanup_(manager_freep) Manager *m = NULL;
Unit *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL,
- *h = NULL, *i = NULL, *a_conj = NULL, *unit_with_multiple_dashes = NULL, *stub = NULL;
+ *h = NULL, *i = NULL, *a_conj = NULL, *unit_with_multiple_dashes = NULL, *stub = NULL,
+ *tomato = NULL, *sauce = NULL, *fruit = NULL, *zupa = NULL;
Job *j;
int r;
@@ -260,5 +262,32 @@ int main(int argc, char *argv[]) {
verify_dependency_atoms();
+ /* Test adding multiple Slice= dependencies; only the last should remain */
+ assert_se(unit_new_for_name(m, sizeof(Service), "tomato.service", &tomato) >= 0);
+ assert_se(unit_new_for_name(m, sizeof(Slice), "sauce.slice", &sauce) >= 0);
+ assert_se(unit_new_for_name(m, sizeof(Slice), "fruit.slice", &fruit) >= 0);
+ assert_se(unit_new_for_name(m, sizeof(Slice), "zupa.slice", &zupa) >= 0);
+
+ unit_set_slice(tomato, sauce);
+ unit_set_slice(tomato, fruit);
+ unit_set_slice(tomato, zupa);
+
+ assert_se(UNIT_GET_SLICE(tomato) == zupa);
+ assert_se(!unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, sauce));
+ assert_se(!unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, fruit));
+ assert_se(unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, zupa));
+
+ assert_se(!unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, sauce));
+ assert_se(!unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, fruit));
+ assert_se(unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, zupa));
+
+ assert_se(!unit_has_dependency(sauce, UNIT_ATOM_SLICE_OF, tomato));
+ assert_se(!unit_has_dependency(fruit, UNIT_ATOM_SLICE_OF, tomato));
+ assert_se(unit_has_dependency(zupa, UNIT_ATOM_SLICE_OF, tomato));
+
+ assert_se(!unit_has_dependency(sauce, UNIT_ATOM_REFERENCED_BY, tomato));
+ assert_se(!unit_has_dependency(fruit, UNIT_ATOM_REFERENCED_BY, tomato));
+ assert_se(unit_has_dependency(zupa, UNIT_ATOM_REFERENCED_BY, tomato));
+
return 0;
}