shaba/systemd-stable
1
1
Fork 0
mirror of https://github.com/systemd/systemd-stable.git synced 2025-01-08 21:17:47 +03:00
Code

Merge pull request #25387 from yuwata/core-fix-gc-logic

Browse Source 
core: fix logic of merging units
This commit is contained in:
Luca Boccassi 2022-12-17 14:49:21 +01:00 committed by GitHub
commit 896785a7d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 167 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

123
src/core/unit.c
View File

@ -721,7 +721,6 @@ Unit* unit_free(Unit *u) {
if (u->on_console)
manager_unref_console(u->manager);
fdset_free(u->initial_socket_bind_link_fds);
#if BPF_FRAMEWORK
bpf_link_free(u->ipv4_socket_bind_link);
@ -938,29 +937,17 @@ static int unit_reserve_dependencies(Unit *u, Unit *other) {
return 0;
}
static void unit_maybe_warn_about_dependency(
Unit *u,
const char *other_id,
UnitDependency dependency) {
assert(u);
static bool unit_should_warn_about_dependency(UnitDependency dependency) {
/* Only warn about some unit types */
if (!IN_SET(dependency,
UNIT_CONFLICTS,
UNIT_CONFLICTED_BY,
UNIT_BEFORE,
UNIT_AFTER,
UNIT_ON_SUCCESS,
UNIT_ON_FAILURE,
UNIT_TRIGGERS,
UNIT_TRIGGERED_BY))
return;
if (streq_ptr(u->id, other_id))
log_unit_warning(u, "Dependency %s=%s dropped", unit_dependency_to_string(dependency), u->id);
else
log_unit_warning(u, "Dependency %s=%s dropped, merged into %s", unit_dependency_to_string(dependency), strna(other_id), u->id);
return IN_SET(dependency,
UNIT_CONFLICTS,
UNIT_CONFLICTED_BY,
UNIT_BEFORE,
UNIT_AFTER,
UNIT_ON_SUCCESS,
UNIT_ON_FAILURE,
UNIT_TRIGGERS,
UNIT_TRIGGERED_BY);
}
static int unit_per_dependency_type_hashmap_update(
@ -1045,11 +1032,10 @@ static int unit_add_dependency_hashmap(
return unit_per_dependency_type_hashmap_update(per_type, other, origin_mask, destination_mask);
}
static void unit_merge_dependencies(
Unit *u,
Unit *other) {
int r;
static void unit_merge_dependencies(Unit *u, Unit *other) {
Hashmap *deps;
void *dt; /* Actually of type UnitDependency, except that we don't bother casting it here,
* since the hashmaps all want it as void pointer. */
assert(u);
assert(other);
@ -1057,18 +1043,29 @@ static void unit_merge_dependencies(
if (u == other)
return;
/* First, remove dependency to other. */
HASHMAP_FOREACH_KEY(deps, dt, u->dependencies) {
if (hashmap_remove(deps, other) && unit_should_warn_about_dependency(UNIT_DEPENDENCY_FROM_PTR(dt)))
log_unit_warning(u, "Dependency %s=%s is dropped, as %s is merged into %s.",
unit_dependency_to_string(UNIT_DEPENDENCY_FROM_PTR(dt)),
other->id, other->id, u->id);
if (hashmap_isempty(deps))
hashmap_free(hashmap_remove(u->dependencies, dt));
}
for (;;) {
_cleanup_(hashmap_freep) Hashmap *other_deps = NULL;
UnitDependencyInfo di_back;
Unit *back;
void *dt; /* Actually of type UnitDependency, except that we don't bother casting it here,
* since the hashmaps all want it as void pointer. */
/* Let's focus on one dependency type at a time, that 'other' has defined. */
other_deps = hashmap_steal_first_key_and_value(other->dependencies, &dt);
if (!other_deps)
break; /* done! */
deps = hashmap_get(u->dependencies, dt);
/* Now iterate through all dependencies of this dependency type, of 'other'. We refer to the
* referenced units as 'back'. */
HASHMAP_FOREACH_KEY(di_back.data, back, other_deps) {
@ -1078,7 +1075,12 @@ static void unit_merge_dependencies(
if (back == u) {
/* This is a dependency pointing back to the unit we want to merge with?
* Suppress it (but warn) */
unit_maybe_warn_about_dependency(u, other->id, UNIT_DEPENDENCY_FROM_PTR(dt));
if (unit_should_warn_about_dependency(UNIT_DEPENDENCY_FROM_PTR(dt)))
log_unit_warning(u, "Dependency %s=%s in %s is dropped, as %s is merged into %s.",
unit_dependency_to_string(UNIT_DEPENDENCY_FROM_PTR(dt)),
u->id, other->id, other->id, u->id);
hashmap_remove(other_deps, back);
continue;
}
@ -1097,41 +1099,21 @@ static void unit_merge_dependencies(
di_move.origin_mask,
di_move.destination_mask) >= 0);
}
}
/* Now all references towards 'other' of the current type 'dt' are corrected to point to
* 'u'. Lets's now move the deps of type 'dt' from 'other' to 'u'. First, let's try to move
* them per type wholesale. */
r = hashmap_put(u->dependencies, dt, other_deps);
if (r == -EEXIST) {
Hashmap *deps;
/* The target unit already has dependencies of this type, let's then merge this individually. */
assert_se(deps = hashmap_get(u->dependencies, dt));
for (;;) {
UnitDependencyInfo di_move;
/* Get first dep */
di_move.data = hashmap_steal_first_key_and_value(other_deps, (void**) &back);
if (!di_move.data)
break; /* done */
if (back == u) {
/* Would point back to us, ignore */
unit_maybe_warn_about_dependency(u, other->id, UNIT_DEPENDENCY_FROM_PTR(dt));
continue;
}
assert_se(unit_per_dependency_type_hashmap_update(deps, back, di_move.origin_mask, di_move.destination_mask) >= 0);
}
} else {
assert_se(r >= 0);
TAKE_PTR(other_deps);
if (hashmap_remove(other_deps, u))
unit_maybe_warn_about_dependency(u, other->id, UNIT_DEPENDENCY_FROM_PTR(dt));
if (deps)
assert_se(unit_per_dependency_type_hashmap_update(
deps,
back,
di_back.origin_mask,
di_back.destination_mask) >= 0);
}
/* Now all references towards 'other' of the current type 'dt' are corrected to point to 'u'.
* Lets's now move the deps of type 'dt' from 'other' to 'u'. If the unit does not have
* dependencies of this type, let's move them per type wholesale. */
if (!deps)
assert_se(hashmap_put(u->dependencies, dt, TAKE_PTR(other_deps)) >= 0);
}
other->dependencies = hashmap_free(other->dependencies);
@ -1177,11 +1159,6 @@ int unit_merge(Unit *u, Unit *other) {
if (r < 0)
return r;
/* Merge names */
r = unit_merge_names(u, other);
if (r < 0)
return r;
/* Redirect all references */
while (other->refs_by_target)
unit_ref_set(other->refs_by_target, other->refs_by_target->source, u);
@ -1189,6 +1166,11 @@ int unit_merge(Unit *u, Unit *other) {
/* Merge dependencies */
unit_merge_dependencies(u, other);
/* Merge names. It is better to do that after merging deps, otherwise the log message contains n/a. */
r = unit_merge_names(u, other);
if (r < 0)
return r;
other->load_state = UNIT_MERGED;
other->merged_into = u;
@ -3067,7 +3049,6 @@ int unit_add_dependency(
[UNIT_IN_SLICE] = UNIT_SLICE_OF,
[UNIT_SLICE_OF] = UNIT_IN_SLICE,
};
Unit *original_u = u, *original_other = other;
UnitDependencyAtom a;
int r;
@ -3086,7 +3067,9 @@ int unit_add_dependency(
/* We won't allow dependencies on ourselves. We will not consider them an error however. */
if (u == other) {
unit_maybe_warn_about_dependency(original_u, original_other->id, d);
if (unit_should_warn_about_dependency(d))
log_unit_warning(u, "Dependency %s=%s is dropped.",
unit_dependency_to_string(d), u->id);
return 0;
}

116
test/units/testsuite-26.sh
View File

@ -4,10 +4,18 @@
set -eux
set -o pipefail
# shellcheck source=test/units/assert.sh
. "$(dirname "$0")"/assert.sh
: >/failed
at_exit() {
if [[ -v UNIT_NAME && -e "/usr/lib/systemd/system/$UNIT_NAME" ]]; then
rm -fvr "/usr/lib/systemd/system/$UNIT_NAME" "/etc/systemd/system/$UNIT_NAME.d" "+4"
fi
rm -f /etc/init.d/issue-24990
return 0
}
# shellcheck source=test/units/assert.sh
@ -307,6 +315,110 @@ systemctl unset-environment IMPORT_THIS IMPORT_THIS_TOO
(! systemctl show-environment | grep "^IMPORT_THIS=")
(! systemctl show-environment | grep "^IMPORT_THIS_TOO=")
echo OK >/testok
# test for sysv-generator (issue #24990)
if [[ -x /usr/lib/systemd/system-generators/systemd-sysv-generator ]]; then
exit 0
# invalid dependency
cat >/etc/init.d/issue-24990 <<\EOF
#!/bin/bash
### BEGIN INIT INFO
# Provides:test1 test2
# Required-Start:test1 $remote_fs $network
# Required-Stop:test1 $remote_fs $network
# Description:Test
# Short-Description: Test
### END INIT INFO
case "$1" in
start)
echo "Starting issue-24990.service"
sleep 1000 &
;;
stop)
echo "Stopping issue-24990.service"
sleep 10 &
;;
*)
echo "Usage: service test {start|stop|restart|status}"
;;
esac
EOF
chmod +x /etc/init.d/issue-24990
systemctl daemon-reload
[[ -L /run/systemd/generator.late/test1.service ]]
[[ -L /run/systemd/generator.late/test2.service ]]
assert_eq "$(readlink -f /run/systemd/generator.late/test1.service)" "/run/systemd/generator.late/issue-24990.service"
assert_eq "$(readlink -f /run/systemd/generator.late/test2.service)" "/run/systemd/generator.late/issue-24990.service"
output=$(systemctl cat issue-24990)
assert_in "SourcePath=/etc/init.d/issue-24990" "$output"
assert_in "Description=LSB: Test" "$output"
assert_in "After=test1.service" "$output"
assert_in "After=remote-fs.target" "$output"
assert_in "After=network-online.target" "$output"
assert_in "Wants=network-online.target" "$output"
assert_in "ExecStart=/etc/init.d/issue-24990 start" "$output"
assert_in "ExecStop=/etc/init.d/issue-24990 stop" "$output"
systemctl status issue-24990 || :
systemctl show issue-24990
assert_not_in "issue-24990.service" "$(systemctl show --property=After --value)"
assert_not_in "issue-24990.service" "$(systemctl show --property=Before --value)"
if ! systemctl is-active network-online.target; then
systemctl start network-online.target
fi
systemctl restart issue-24990
systemctl stop issue-24990
# valid dependency
cat >/etc/init.d/issue-24990 <<\EOF
#!/bin/bash
### BEGIN INIT INFO
# Provides:test1 test2
# Required-Start:$remote_fs
# Required-Stop:$remote_fs
# Description:Test
# Short-Description: Test
### END INIT INFO
case "$1" in
start)
echo "Starting issue-24990.service"
sleep 1000 &
;;
stop)
echo "Stopping issue-24990.service"
sleep 10 &
;;
*)
echo "Usage: service test {start|stop|restart|status}"
;;
esac
EOF
chmod +x /etc/init.d/issue-24990
systemctl daemon-reload
[[ -L /run/systemd/generator.late/test1.service ]]
[[ -L /run/systemd/generator.late/test2.service ]]
assert_eq "$(readlink -f /run/systemd/generator.late/test1.service)" "/run/systemd/generator.late/issue-24990.service"
assert_eq "$(readlink -f /run/systemd/generator.late/test2.service)" "/run/systemd/generator.late/issue-24990.service"
output=$(systemctl cat issue-24990)
assert_in "SourcePath=/etc/init.d/issue-24990" "$output"
assert_in "Description=LSB: Test" "$output"
assert_in "After=remote-fs.target" "$output"
assert_in "ExecStart=/etc/init.d/issue-24990 start" "$output"
assert_in "ExecStop=/etc/init.d/issue-24990 stop" "$output"
systemctl status issue-24990 || :
systemctl show issue-24990
assert_not_in "issue-24990.service" "$(systemctl show --property=After --value)"
assert_not_in "issue-24990.service" "$(systemctl show --property=Before --value)"
systemctl restart issue-24990
systemctl stop issue-24990
fi
touch /testok
rm /failed