From c08e08530274672882c35b5170441929eb5a9f14 Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Oct 20 2021 08:11:57 +0000 Subject: 249.4-2.5: revert commit that breaks deterministic Slice= assignments --- diff --git a/SOURCES/revert_d219a2b07cc5dc8ffd5010f08561fab2780d8616.patch b/SOURCES/revert_d219a2b07cc5dc8ffd5010f08561fab2780d8616.patch new file mode 100644 index 0000000..2ffb1a0 --- /dev/null +++ b/SOURCES/revert_d219a2b07cc5dc8ffd5010f08561fab2780d8616.patch @@ -0,0 +1,405 @@ +From 2f9e75564e04ed3e6f7aacb264ea69f5582050ca Mon Sep 17 00:00:00 2001 +From: Anita Zhang +Date: Tue, 19 Oct 2021 18:00:58 -0700 +Subject: [PATCH] Revert "core: convert Slice= into a proper dependency (and + add a back dependency)" + +This reverts commit d219a2b07cc5dc8ffd5010f08561fab2780d8616. +--- + man/org.freedesktop.systemd1.xml | 6 ----- + src/basic/unit-def.c | 2 -- + src/basic/unit-def.h | 4 ---- + src/core/cgroup.c | 23 ++++++++++++++---- + src/core/dbus-unit.c | 3 +-- + src/core/load-fragment.c | 2 +- + src/core/slice.c | 21 +++++++++++++---- + src/core/unit-dependency-atom.c | 8 ------- + src/core/unit-dependency-atom.h | 4 +--- + src/core/unit.c | 40 +++++++++----------------------- + src/core/unit.h | 7 ++++-- + 11 files changed, 54 insertions(+), 66 deletions(-) + +diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml +index c14c5b6601..c87feea201 100644 +--- a/man/org.freedesktop.systemd1.xml ++++ b/man/org.freedesktop.systemd1.xml +@@ -1650,8 +1650,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly as JoinsNamespaceOf = ['...', ...]; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") +- readonly as SliceOf = ['...', ...]; +- @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly as RequiresMountsFor = ['...', ...]; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly as Documentation = ['...', ...]; +@@ -1799,8 +1797,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + + +- +- + + + +@@ -1949,8 +1945,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + + +- +- + + + +diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c +index 2667e61dc4..23e4825bd7 100644 +--- a/src/basic/unit-def.c ++++ b/src/basic/unit-def.c +@@ -289,8 +289,6 @@ static const char* const unit_dependency_table[_UNIT_DEPENDENCY_MAX] = { + [UNIT_JOINS_NAMESPACE_OF] = "JoinsNamespaceOf", + [UNIT_REFERENCES] = "References", + [UNIT_REFERENCED_BY] = "ReferencedBy", +- [UNIT_IN_SLICE] = "InSlice", +- [UNIT_SLICE_OF] = "SliceOf", + }; + + DEFINE_STRING_TABLE_LOOKUP(unit_dependency, UnitDependency); +diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h +index 08651efa57..e72452702a 100644 +--- a/src/basic/unit-def.h ++++ b/src/basic/unit-def.h +@@ -254,10 +254,6 @@ typedef enum UnitDependency { + UNIT_REFERENCES, /* Inverse of 'references' is 'referenced_by' */ + UNIT_REFERENCED_BY, + +- /* Slice= */ +- UNIT_IN_SLICE, +- UNIT_SLICE_OF, +- + _UNIT_DEPENDENCY_MAX, + _UNIT_DEPENDENCY_INVALID = -EINVAL, + } UnitDependency; +diff --git a/src/core/cgroup.c b/src/core/cgroup.c +index 5c07aa71d1..9033bea169 100644 +--- a/src/core/cgroup.c ++++ b/src/core/cgroup.c +@@ -1706,8 +1706,12 @@ CGroupMask unit_get_members_mask(Unit *u) { + if (u->type == UNIT_SLICE) { + Unit *member; + +- UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF) ++ UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_BEFORE) { ++ if (UNIT_DEREF(member->slice) != u) ++ continue; ++ + u->cgroup_members_mask |= unit_get_subtree_mask(member); /* note that this calls ourselves again, for the children */ ++ } + } + + u->cgroup_members_mask_valid = true; +@@ -2365,10 +2369,13 @@ static int unit_realize_cgroup_now_disable(Unit *u, ManagerState state) { + if (u->type != UNIT_SLICE) + return 0; + +- UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_SLICE_OF) { ++ UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_BEFORE) { + CGroupMask target_mask, enable_mask, new_target_mask, new_enable_mask; + int r; + ++ if (UNIT_DEREF(m->slice) != u) ++ continue; ++ + /* The cgroup for this unit might not actually be fully realised yet, in which case it isn't + * holding any controllers open anyway. */ + if (!m->cgroup_realized) +@@ -2530,7 +2537,11 @@ void unit_add_family_to_cgroup_realize_queue(Unit *u) { + /* Children of u likely changed when we're called */ + u->cgroup_members_mask_valid = false; + +- UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_SLICE_OF) { ++ UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_BEFORE) { ++ ++ /* Skip units that have a dependency on the slice but aren't actually in it. */ ++ if (UNIT_DEREF(m->slice) != u) ++ continue; + + /* No point in doing cgroup application for units without active processes. */ + if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(m))) +@@ -3869,8 +3880,12 @@ void unit_invalidate_cgroup_bpf(Unit *u) { + if (u->type == UNIT_SLICE) { + Unit *member; + +- UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF) ++ UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_BEFORE) { ++ if (UNIT_DEREF(member->slice) != u) ++ continue; ++ + unit_invalidate_cgroup_bpf(member); ++ } + } + } + +diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c +index 124b9e496b..f861988bd8 100644 +--- a/src/core/dbus-unit.c ++++ b/src/core/dbus-unit.c +@@ -875,7 +875,6 @@ const sd_bus_vtable bus_unit_vtable[] = { + SD_BUS_PROPERTY("PropagatesStopTo", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StopPropagatedFrom", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("JoinsNamespaceOf", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), +- SD_BUS_PROPERTY("SliceOf", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("RequiresMountsFor", "as", property_get_requires_mounts_for, offsetof(Unit, requires_mounts_for), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Documentation", "as", NULL, offsetof(Unit, documentation), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Description", "s", property_get_description, 0, SD_BUS_VTABLE_PROPERTY_CONST), +@@ -2273,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 8fb3c378ee..a068fdf313 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/slice.c b/src/core/slice.c +index 2e43c00119..009c98b434 100644 +--- a/src/core/slice.c ++++ b/src/core/slice.c +@@ -47,7 +47,7 @@ static void slice_set_state(Slice *t, SliceState state) { + } + + static int slice_add_parent_slice(Slice *s) { +- Unit *u = UNIT(s); ++ Unit *u = UNIT(s), *parent; + _cleanup_free_ char *a = NULL; + int r; + +@@ -60,7 +60,12 @@ static int slice_add_parent_slice(Slice *s) { + if (r <= 0) /* 0 means root slice */ + return r; + +- return unit_add_dependency_by_name(u, UNIT_IN_SLICE, a, true, UNIT_DEPENDENCY_IMPLICIT); ++ r = manager_load_unit(u->manager, a, NULL, NULL, &parent); ++ if (r < 0) ++ return r; ++ ++ unit_ref_set(&u->slice, u, parent); ++ return 0; + } + + static int slice_add_default_dependencies(Slice *s) { +@@ -349,11 +354,14 @@ static void slice_enumerate_perpetual(Manager *m) { + + static bool slice_freezer_action_supported_by_children(Unit *s) { + Unit *member; +- int r; + + assert(s); + +- UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) { ++ UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_BEFORE) { ++ int r; ++ ++ if (UNIT_DEREF(member->slice) != s) ++ continue; + + if (member->type == UNIT_SLICE) { + r = slice_freezer_action_supported_by_children(member); +@@ -380,7 +388,10 @@ static int slice_freezer_action(Unit *s, FreezerAction action) { + return 0; + } + +- UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) { ++ UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_BEFORE) { ++ if (UNIT_DEREF(member->slice) != s) ++ continue; ++ + if (action == FREEZER_FREEZE) + r = UNIT_VTABLE(member)->freeze(member); + else +diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c +index 761835828a..9a8e4615f1 100644 +--- a/src/core/unit-dependency-atom.c ++++ b/src/core/unit-dependency-atom.c +@@ -93,8 +93,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { + [UNIT_JOINS_NAMESPACE_OF] = UNIT_ATOM_JOINS_NAMESPACE_OF, + [UNIT_REFERENCES] = UNIT_ATOM_REFERENCES, + [UNIT_REFERENCED_BY] = UNIT_ATOM_REFERENCED_BY, +- [UNIT_IN_SLICE] = UNIT_ATOM_IN_SLICE, +- [UNIT_SLICE_OF] = UNIT_ATOM_SLICE_OF, + + /* These are dependency types without effect on our state engine. We maintain them only to make + * things discoverable/debuggable as they are the inverse dependencies to some of the above. As they +@@ -228,12 +226,6 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) { + case UNIT_ATOM_REFERENCED_BY: + return UNIT_REFERENCED_BY; + +- case UNIT_ATOM_IN_SLICE: +- return UNIT_IN_SLICE; +- +- case UNIT_ATOM_SLICE_OF: +- return UNIT_SLICE_OF; +- + default: + return _UNIT_DEPENDENCY_INVALID; + } +diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h +index c5f8f5d400..f79f1b732a 100644 +--- a/src/core/unit-dependency-atom.h ++++ b/src/core/unit-dependency-atom.h +@@ -77,9 +77,7 @@ typedef enum UnitDependencyAtom { + UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 29, + UNIT_ATOM_REFERENCES = UINT64_C(1) << 30, + UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 31, +- UNIT_ATOM_IN_SLICE = UINT64_C(1) << 32, +- UNIT_ATOM_SLICE_OF = UINT64_C(1) << 33, +- _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 34) - 1, ++ _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 32) - 1, + _UNIT_DEPENDENCY_ATOM_INVALID = -EINVAL, + } UnitDependencyAtom; + +diff --git a/src/core/unit.c b/src/core/unit.c +index 38d3eb703f..48810a2ba9 100644 +--- a/src/core/unit.c ++++ b/src/core/unit.c +@@ -692,10 +692,11 @@ Unit* unit_free(Unit *u) { + job_free(j); + } + ++ unit_clear_dependencies(u); ++ + /* A unit is being dropped from the tree, make sure our family is realized properly. Do this after we + * detach the unit from slice tree in order to eliminate its effect on controller masks. */ + slice = UNIT_GET_SLICE(u); +- unit_clear_dependencies(u); + if (slice) + unit_add_family_to_cgroup_realize_queue(slice); + +@@ -721,6 +722,7 @@ Unit* unit_free(Unit *u) { + + unit_unwatch_all_pids(u); + ++ unit_ref_unset(&u->slice); + while (u->refs_by_target) + unit_ref_unset(u->refs_by_target); + +@@ -3032,8 +3034,6 @@ int unit_add_dependency( + [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF, /* symmetric! 👓 */ + [UNIT_REFERENCES] = UNIT_REFERENCED_BY, + [UNIT_REFERENCED_BY] = UNIT_REFERENCES, +- [UNIT_IN_SLICE] = UNIT_SLICE_OF, +- [UNIT_SLICE_OF] = UNIT_IN_SLICE, + }; + Unit *original_u = u, *original_other = other; + UnitDependencyAtom a; +@@ -3077,21 +3077,6 @@ int unit_add_dependency( + return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL), + "Requested dependency TriggeredBy=%s refused (%s units cannot trigger other units).", other->id, unit_type_to_string(other->type)); + +- if (FLAGS_SET(a, UNIT_ATOM_IN_SLICE) && other->type != UNIT_SLICE) +- return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL), +- "Requested dependency Slice=%s refused (%s is not a slice unit).", other->id, other->id); +- if (FLAGS_SET(a, UNIT_ATOM_SLICE_OF) && u->type != UNIT_SLICE) +- return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL), +- "Requested dependency SliceOf=%s refused (%s is not a slice unit).", other->id, u->id); +- +- if (FLAGS_SET(a, UNIT_ATOM_IN_SLICE) && !UNIT_HAS_CGROUP_CONTEXT(u)) +- return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL), +- "Requested dependency Slice=%s refused (%s is not a cgroup unit).", other->id, u->id); +- +- if (FLAGS_SET(a, UNIT_ATOM_SLICE_OF) && !UNIT_HAS_CGROUP_CONTEXT(other)) +- return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL), +- "Requested dependency SliceOf=%s refused (%s is not a cgroup unit).", other->id, other->id); +- + r = unit_add_dependency_hashmap(&u->dependencies, d, other, mask, 0); + if (r < 0) + return r; +@@ -3270,15 +3255,15 @@ reset: + return r; + } + +-int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) { +- int r; +- ++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. */ ++ /* 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; +@@ -3303,10 +3288,7 @@ 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); +- if (r < 0) +- return r; +- ++ unit_ref_set(&u->slice, u, slice); + return 1; + } + +@@ -3356,7 +3338,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 759104ffa7..7a8bfd0568 100644 +--- a/src/core/unit.h ++++ b/src/core/unit.h +@@ -202,6 +202,8 @@ typedef struct Unit { + dual_timestamp active_exit_timestamp; + dual_timestamp inactive_enter_timestamp; + ++ UnitRef slice; ++ + /* Per type list */ + LIST_FIELDS(Unit, units_by_type); + +@@ -716,7 +718,8 @@ static inline Unit* UNIT_TRIGGER(Unit *u) { + } + + static inline Unit* UNIT_GET_SLICE(const Unit *u) { +- return unit_has_dependency(u, UNIT_ATOM_IN_SLICE, NULL); ++ assert(u); ++ return u->slice.target; + } + + Unit* unit_new(Manager *m, size_t size); +@@ -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_; +-- +2.31.1 + diff --git a/SPECS/systemd.spec b/SPECS/systemd.spec index 25924ab..a06f6f4 100644 --- a/SPECS/systemd.spec +++ b/SPECS/systemd.spec @@ -40,7 +40,7 @@ Name: systemd Url: https://www.freedesktop.org/wiki/Software/systemd %if %{without inplace} Version: 249.4 -Release: 2.5%{?dist} +Release: 2.6%{?dist} %else # determine the build information from local checkout Version: %(tools/meson-vcs-tag.sh . error | sed -r 's/-([0-9])/.^\1/; s/-g/_g/') @@ -144,6 +144,8 @@ Patch0020: 20676_cherrypicked.patch Patch0501: https://github.com/systemd/systemd/pull/17050/commits/f58b96d3e8d1cb0dd3666bc74fa673918b586612.patch # Downgrade sysv-generator messages from warning to debug Patch0502: 0001-sysv-generator-downgrade-log-warning-about-autogener.patch +# Fixes non-deterministic Slice= assignments +Patch0503: revert_d219a2b07cc5dc8ffd5010f08561fab2780d8616.patch %ifarch %{ix86} x86_64 aarch64 %global have_gnu_efi 1 @@ -1081,6 +1083,9 @@ fi %endif %changelog +* Wed Oct 20 2021 Anita Zhang - 249.4-2.6 +- Revert d219a2b because it creates non-determinisitic Slice= assignments + * Mon Oct 11 2021 Anita Zhang - 249.4-2.5 - Remove duplicate Address= properties in network configs (part of PR #20892) - Serialize bpf device programs across reloads/reexecs (PR #20978)