[1/4,v2] fstests: fix btrfs test failures after commit 27d077ec0bda

Commit Message

Filipe Manana Dec. 23, 2015, 4:43 a.m. UTC

From: Filipe Manana <[e-mail]>

Commit 27d077ec0bda (common: use mount/umount helpers everywhere) made
a few btrfs test fail for 2 different reasons:

1) Some tests (btrfs/029 and btrfs/031) use $SCRATCH_MNT as a mount
   point for some subvolume created in $TEST_DEV, therefore calling
   _scratch_unmount does not work as it passes $SCRATCH_DEV as the
   argument to the umount program. This is intentional to test reflinks
   accross different mountpoints of the same filesystem but for different
   subvolumes;

2) For multiple devices filesystems (btrfs/003 and btrfs/011) that test
   the device replace feature, we need to unmount using the mount path
   ($SCRATCH_MNT) because unmounting using one of the devices as an
   argument ($SCRATCH_DEV) does not always work - after replace operations
   we get in /proc/mounts a device other than $SCRATCH_DEV associated
   with the mount point $SCRATCH_MNT (this is mentioned in a comment at
   btrfs/011 for example), so we need to pass that other device to the
   umount program or pass it the mount point.

Using $SCRATCH_MNT as a mountpoint for a device other than $SCRATCH_DEV is
misleading, but that's a different problem that existed long before and
this change attempts only to fix the regression from 27d077ec0bda, leaving
such cleanups to later.
Fix this by making _sctatch_unmmount() pass $SCRATCH_MNT to umount instead
of $SCRATCH_DEV.

Signed-off-by: Filipe Manana <[e-mail]>
---

V2: Change _sctatch_unmount() to pass $SCRATCH_MNT as the argument to the
    umount program instead of $SCRATCH_DEV. This makes the btrfs tests
    pass again.

 common/rc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dave Chinner Jan. 11, 2016, 3:41 a.m. UTC | #1

On Wed, Dec 23, 2015 at 04:43:12AM +0000, [e-mail] wrote:
> From: Filipe Manana <[e-mail]>
> 
> Commit 27d077ec0bda (common: use mount/umount helpers everywhere) made
> a few btrfs test fail for 2 different reasons:
> 
> 1) Some tests (btrfs/029 and btrfs/031) use $SCRATCH_MNT as a mount
>    point for some subvolume created in $TEST_DEV, therefore calling
>    _scratch_unmount does not work as it passes $SCRATCH_DEV as the
>    argument to the umount program. This is intentional to test reflinks
>    accross different mountpoints of the same filesystem but for different
>    subvolumes;
> 
> 2) For multiple devices filesystems (btrfs/003 and btrfs/011) that test
>    the device replace feature, we need to unmount using the mount path
>    ($SCRATCH_MNT) because unmounting using one of the devices as an
>    argument ($SCRATCH_DEV) does not always work - after replace operations
>    we get in /proc/mounts a device other than $SCRATCH_DEV associated
>    with the mount point $SCRATCH_MNT (this is mentioned in a comment at
>    btrfs/011 for example), so we need to pass that other device to the
>    umount program or pass it the mount point.
> 
> Using $SCRATCH_MNT as a mountpoint for a device other than $SCRATCH_DEV is
> misleading, but that's a different problem that existed long before and
> this change attempts only to fix the regression from 27d077ec0bda, leaving
> such cleanups to later.
> Fix this by making _sctatch_unmmount() pass $SCRATCH_MNT to umount instead
> of $SCRATCH_DEV.
> 
> Signed-off-by: Filipe Manana <[e-mail]>
> ---
> 
> V2: Change _sctatch_unmount() to pass $SCRATCH_MNT as the argument to the
>     umount program instead of $SCRATCH_DEV. This makes the btrfs tests
>     pass again.
> 
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index d33e3fb..16a47fe 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -319,7 +319,7 @@ _scratch_unmount()
>  	if [ "$FSTYP" == "overlay" ]; then
>  		_overlay_scratch_unmount
>  	else
> -		$UMOUNT_PROG $SCRATCH_DEV
> +		$UMOUNT_PROG $SCRATCH_MNT
>  	fi

This causes generic/050 to fail (and there's probably others, too):

@@ -15,7 +15,7 @@
     mount: SCRATCH_DEV is write-protected, mounting read-only
     mount: cannot mount SCRATCH_DEV read-only
     unmounting read-only filesystem
    -umount: SCRATCH_DEV: not mounted
    +umount: SCRATCH_MNT: not mounted
     mounting filesystem with -o norecovery on a read-only device:
     mount: SCRATCH_DEV is write-protected, mounting read-only
    ...

And, looking at the change for a second or two, the above code is
obviously wrong. Have a look at _overlay_scratch_unmount() and have
a think about what we normally do when multiple fileystem types need
a non-standard behaviour for a generic operation?

Hint: it starts with 'case "$FSTYP" in'....

Cheers,

Dave.

Patch

diff --git a/common/rc b/common/rc
index d33e3fb..16a47fe 100644
--- a/common/rc
+++ b/common/rc
@@ -319,7 +319,7 @@  _scratch_unmount()
 	if [ "$FSTYP" == "overlay" ]; then
 		_overlay_scratch_unmount
 	else
-		$UMOUNT_PROG $SCRATCH_DEV
+		$UMOUNT_PROG $SCRATCH_MNT
 	fi
 }