SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #23367 from yuwata/core-mount

Browse Source 
core/mount: fix dependency issues
This commit is contained in:
Yu Watanabe
2022-08-05 13:48:23 +09:00
committed by GitHub
parent 49ad287292 413f20b4ac
commit 14e7bc2e77
6 changed files with 207 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/mount.c
+45 -67
View File
@@ -347,7 +347,6 @@ static int mount_add_mount_dependencies(Mount *m) {
}
static int mount_add_device_dependencies(Mount *m) {
UnitDependencyMask mask;
MountParameters *p;
UnitDependency dep;
int r;
@@ -396,17 +395,14 @@ static int mount_add_device_dependencies(Mount *m) {
* suddenly. */
dep = mount_is_bound_to_device(m) ? UNIT_BINDS_TO : UNIT_REQUIRES;
/* We always use 'what' from /proc/self/mountinfo if mounted */
mask = m->from_proc_self_mountinfo ? UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT : UNIT_DEPENDENCY_FILE;
r = unit_add_node_dependency(UNIT(m), p->what, dep, mask);
r = unit_add_node_dependency(UNIT(m), p->what, dep, UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
if (r > 0)
log_unit_trace(UNIT(m), "Added %s dependency on %s", unit_dependency_to_string(dep), p->what);
if (mount_propagate_stop(m)) {
r = unit_add_node_dependency(UNIT(m), p->what, UNIT_STOP_PROPAGATED_FROM, mask);
r = unit_add_node_dependency(UNIT(m), p->what, UNIT_STOP_PROPAGATED_FROM, UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
if (r > 0)
@@ -414,11 +410,11 @@ static int mount_add_device_dependencies(Mount *m) {
unit_dependency_to_string(UNIT_STOP_PROPAGATED_FROM), p->what);
}
r = unit_add_blockdev_dependency(UNIT(m), p->what, mask);
r = unit_add_blockdev_dependency(UNIT(m), p->what, UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r > 0)
log_unit_trace(UNIT(m), "Added %s dependency on %s", unit_dependency_to_string(UNIT_AFTER), p->what);
return r;
return 0;
}
static int mount_add_quota_dependencies(Mount *m) {
@@ -474,8 +470,7 @@ static bool mount_is_extrinsic(Unit *u) {
static int mount_add_default_ordering_dependencies(
Mount *m,
MountParameters *p,
UnitDependencyMask mask) {
MountParameters *p) {
const char *after, *before, *e;
int r;
@@ -501,23 +496,25 @@ static int mount_add_default_ordering_dependencies(
}
if (!mount_is_nofail(m)) {
r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, before, true, mask);
r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, before, true,
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
}
if (after) {
r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, true, mask);
r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, true,
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
}
return unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS,
SPECIAL_UMOUNT_TARGET, true, mask);
SPECIAL_UMOUNT_TARGET, true,
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
}
static int mount_add_default_dependencies(Mount *m) {
UnitDependencyMask mask;
MountParameters *p;
int r;
@@ -537,9 +534,7 @@ static int mount_add_default_dependencies(Mount *m) {
if (!p)
return 0;
mask = m->from_fragment ? UNIT_DEPENDENCY_FILE : UNIT_DEPENDENCY_MOUNTINFO_DEFAULT;
r = mount_add_default_ordering_dependencies(m, p, mask);
r = mount_add_default_ordering_dependencies(m, p);
if (r < 0)
return r;
@@ -549,7 +544,8 @@ static int mount_add_default_dependencies(Mount *m) {
* network.target, so that they are shut down only after this mount unit is
* stopped. */
r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_NETWORK_TARGET, true, mask);
r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_NETWORK_TARGET, true,
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
@@ -558,14 +554,17 @@ static int mount_add_default_dependencies(Mount *m) {
* mounting network file systems, and whose purpose it is to delay this until the
* network is "up". */
r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS, UNIT_AFTER, SPECIAL_NETWORK_ONLINE_TARGET, true, mask);
r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS, UNIT_AFTER,
SPECIAL_NETWORK_ONLINE_TARGET, true,
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
}
/* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */
if (streq_ptr(p->fstype, "tmpfs")) {
r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, true, mask);
r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, true,
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (r < 0)
return r;
}
@@ -607,8 +606,18 @@ static int mount_verify(Mount *m) {
static int mount_add_non_exec_dependencies(Mount *m) {
int r;
assert(m);
/* Any dependencies based on /proc/self/mountinfo may be now stale. */
unit_remove_dependencies(UNIT(m),
UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT |
UNIT_DEPENDENCY_MOUNTINFO_DEFAULT |
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE);
if (!m->where)
return 0;
/* Adds in all dependencies directly responsible for ordering the mount, as opposed to dependencies
* resulting from the ExecContext and such. */
@@ -908,8 +917,9 @@ static void mount_enter_dead(Mount *m, MountResult f) {
dynamic_creds_destroy(&m->dynamic_creds);
/* Any dependencies based on /proc/self/mountinfo are now stale */
unit_remove_dependencies(UNIT(m), UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT);
/* Any dependencies based on /proc/self/mountinfo are now stale. Let's re-generate dependencies from
* .mount unit. */
(void) mount_add_non_exec_dependencies(m);
}
static void mount_enter_mounted(Mount *m, MountResult f) {
@@ -1600,15 +1610,15 @@ static int mount_setup_new_unit(
if (r < 0)
return r;
r = mount_add_non_exec_dependencies(MOUNT(u));
if (r < 0)
return r;
/* This unit was generated because /proc/self/mountinfo reported it. Remember this, so that by the
* time we load the unit file for it (and thus add in extra deps right after) we know what source to
* attributes the deps to. */
MOUNT(u)->from_proc_self_mountinfo = true;
r = mount_add_non_exec_dependencies(MOUNT(u));
if (r < 0)
return r;
/* We have only allocated the stub now, let's enqueue this unit for loading now, so that everything
* else is loaded in now. */
unit_add_to_load_queue(u);
@@ -1674,9 +1684,6 @@ static int mount_setup_existing_unit(
if (FLAGS_SET(flags, MOUNT_PROC_JUST_CHANGED)) {
/* If things changed, then make sure that all deps are regenerated. Let's
* first remove all automatic deps, and then add in the new ones. */
unit_remove_dependencies(u, UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT);
r = mount_add_non_exec_dependencies(MOUNT(u));
if (r < 0)
return r;
@@ -1717,37 +1724,14 @@ static int mount_setup_unit(
if (!is_path(where))
return 0;
/* Mount unit names have to be (like all other unit names) short enough to fit into file names. This
* means there's a good chance that overly long mount point paths after mangling them to look like a
* unit name would result in unit names we don't actually consider valid. This should be OK however
* as such long mount point paths should not happen on regular systems — and if they appear
* nonetheless they are generally synthesized by software, and thus managed by that other
* software. Having such long names just means you cannot use systemd to manage those specific mount
* points, which should be an OK restriction to make. After all we don't have to be able to manage
* all mount points in the world — as long as we don't choke on them when we encounter them. */
r = unit_name_from_path(where, ".mount", &e);
if (r < 0) {
static RateLimit rate_limit = { /* Let's log about this at warning level at most once every
* 5s. Given that we generate this whenever we read the file
* otherwise we probably shouldn't flood the logs with
* this */
.interval = 5 * USEC_PER_SEC,
.burst = 1,
};
if (r == -ENAMETOOLONG)
return log_struct_errno(
ratelimit_below(&rate_limit) ? LOG_WARNING : LOG_DEBUG, r,
"MESSAGE_ID=" SD_MESSAGE_MOUNT_POINT_PATH_NOT_SUITABLE_STR,
"MOUNT_POINT=%s", where,
LOG_MESSAGE("Mount point path '%s' too long to fit into unit name, ignoring mount point.", where));
if (r < 0)
return log_struct_errno(
ratelimit_below(&rate_limit) ? LOG_WARNING : LOG_DEBUG, r,
LOG_WARNING, r,
"MESSAGE_ID=" SD_MESSAGE_MOUNT_POINT_PATH_NOT_SUITABLE_STR,
"MOUNT_POINT=%s", where,
LOG_MESSAGE("Failed to generate valid unit name from mount point path '%s', ignoring mount point: %m", where));
}
LOG_MESSAGE("Failed to generate valid unit name from mount point path '%s', ignoring mount point: %m",
where));
u = manager_get_unit(m, e);
if (u)
@@ -1984,7 +1968,7 @@ static int drain_libmount(Manager *m) {
}
static int mount_process_proc_self_mountinfo(Manager *m) {
_cleanup_set_free_free_ Set *around = NULL, *gone = NULL;
_cleanup_set_free_ Set *around = NULL, *gone = NULL;
const char *what;
int r;
@@ -2015,13 +1999,10 @@ static int mount_process_proc_self_mountinfo(Manager *m) {
* existed. */
if (mount->from_proc_self_mountinfo &&
mount->parameters_proc_self_mountinfo.what) {
mount->parameters_proc_self_mountinfo.what)
/* Remember that this device might just have disappeared */
if (set_ensure_allocated(&gone, &path_hash_ops) < 0 ||
set_put_strdup(&gone, mount->parameters_proc_self_mountinfo.what) < 0)
if (set_put_strdup_full(&gone, &path_hash_ops_free, mount->parameters_proc_self_mountinfo.what) < 0)
log_oom(); /* we don't care too much about OOM here... */
}
mount->from_proc_self_mountinfo = false;
assert_se(update_parameters_proc_self_mountinfo(mount, NULL, NULL, NULL) >= 0);
@@ -2079,13 +2060,10 @@ static int mount_process_proc_self_mountinfo(Manager *m) {
if (mount_is_mounted(mount) &&
mount->from_proc_self_mountinfo &&
mount->parameters_proc_self_mountinfo.what) {
mount->parameters_proc_self_mountinfo.what)
/* Track devices currently used */
if (set_ensure_allocated(&around, &path_hash_ops) < 0 ||
set_put_strdup(&around, mount->parameters_proc_self_mountinfo.what) < 0)
if (set_put_strdup_full(&around, &path_hash_ops_free, mount->parameters_proc_self_mountinfo.what) < 0)
log_oom();
}
/* Reset the flags for later calls */
mount->proc_flags = 0;
src/core/swap.c
+1 -2
View File
@@ -1437,8 +1437,7 @@ int swap_process_device_new(Manager *m, sd_device *dev) {
int q;
q = unit_name_from_path(devlink, ".swap", &n);
if (IN_SET(q, -EINVAL, -ENAMETOOLONG)) /* If name too long or otherwise not convertible to
* unit name, we can't manage it */
if (q == -EINVAL) /* If the name is not convertible to unit name, we can't manage it */
continue;
if (q < 0)
return q;
src/core/unit-serialize.c
+1
View File
@@ -592,6 +592,7 @@ static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependency
{ UNIT_DEPENDENCY_PATH, "path" },
{ UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT, "mountinfo-implicit" },
{ UNIT_DEPENDENCY_MOUNTINFO_DEFAULT, "mountinfo-default" },
{ UNIT_DEPENDENCY_MOUNTINFO_OR_FILE, "mountinfo-or-file" },
{ UNIT_DEPENDENCY_PROC_SWAP, "proc-swap" },
{ UNIT_DEPENDENCY_SLICE_PROPERTY, "slice-property" },
};
src/core/unit.c
+1 -1
View File
@@ -1519,7 +1519,7 @@ static int unit_add_mount_dependencies(Unit *u) {
Unit *m;
r = unit_name_from_path(prefix, ".mount", &p);
if (IN_SET(r, -EINVAL, -ENAMETOOLONG))
if (r == -EINVAL)
continue; /* If the path cannot be converted to a mount unit name, then it's
* not manageable as a unit by systemd, and hence we don't need a
* dependency on it. Let's thus silently ignore the issue. */
src/core/unit.h
+7 -3
View File
@@ -86,13 +86,17 @@ typedef enum UnitDependencyMask {
* DefaultDependencies= and thus also involving configuration from UNIT_DEPENDENCY_FILE sources */
UNIT_DEPENDENCY_MOUNTINFO_DEFAULT = 1 << 6,
/* A dependency created because of data read from /proc/self/mountinfo, but fallback to unit configuration
* sources */
UNIT_DEPENDENCY_MOUNTINFO_OR_FILE = 1 << 7,
/* A dependency created because of data read from /proc/swaps and no other configuration source */
UNIT_DEPENDENCY_PROC_SWAP = 1 << 7,
UNIT_DEPENDENCY_PROC_SWAP = 1 << 8,
/* A dependency for units in slices assigned by directly setting Slice= */
UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 8,
UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 9,
_UNIT_DEPENDENCY_MASK_FULL = (1 << 9) - 1,
_UNIT_DEPENDENCY_MASK_FULL = (1 << 10) - 1,
} UnitDependencyMask;
/* The Unit's dependencies[] hashmaps use this structure as value. It has the same size as a void pointer, and thus can
test/units/testsuite-60.sh
+152
View File
@@ -3,6 +3,155 @@
set -eux
set -o pipefail
# shellcheck source=test/units/assert.sh
. "$(dirname "$0")"/assert.sh
teardown_test_dependencies() (
set +eux
if mountpoint /tmp/deptest; then
umount /tmp/deptest
fi
if [[ -n "${LOOP}" ]]; then
losetup -d "${LOOP}" || :
fi
if [[ -n "${LOOP_0}" ]]; then
losetup -d "${LOOP_0}" || :
fi
if [[ -n "${LOOP_1}" ]]; then
losetup -d "${LOOP_1}" || :
fi
rm -f /tmp/testsuite-60-dependencies-0.img
rm -f /tmp/testsuite-60-dependencies-1.img
rm -f /run/systemd/system/tmp-deptest.mount
systemctl daemon-reload
return 0
)
setup_loop() {
truncate -s 30m "/tmp/testsuite-60-dependencies-${1?}.img"
sfdisk --wipe=always "/tmp/testsuite-60-dependencies-${1?}.img" <<EOF
label:gpt
name="loop${1?}-part1"
EOF
LOOP=$(losetup -P --show -f "/tmp/testsuite-60-dependencies-${1?}.img")
udevadm wait --settle --timeout=10 "${LOOP}p1"
udevadm lock --device="${LOOP}p1" mkfs.ext4 -L "partname${1?}-1" "${LOOP}p1"
}
check_dependencies() {
local escaped_0 escaped_1 after
escaped_0=$(systemd-escape -p "${LOOP_0}p1")
escaped_1=$(systemd-escape -p "${LOOP_1}p1")
if [[ -f /run/systemd/system/tmp-deptest.mount ]]; then
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_not_in "local-fs-pre.target" "$after"
assert_in "remote-fs-pre.target" "$after"
assert_in "network.target" "$after"
fi
# mount LOOP_0
mount -t ext4 "${LOOP_0}p1" /tmp/deptest
sleep 1
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_in "local-fs-pre.target" "$after"
assert_not_in "remote-fs-pre.target" "$after"
assert_not_in "network.target" "$after"
assert_in "${escaped_0}.device" "$after"
assert_in "blockdev@${escaped_0}.target" "$after"
assert_not_in "${escaped_1}.device" "$after"
assert_not_in "blockdev@${escaped_1}.target" "$after"
umount /tmp/deptest
if [[ -f /run/systemd/system/tmp-deptest.mount ]]; then
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_not_in "local-fs-pre.target" "$after"
assert_in "remote-fs-pre.target" "$after"
assert_in "network.target" "$after"
fi
# mount LOOP_1 (using fake _netdev option)
mount -t ext4 -o _netdev "${LOOP_1}p1" /tmp/deptest
sleep 1
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_not_in "local-fs-pre.target" "$after"
assert_in "remote-fs-pre.target" "$after"
assert_in "network.target" "$after"
assert_not_in "${escaped_0}.device" "$after"
assert_not_in "blockdev@${escaped_0}.target" "$after"
assert_in "${escaped_1}.device" "$after"
assert_in "blockdev@${escaped_1}.target" "$after"
umount /tmp/deptest
if [[ -f /run/systemd/system/tmp-deptest.mount ]]; then
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_not_in "local-fs-pre.target" "$after"
assert_in "remote-fs-pre.target" "$after"
assert_in "network.target" "$after"
fi
# mount tmpfs
mount -t tmpfs tmpfs /tmp/deptest
sleep 1
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_in "local-fs-pre.target" "$after"
assert_not_in "remote-fs-pre.target" "$after"
assert_not_in "network.target" "$after"
assert_not_in "${escaped_0}.device" "$after"
assert_not_in "blockdev@${escaped_0}.target" "$after"
assert_not_in "${escaped_1}.device" "$after"
assert_not_in "blockdev@${escaped_1}.target" "$after"
umount /tmp/deptest
if [[ -f /run/systemd/system/tmp-deptest.mount ]]; then
after=$(systemctl show --property=After --value tmp-deptest.mount)
assert_not_in "local-fs-pre.target" "$after"
assert_in "remote-fs-pre.target" "$after"
assert_in "network.target" "$after"
fi
}
test_dependencies() {
if systemd-detect-virt --quiet --container; then
echo "Skipping test_dependencies in container"
return
fi
trap teardown_test_dependencies RETURN
setup_loop 0
LOOP_0="${LOOP}"
LOOP=
setup_loop 1
LOOP_1="${LOOP}"
LOOP=
mkdir -p /tmp/deptest
# without .mount file
check_dependencies
# create .mount file
mkdir -p /run/systemd/system
cat >/run/systemd/system/tmp-deptest.mount <<EOF
[Mount]
Where=/tmp/deptest
What=192.168.0.1:/tmp/mnt
Type=nfs
EOF
systemctl daemon-reload
# with .mount file
check_dependencies
}
test_issue_20329() {
local tmpdir unit
tmpdir="$(mktemp -d)"
@@ -108,6 +257,9 @@ fi
# Give some time for units to settle so we don't race between exiting the rate limit state and cleaning up the units.
timeout 2m bash -c 'while systemctl list-units -t mount tmp-meow* | grep -q tmp-meow; do systemctl daemon-reload; sleep 10; done'
# test for issue #19983 and #23552.
test_dependencies
# test that handling of mount start jobs is delayed when /proc/self/mouninfo monitor is rate limited
test_issue_20329