]> git.apps.os.sepia.ceph.com Git - xfstests-dev.git/commitdiff
common/rc: revert recursive unmount in _clear_mount_stack
authorDarrick J. Wong <djwong@kernel.org>
Mon, 3 Feb 2025 22:00:26 +0000 (14:00 -0800)
committerZorro Lang <zlang@kernel.org>
Sun, 16 Feb 2025 11:25:57 +0000 (19:25 +0800)
Zorro complained about the following regression in generic/589 that I
can't reproduce here on Debian 12:

>  --- /dev/fd/63  2025-01-08 19:53:49.621421512 -0500
>  +++ generic/589.out.bad 2025-01-08 19:53:49.244099127 -0500
>  @@ -30,6 +30,7 @@
>   mpC SCRATCH_DEV
>   mpC SCRATCH_DEV
>   ======
>  +umount: /mnt/xfstests/test/589-dst: not mounted
>
> It fails on generic/589 -> end_test -> _clear_mount_stack
>
>   ...
>   + end_test
>   + _clear_mount_stack
>   + '[' -n '/mnt/test/589-dst/201227_mpC /mnt/test/589-src/201227_mpA /mnt/test/589-dst /mnt/test/589-dst /mnt/test/589-src' ']'
>   + _unmount -R /mnt/test/589-dst/201227_mpC /mnt/test/589-src/201227_mpA /mnt/test/589-dst /mnt/test/589-dst /mnt/test/589-src
>   + local outlog=/tmp/201227.201227.umount
>   + local errlog=/tmp/201227.201227.umount.err
>   + rm -f /tmp/201227.201227.umount /tmp/201227.201227.umount.err
>   + /usr/bin/umount -R /mnt/test/589-dst/201227_mpC /mnt/test/589-src/201227_mpA /mnt/test/589-dst /mnt/test/589-dst /mnt/test/589-src
>   + local res=1
>   + '[' -s /tmp/201227.201227.umount ']'
>   + '[' -s /tmp/201227.201227.umount.err ']'
>   + cat /tmp/201227.201227.umount.err
>   + cat /tmp/201227.201227.umount.err
>   umount: /mnt/test/589-dst: not mounted
>
> The _get_mount already save all mount points into MOUNTED_POINT_STACK,
> MOUNTED_POINT_STACK="/mnt/test/589-dst/201227_mpC /mnt/test/589-src/201227_mpA /mnt/test/589-dst /mnt/test/589-dst /mnt/test/589-src"
>
> '-/mnt/test                                     /dev/sda5                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>   |-/mnt/test/589-src                           /dev/sda6                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>   | '-/mnt/test/589-src/223262_mpA              /dev/sda6                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>   '-/mnt/test/589-dst                           /dev/sda6                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>     |-/mnt/test/589-dst                         /dev/sda6                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>     | '-/mnt/test/589-dst/223262_mpC            /dev/sda6                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>     '-/mnt/test/589-dst/223262_mpC              /dev/sda6                                xfs        rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=64k,sunit=128,swidth=256,noquota
>
> Orignal _clear_mount_stack trys to umount all of them. But Dave gave -R option to
> umount command, so
>   `umount -R /mnt/test/589-dst/201227_mpC` and `umount -R /mnt/test/589-src/201227_mpA`
> already umount /mnt/test/589-dst and /mnt/test/589-src. recursively.
> Then `umount -R /mnt/test/589-dst` shows "not mounted".

I /think/ this is a result of commit 4c6bc4565105e6 performing this
"conversion" in _clear_mount_stack:

- $UMOUNT_PROG $MOUNTED_POINT_STACK
+ _unmount -R $MOUNTED_POINT_STACK

This is not a 1:1 conversion here -- previously there was no
-R(ecursive) unmount option, and now there is.  It looks as though
umount parses /proc/self/mountinfo to figure out what to unmount, and
maybe on Zorro's system it balks if the argument passed is not present
in that file?  Debian 12's umount doesn't care.

Regardless, there was no justification for this change in behavior that
was buried in what is otherwise a hoist patch, so revert it.  The author
can resubmit the change with proper documentation.

Cc: <fstests@vger.kernel.org> # v2024.12.08
Fixes: 4c6bc4565105e6 ("fstests: clean up mount and unmount operations")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Zorro Lang <zlang@kernel.org>
common/rc

index 6840bb1dfa1001386d98d50133585f11b9571a41..7ce0a9d2749e20b6936e2a090dfc78dd43c5af14 100644 (file)
--- a/common/rc
+++ b/common/rc
@@ -334,7 +334,7 @@ _put_mount()
 _clear_mount_stack()
 {
        if [ -n "$MOUNTED_POINT_STACK" ]; then
-               _unmount -R $MOUNTED_POINT_STACK
+               _unmount $MOUNTED_POINT_STACK
        fi
        MOUNTED_POINT_STACK=""
 }