umount: (recursive) don't call umount(2) for already unmounted targets

In the umount --recursive we follow entries from mountinfo, but the
entries maybe already obsolete. Especially if the hierarchy of the
mountpoints contains shared subtrees and umount(2) for one entry may
generate umount for some other entry too.

Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/libmount/src/tab.c b/libmount/src/tab.c
index e5eb1063eaa1ff48c42f394f02a19abc4d124ea0..5287da4a50d9bd5a565417cf2899377d2eca3201 100644 (file)
--- a/libmount/src/tab.c
+++ b/libmount/src/tab.c
@@ -901,8 +901,6 @@ static int is_mountinfo(struct libmnt_table *tb)
  *
  * This function is designed mostly for "mount -a".
  *
- * TODO: check for loopdev (see mount/mount.c is_fstab_entry_mounted().
- *
  * Returns: 0 or 1
  */
 int mnt_table_is_fs_mounted(struct libmnt_table *tb, struct libmnt_fs *fstab_fs)
@@ -918,8 +916,13 @@ int mnt_table_is_fs_mounted(struct libmnt_table *tb, struct libmnt_fs *fstab_fs)
        assert(tb);
        assert(fstab_fs);
 
-       if (mnt_fs_is_swaparea(fstab_fs) || mnt_table_get_nents(tb) == 0)
+       DBG(FS, mnt_debug_h(fstab_fs, "is FS mounted? [target=%s]",
+                               mnt_fs_get_target(fstab_fs)));
+
+       if (mnt_fs_is_swaparea(fstab_fs) || mnt_table_get_nents(tb) == 0) {
+               DBG(FS, mnt_debug_h(fstab_fs, "- ignore (swap or no data)"));
                return 0;
+       }
 
        if (is_mountinfo(tb)) {
                /* @tb is mountinfo, so we can try to use fs-roots */
@@ -942,9 +945,10 @@ int mnt_table_is_fs_mounted(struct libmnt_table *tb, struct libmnt_fs *fstab_fs)
 
        tgt = mnt_fs_get_target(fstab_fs);
 
-       if (!tgt || !src)
+       if (!tgt || !src) {
+               DBG(FS, mnt_debug_h(fstab_fs, "- ignore (no source/target)"));
                goto done;
-
+       }
        mnt_reset_iter(&itr, MNT_ITER_FORWARD);
 
        while (mnt_table_next_fs(tb, &itr, &fs) == 0) {

diff --git a/sys-utils/umount.c b/sys-utils/umount.c
index 1cd2ebf6718d0a4bb6afbaa8d68b2acd18173a5d..b21e39674029533cdffa9dc8b8c52da2bcd616cb 100644 (file)
--- a/sys-utils/umount.c
+++ b/sys-utils/umount.c
@@ -289,11 +289,6 @@ static int umount_one(struct libmnt_context *cxt, const char *spec)
        if (!spec)
                return MOUNT_EX_SOFTWARE;
 
-       /* We have to reset the context to make this function and the
-        * context re-usable more than once (for example in --recursive)
-        */
-       mnt_reset_context(cxt);
-
        if (mnt_context_set_target(cxt, spec))
                err(MOUNT_EX_SYSERR, _("failed to set umount target"));
 
@@ -310,7 +305,7 @@ static int umount_one(struct libmnt_context *cxt, const char *spec)
 static int umount_do_recurse(struct libmnt_context *cxt,
                struct libmnt_table *tb, struct libmnt_fs *parent)
 {
-       int rc;
+       int rc, mounted = 0;
        struct libmnt_fs *child;
        const char *target = mnt_fs_get_target(parent);
        struct libmnt_iter *itr = mnt_new_iter(MNT_ITER_BACKWARD);
@@ -334,7 +329,33 @@ static int umount_do_recurse(struct libmnt_context *cxt,
                        goto done;
        }
 
-       rc = umount_one(cxt, target);
+
+       /*
+        * Let's check if the pointpoint is still mounted -- for example with
+        * shared subtrees maybe the mountpoint already unmounted by any
+        * previous umount(2) call.
+        *
+        * Note that here we a little duplicate code from umount_one() and
+        * mnt_context_umount(). It's no problem to call
+        * mnt_context_prepare_umount() more than once. This solution is better
+        * than directly call mnt_context_is_fs_mounted(), because libmount is
+        * able to optimize mtab usage by mnt_context_set_tabfilte().
+        */
+       if (mnt_context_set_target(cxt, mnt_fs_get_target(parent)))
+               err(MOUNT_EX_SYSERR, _("failed to set umount target"));
+
+       rc = mnt_context_prepare_umount(cxt);
+       if (!rc)
+               rc = mnt_context_is_fs_mounted(cxt, parent, &mounted);
+       if (mounted)
+               rc = umount_one(cxt, target);
+       else {
+               if (rc)
+                       rc = mk_exit_code(cxt, rc);     /* error */
+               else
+                       rc = MOUNT_EX_SUCCESS;          /* alredy unmounted */
+               mnt_reset_context(cxt);
+       }
 done:
        mnt_free_iter(itr);
        return rc;
@@ -345,6 +366,9 @@ static int umount_recursive(struct libmnt_context *cxt, const char *spec)
        struct libmnt_table *tb;
        int rc;
 
+       /* it's always real mountpoint, don't assume that the target maybe a device */
+       mnt_context_disable_swapmatch(cxt, 1);
+
        tb = mnt_new_table();
        if (!tb)
                err(MOUNT_EX_SYSERR, _("libmount table allocation failed"));