Revert "mount: check right before invoking /bin/umount if it makes sense"

This reverts commit 1483892a421ca34bc841a8e8b1f385744c0407ed.

As the commit says, it does not solve the race. Moreover, it introduces
an regression #28410.

Also, checking by `path_is_mount_point()` may trigger automount. From
statx(2),
> AT_NO_AUTOMOUNT
>     Don't automount the terminal ("basename") component of pathname
>     if it is a directory that is an automount point.
Similar statements can be found in fstatat(2), which is used in the
fallback call for statx() in glibc, and name_to_handle_at(2), which is
used as the fallback when statx() failed.
So, `path_is_mount_point()` may _do_ trigger automount for parent paths.
That should be avoided especially on shutdown.

The original issue #25527 that is 'fixed' by the commit is not serious,
and should be fixed by making umount command handle path gracefully:
https://github.com/util-linux/util-linux/issues/2132

Fixes #28410.

diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c
index 595e4414c7e0a04e57ac53f503df70dfb95fd3b4..2361132936894410cb62401e8d9b320cdd941d92 100644 (file)
--- a/src/basic/unit-def.c
+++ b/src/basic/unit-def.c
@@ -152,7 +152,6 @@ static const char* const mount_state_table[_MOUNT_STATE_MAX] = {
         [MOUNT_REMOUNTING_SIGKILL] = "remounting-sigkill",
         [MOUNT_UNMOUNTING_SIGTERM] = "unmounting-sigterm",
         [MOUNT_UNMOUNTING_SIGKILL] = "unmounting-sigkill",
-        [MOUNT_UNMOUNTING_CATCHUP] = "unmounting-catchup",
         [MOUNT_FAILED]             = "failed",
         [MOUNT_CLEANING]           = "cleaning",
 };

diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h
index f67fc1d620d8e04af7efbd21dcff9a49fcf3b692..345ca533a30e76832ff169d97cbf751ec5b14c57 100644 (file)
--- a/src/basic/unit-def.h
+++ b/src/basic/unit-def.h
@@ -97,7 +97,6 @@ typedef enum MountState {
         MOUNT_REMOUNTING_SIGKILL,
         MOUNT_UNMOUNTING_SIGTERM,
         MOUNT_UNMOUNTING_SIGKILL,
-        MOUNT_UNMOUNTING_CATCHUP,
         MOUNT_FAILED,
         MOUNT_CLEANING,
         _MOUNT_STATE_MAX,

diff --git a/src/core/mount.c b/src/core/mount.c
index 5bd72a0b656ce7ef9c12ae43d65b3d585ce33e8a..b773ce5fcae7a05a976c9a8fdc71a23a6078b456 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -48,7 +48,6 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = {
         [MOUNT_REMOUNTING_SIGKILL] = UNIT_RELOADING,
         [MOUNT_UNMOUNTING_SIGTERM] = UNIT_DEACTIVATING,
         [MOUNT_UNMOUNTING_SIGKILL] = UNIT_DEACTIVATING,
-        [MOUNT_UNMOUNTING_CATCHUP] = UNIT_DEACTIVATING,
         [MOUNT_FAILED] = UNIT_FAILED,
         [MOUNT_CLEANING] = UNIT_MAINTENANCE,
 };
@@ -1063,37 +1062,11 @@ static void mount_enter_unmounting(Mount *m) {
 
         assert(m);
 
-        r = path_is_mount_point(m->where, NULL, 0);
-        if (IN_SET(r, 0, -ENOENT)) {
-                /* Not a mount point anymore ? Then we raced against something else which unmounted it? Let's
-                 * handle this gracefully, and wait until we get the kernel notification about it. */
-
-                log_unit_debug(UNIT(m), "Path '%s' is not a mount point anymore, waiting for mount table refresh.", m->where);
-
-                /* Apparently our idea of the kernel mount table is out of date. Make sure we re-read it
-                 * again, soon, so that we don't delay mount handling unnecessarily. */
-                (void) sd_event_source_leave_ratelimit(UNIT(m)->manager->mount_event_source);
-
-                m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID;
-                mount_unwatch_control_pid(m);
-
-                r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec));
-                if (r < 0)
-                        goto fail;
-
-                /* Let's enter a distinct state where we just wait for the mount table to catch up */
-                mount_set_state(m, MOUNT_UNMOUNTING_CATCHUP);
-                return;
-        }
-        if (r < 0)
-                log_unit_debug_errno(UNIT(m), r, "Unable to determine if '%s' is a mount point, ignoring: %m", m->where);
-
         /* Start counting our attempts */
         if (!IN_SET(m->state,
                     MOUNT_UNMOUNTING,
                     MOUNT_UNMOUNTING_SIGTERM,
-                    MOUNT_UNMOUNTING_SIGKILL,
-                    MOUNT_UNMOUNTING_CATCHUP))
+                    MOUNT_UNMOUNTING_SIGKILL))
                 m->n_retry_umount = 0;
 
         m->control_command_id = MOUNT_EXEC_UNMOUNT;
@@ -1114,6 +1087,7 @@ static void mount_enter_unmounting(Mount *m) {
                 goto fail;
 
         mount_set_state(m, MOUNT_UNMOUNTING);
+
         return;
 
 fail:
@@ -1286,7 +1260,6 @@ static int mount_start(Unit *u) {
                    MOUNT_UNMOUNTING,
                    MOUNT_UNMOUNTING_SIGTERM,
                    MOUNT_UNMOUNTING_SIGKILL,
-                   MOUNT_UNMOUNTING_CATCHUP,
                    MOUNT_CLEANING))
                 return -EAGAIN;
 
@@ -1316,7 +1289,6 @@ static int mount_stop(Unit *u) {
         case MOUNT_UNMOUNTING:
         case MOUNT_UNMOUNTING_SIGKILL:
         case MOUNT_UNMOUNTING_SIGTERM:
-        case MOUNT_UNMOUNTING_CATCHUP:
                 /* Already on it */
                 return 0;
 
@@ -1582,10 +1554,6 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                 mount_enter_dead_or_mounted(m, f);
                 break;
 
-        case MOUNT_UNMOUNTING_CATCHUP:
-                log_unit_debug(u, "Was notified about control process death, but wasn't expecting it. Ignoring.");
-                break;
-
         case MOUNT_CLEANING:
                 if (m->clean_result == MOUNT_SUCCESS)
                         m->clean_result = f;
@@ -1660,11 +1628,6 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user
                 mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT);
                 break;
 
-        case MOUNT_UNMOUNTING_CATCHUP:
-                log_unit_warning(UNIT(m), "Waiting for unmount notification timed out. Giving up.");
-                mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT);
-                break;
-
         case MOUNT_CLEANING:
                 log_unit_warning(UNIT(m), "Cleaning timed out. killing.");
 
@@ -2121,7 +2084,6 @@ static int mount_process_proc_self_mountinfo(Manager *m) {
                         switch (mount->state) {
 
                         case MOUNT_MOUNTED:
-                        case MOUNT_UNMOUNTING_CATCHUP:
                                 /* This has just been unmounted by somebody else, follow the state change. */
                                 mount_enter_dead(mount, MOUNT_SUCCESS);
                                 break;