On Wed, Dec 23, 2015 at 04:43:12AM +0000, fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> 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 <fdmanana@xxxxxxxx>
> ---
>
> 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.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
