[git commit] Revert "umount: make -d always active, add -D to suppress it"

Denys Vlasenko vda.linux at googlemail.com
Thu Mar 16 16:51:06 UTC 2017


commit: https://git.busybox.net/busybox/commit/?id=a98db793cffb77a8794c854443b8fe12bad98c0a
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

This reverts commit 86a03bee1d3d6990c03bf500836b19ec8a1c1f12.

Since now our "mount -oloop" creates AUTOCLEARed loopdevs, we no longer
need our umount to destroy loopdevs to match the usual util-linux behaviour.

Now this revert fixes another, opposite bug: "explicit" mount /dev/loopN
and then umount must not drop loopdevs!

User complaint is as follows:

It seems LOOP_CLR_FD called on a loop-*partition* removes the mapping of
the whole *device* - which results in the following:

root at LEDE:/# loop=$(losetup -f)
root at LEDE:/# echo ${loop}
/dev/loop2
root at LEDE:/# losetup ${loop} /IMAGE
root at LEDE:/# ls -l ${loop}*
brw-------  1 root root     7,   2 Mar  6 20:09 /dev/loop2
root at LEDE:/# partprobe ${loop}
root at LEDE:/# ls -l ${loop}*
brw-------  1 root  root    7,   2 Mar  6 20:09 /dev/loop2
brw-------  1 root  root  259,   8 Mar  6 21:59 /dev/loop2p1
brw-------  1 root  root  259,   9 Mar  6 21:59 /dev/loop2p2
brw-------  1 root  root  259,  10 Mar  6 21:59 /dev/loop2p3
brw-------  1 root  root  259,  11 Mar  6 21:59 /dev/loop2p4
brw-------  1 root  root  259,  12 Mar  6 21:59 /dev/loop2p5
brw-------  1 root  root  259,  13 Mar  6 21:59 /dev/loop2p6
brw-------  1 root  root  259,  14 Mar  6 21:59 /dev/loop2p7
brw-------  1 root  root  259,  15 Mar  6 21:59 /dev/loop2p8
root at LEDE:/# mount ${loop}p8 /MOUNT       # mount loop partition
root at LEDE:/# losetup -a | grep $loop      # loop dev mapping still there
/dev/loop2: 0 /mnt/IMAGE
root at LEDE:/# strace umount /MOUNT 2> /log # unmount loop partition
root at LEDE:/# losetup -a | grep ${loop}    # loop device mapping is gone
root at LEDE:/# grep -i loop /log
open("/dev/loop2p7", O_RDONLY|O_LARGEFILE) = 3
ioctl(3, LOOP_CLR_FD)                   = 0
root at LEDE:/#

The strace was done to figure out, if maybe umount wrongly ioctl()'s the
parent device instead of the partition - it doesn't.

I already wasn't a fan of umount implicitly removing the mapping in the
first place (as I usually setup and release loop devices with `losetup`
and scripts needed to call umount differently in order to work and
outside busybox).

However taking above (kernel-)behaviour into account - umount calling
ioctl(LOOP_CLR_FD) unconditionally potentially causes some nasty side
effects

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 util-linux/umount.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/util-linux/umount.c b/util-linux/umount.c
index c958fd5..0c50dc9 100644
--- a/util-linux/umount.c
+++ b/util-linux/umount.c
@@ -42,7 +42,7 @@
 //usage:     "\n	-l	Lazy umount (detach filesystem)"
 //usage:     "\n	-f	Force umount (i.e., unreachable NFS server)"
 //usage:	IF_FEATURE_MOUNT_LOOP(
-//usage:     "\n	-D	Don't free loop device even if it has been used"
+//usage:     "\n	-d	Free loop device if it has been used"
 //usage:	)
 //usage:
 //usage:#define umount_example_usage
@@ -68,22 +68,14 @@ static struct mntent *getmntent_r(FILE* stream, struct mntent* result,
 }
 #endif
 
-/* Ignored: -v -t -i
- * bbox always acts as if -d is present.
- * -D can be used to suppress it (bbox extension).
- * Rationale:
- * (1) util-linux's umount does it if "loop=..." is seen in /etc/mtab:
- * thus, on many systems, bare umount _does_ drop loop devices.
- * (2) many users request this feature.
- */
-#define OPTION_STRING           "fldDnra" "vt:i"
+/* ignored: -v -t -i */
+#define OPTION_STRING           "fldnra" "vt:i"
 #define OPT_FORCE               (1 << 0) // Same as MNT_FORCE
 #define OPT_LAZY                (1 << 1) // Same as MNT_DETACH
-//#define OPT_FREE_LOOP           (1 << 2) // -d is assumed always present
-#define OPT_DONT_FREE_LOOP      (1 << 3)
-#define OPT_NO_MTAB             (1 << 4)
-#define OPT_REMOUNT             (1 << 5)
-#define OPT_ALL                 (ENABLE_FEATURE_UMOUNT_ALL ? (1 << 6) : 0)
+#define OPT_FREELOOP            (1 << 2)
+#define OPT_NO_MTAB             (1 << 3)
+#define OPT_REMOUNT             (1 << 4)
+#define OPT_ALL                 (ENABLE_FEATURE_UMOUNT_ALL ? (1 << 5) : 0)
 
 int umount_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int umount_main(int argc UNUSED_PARAM, char **argv)
@@ -206,7 +198,7 @@ int umount_main(int argc UNUSED_PARAM, char **argv)
 		} else {
 			// De-allocate the loop device.  This ioctl should be ignored on
 			// any non-loop block devices.
-			if (ENABLE_FEATURE_MOUNT_LOOP && !(opt & OPT_DONT_FREE_LOOP) && m)
+			if (ENABLE_FEATURE_MOUNT_LOOP && (opt & OPT_FREELOOP) && m)
 				del_loop(m->device);
 			if (ENABLE_FEATURE_MTAB_SUPPORT && !(opt & OPT_NO_MTAB) && m)
 				erase_mtab(m->dir);


More information about the busybox-cvs mailing list