From c5d82021ba126a52964753d7327c143b27a54662 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 5 Oct 2022 10:26:59 +0200 Subject: [PATCH 1/3] mount: always use UNIT_DEPENDENCY_FILE in mount_add_quota_dependencies() The quota options have always been read from the unit file and ignored if only present in /proc/self/mountinfo. IOW the quota services are not (automagically) pulled in for mounts initiated by the user running mount(8). --- src/core/mount.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index fba7c4cac3..f971519dc6 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -421,7 +421,6 @@ static int mount_add_device_dependencies(Mount *m) { } static int mount_add_quota_dependencies(Mount *m) { - UnitDependencyMask mask; MountParameters *p; int r; @@ -437,13 +436,13 @@ static int mount_add_quota_dependencies(Mount *m) { if (!mount_needs_quota(p)) return 0; - mask = m->from_fragment ? UNIT_DEPENDENCY_FILE : UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT; - - r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTACHECK_SERVICE, true, mask); + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTACHECK_SERVICE, + /* add_reference= */ true, UNIT_DEPENDENCY_FILE); if (r < 0) return r; - r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTAON_SERVICE, true, mask); + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTAON_SERVICE, + /* add_reference= */true, UNIT_DEPENDENCY_FILE); if (r < 0) return r; From 9c77ffc7e0459e2bbc4c0c54ff065aa302ecd62e Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 12 Sep 2022 17:50:51 +0200 Subject: [PATCH 2/3] mount: drop UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT and UNIT_DEPENDENCY_MOUNTINFO_DEFAULT They're not used anymore. --- src/core/mount.c | 9 ++++----- src/core/unit-serialize.c | 2 -- src/core/unit.h | 15 ++++----------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index f971519dc6..f9c5c80e35 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -611,11 +611,10 @@ static int mount_add_non_exec_dependencies(Mount *m) { 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); + /* We may be called due to this mount appearing in /proc/self/mountinfo, hence we clear all existing + * dependencies that were initialized from the unit file but whose final value really depends on the + * content of /proc/self/mountinfo. Some (such as m->where) might have become stale now. */ + unit_remove_dependencies(UNIT(m), UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); if (!m->where) return 0; diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index 797bf6c5ce..a8fb405550 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -590,8 +590,6 @@ static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependency { UNIT_DEPENDENCY_DEFAULT, "default" }, { UNIT_DEPENDENCY_UDEV, "udev" }, { 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" }, diff --git a/src/core/unit.h b/src/core/unit.h index 01cef89525..06ec7ea7b5 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -79,24 +79,17 @@ typedef enum UnitDependencyMask { /* A dependency created because of some unit's RequiresMountsFor= setting */ UNIT_DEPENDENCY_PATH = 1 << 4, - /* A dependency created because of data read from /proc/self/mountinfo and no other configuration source */ - UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT = 1 << 5, - - /* A dependency created because of data read from /proc/self/mountinfo, but conditionalized by - * 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, + UNIT_DEPENDENCY_MOUNTINFO_OR_FILE = 1 << 5, /* A dependency created because of data read from /proc/swaps and no other configuration source */ - UNIT_DEPENDENCY_PROC_SWAP = 1 << 8, + UNIT_DEPENDENCY_PROC_SWAP = 1 << 6, /* A dependency for units in slices assigned by directly setting Slice= */ - UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 9, + UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 7, - _UNIT_DEPENDENCY_MASK_FULL = (1 << 10) - 1, + _UNIT_DEPENDENCY_MASK_FULL = (1 << 8) - 1, } UnitDependencyMask; /* The Unit's dependencies[] hashmaps use this structure as value. It has the same size as a void pointer, and thus can From 87c734eedd4cba78d2a5b8aa1fe07b85efbc5a5a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 12 Sep 2022 17:54:22 +0200 Subject: [PATCH 3/3] mount: replace UNIT_DEPENDENCY_MOUNTINFO_OR_FILE with UNIT_DEPENDENCY_MOUNTINFO/UNIT_DEPENDENCY_MOUNT_FILE UNIT_DEPENDENCY_MOUNTINFO_OR_FILE was a bit strange as unlike the other flags we don't know where the dependency came from exactly. Indeed its origin could have been from the mount unit file or from /proc/self/mountinfo. Instead this patch replaces UNIT_DEPENDENCY_MOUNTINFO_OR_FILE with 2 new dependency flags: UNIT_DEPENDENCY_MOUNT_FILE and UNIT_DEPENDENCY_MOUNTINFO. The former indicates that the dep is created from the unit file but unlike UNIT_DEPENDENCY_FILE, it will be replaced by a dep with the UNIT_DEPENDENCY_MOUNTINFO flag as soon as the kernel will make the mount available in /proc/self/mountinfo. --- src/core/mount.c | 46 +++++++++++++++++++-------------------- src/core/unit-serialize.c | 3 ++- src/core/unit.h | 16 +++++++++----- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index f9c5c80e35..d6bf009ada 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -350,6 +350,7 @@ static int mount_add_mount_dependencies(Mount *m) { } static int mount_add_device_dependencies(Mount *m) { + UnitDependencyMask mask; MountParameters *p; UnitDependency dep; int r; @@ -398,14 +399,17 @@ static int mount_add_device_dependencies(Mount *m) { * suddenly. */ dep = mount_is_bound_to_device(m) ? UNIT_BINDS_TO : UNIT_REQUIRES; - r = unit_add_node_dependency(UNIT(m), p->what, dep, UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + /* We always use 'what' from /proc/self/mountinfo if mounted */ + mask = m->from_proc_self_mountinfo ? UNIT_DEPENDENCY_MOUNTINFO : UNIT_DEPENDENCY_MOUNT_FILE; + + r = unit_add_node_dependency(UNIT(m), p->what, dep, mask); 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, UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_node_dependency(UNIT(m), p->what, UNIT_STOP_PROPAGATED_FROM, mask); if (r < 0) return r; if (r > 0) @@ -413,7 +417,7 @@ 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, UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_blockdev_dependency(UNIT(m), p->what, mask); if (r > 0) log_unit_trace(UNIT(m), "Added %s dependency on %s", unit_dependency_to_string(UNIT_AFTER), p->what); @@ -470,10 +474,7 @@ static bool mount_is_extrinsic(Unit *u) { return false; } -static int mount_add_default_ordering_dependencies( - Mount *m, - MountParameters *p) { - +static int mount_add_default_ordering_dependencies(Mount *m, MountParameters *p, UnitDependencyMask mask) { const char *after, *before, *e; int r; @@ -498,25 +499,23 @@ static int mount_add_default_ordering_dependencies( } if (!mount_is_nofail(m)) { - r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, before, true, - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, before, /* add_reference= */ true, mask); if (r < 0) return r; } if (after) { - r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, true, - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, /* add_reference= */ true, mask); if (r < 0) return r; } - return unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, - SPECIAL_UMOUNT_TARGET, true, - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + return unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, + /* add_reference= */ true, mask); } static int mount_add_default_dependencies(Mount *m) { + UnitDependencyMask mask; MountParameters *p; int r; @@ -536,7 +535,9 @@ static int mount_add_default_dependencies(Mount *m) { if (!p) return 0; - r = mount_add_default_ordering_dependencies(m, p); + mask = m->from_proc_self_mountinfo ? UNIT_DEPENDENCY_MOUNTINFO : UNIT_DEPENDENCY_MOUNT_FILE; + + r = mount_add_default_ordering_dependencies(m, p, mask); if (r < 0) return r; @@ -546,8 +547,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, - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_NETWORK_TARGET, + /* add_reference= */ true, mask); if (r < 0) return r; @@ -556,17 +557,16 @@ 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, - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS, UNIT_AFTER, SPECIAL_NETWORK_ONLINE_TARGET, + /* add_reference= */ true, mask); 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, - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, + /* add_reference= */ true, mask); if (r < 0) return r; } @@ -614,7 +614,7 @@ static int mount_add_non_exec_dependencies(Mount *m) { /* We may be called due to this mount appearing in /proc/self/mountinfo, hence we clear all existing * dependencies that were initialized from the unit file but whose final value really depends on the * content of /proc/self/mountinfo. Some (such as m->where) might have become stale now. */ - unit_remove_dependencies(UNIT(m), UNIT_DEPENDENCY_MOUNTINFO_OR_FILE); + unit_remove_dependencies(UNIT(m), UNIT_DEPENDENCY_MOUNTINFO | UNIT_DEPENDENCY_MOUNT_FILE); if (!m->where) return 0; diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index a8fb405550..e85b41a97d 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -590,7 +590,8 @@ static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependency { UNIT_DEPENDENCY_DEFAULT, "default" }, { UNIT_DEPENDENCY_UDEV, "udev" }, { UNIT_DEPENDENCY_PATH, "path" }, - { UNIT_DEPENDENCY_MOUNTINFO_OR_FILE, "mountinfo-or-file" }, + { UNIT_DEPENDENCY_MOUNT_FILE, "mount-file" }, + { UNIT_DEPENDENCY_MOUNTINFO, "mountinfo" }, { UNIT_DEPENDENCY_PROC_SWAP, "proc-swap" }, { UNIT_DEPENDENCY_SLICE_PROPERTY, "slice-property" }, }; diff --git a/src/core/unit.h b/src/core/unit.h index 06ec7ea7b5..b34c59c3eb 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -79,17 +79,21 @@ typedef enum UnitDependencyMask { /* A dependency created because of some unit's RequiresMountsFor= setting */ UNIT_DEPENDENCY_PATH = 1 << 4, - /* A dependency created because of data read from /proc/self/mountinfo, but fallback to unit configuration - * sources */ - UNIT_DEPENDENCY_MOUNTINFO_OR_FILE = 1 << 5, + /* A dependency initially configured from the mount unit file however the dependency will be updated + * from /proc/self/mountinfo as soon as the kernel will make the entry for that mount available in + * the /proc file */ + UNIT_DEPENDENCY_MOUNT_FILE = 1 << 5, + + /* A dependency created or updated because of data read from /proc/self/mountinfo */ + UNIT_DEPENDENCY_MOUNTINFO = 1 << 6, /* A dependency created because of data read from /proc/swaps and no other configuration source */ - UNIT_DEPENDENCY_PROC_SWAP = 1 << 6, + UNIT_DEPENDENCY_PROC_SWAP = 1 << 7, /* A dependency for units in slices assigned by directly setting Slice= */ - UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 7, + UNIT_DEPENDENCY_SLICE_PROPERTY = 1 << 8, - _UNIT_DEPENDENCY_MASK_FULL = (1 << 8) - 1, + _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