From b68731eeb692a4cfcdf5790ae118ebf21d827301 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

Reference:https://github.com/systemd/systemd/commit/899acf5c2d4b89caa891d05ccfa4be828a999c2d
Conflict: NA
---
 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 aa10939..c42ae5e 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -2272,7 +2272,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 399a759..3f5d6d3 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3575,7 +3575,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 689a536..3544f18 100644
--- a/src/core/unit-serialize.c
+++ b/src/core/unit-serialize.c
@@ -571,6 +571,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 ffcf8eb..bff0527 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -3267,7 +3267,7 @@ reset:
         return r;
 }
 
-int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) {
+int unit_set_slice(Unit *u, Unit *slice) {
         int r;
 
         assert(u);
@@ -3300,7 +3300,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;
 
@@ -3353,7 +3357,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 759104f..6dc2bcb 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -88,7 +88,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
@@ -761,7 +764,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 6dc1619..1ac15a8 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;
 }
-- 
1.8.3.1