[PATCH] fstests: common/rc: fix device still mounted error with SCRATCH_DEV_POOL

[PATCH] fstests: common/rc: fix device still mounted error with SCRATCH_DEV_POOL

[Liu Bo]

One of btrfs tests, btrfs/011, uses SCRATCH_DEV_POOL and puts a non-SCRATCH_DEV
device as the first one when doing mkfs, and this makes
_require_scratch{_nocheck} fail to umount $SCRATCH_MNT since it checks mount
point with SCRATCH_DEV only, and for sure it finds nothing to umount and the
following tests complain about 'device still mounted' alike errors.

Introduce a helper to address this special case where both btrfs and scratch
dev pool are in use.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 common/rc | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/common/rc b/common/rc
index 9216efd..eab1bb7 100644
--- a/common/rc
+++ b/common/rc
@@ -1450,6 +1450,29 @@ _check_mounted_on()
 	return 0 # 0 = mounted as expected
 }
 
+_check_scratch_dev_pool_mounted_on()
+{
+	local devname=$1
+	local dev=$2
+	local mntname=$3
+	local mnt=$4
+
+	local mount_rec=`findmnt -rncv -M $mnt -o SOURCE,TARGET`
+	[ -n "$mount_rec" ] || return 1 # 1 = not mounted
+
+	# if it's mounted, make sure mount dev is one of the pool devices
+	for mount_dev in $SCRATCH_DEV_POOL; do
+		if [ "$mount_rec" == "$mount_dev $mnt" ]; then
+			return 0 # 0 = mounted as expected
+		fi
+	done
+
+	echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting"
+	echo "Already mounted result:"
+	echo $mount_rec
+	return 2 # 2 == mounted on wrong mnt
+}
+
 # this test needs a scratch partition - check we're ok & unmount it
 # No post-test check of the device is required. e.g. the test intentionally
 # finishes the test with the filesystem in a corrupt state
@@ -1537,6 +1560,14 @@ _require_scratch_nocheck()
     _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
     local err=$?
     [ $err -le 1 ] || exit 1
+
+    if [ $err -eq 1 -a "$FSTYP" == "btrfs" -a ! -z "$SCRATCH_DEV_POOL" ]
+    then
+	_check_scratch_dev_pool_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
+	err=$?
+	[ $err -le 1 ] || exit 1
+    fi
+
     if [ $err -eq 0 ]
     then
         # if it's mounted, unmount it
-- 
2.5.0

Re: [PATCH] fstests: common/rc: fix device still mounted error with SCRATCH_DEV_POOL

[Eryu Guan]

On Fri, Jan 12, 2018 at 06:04:59PM -0700, Liu Bo wrote:
> One of btrfs tests, btrfs/011, uses SCRATCH_DEV_POOL and puts a non-SCRATCH_DEV
> device as the first one when doing mkfs, and this makes
> _require_scratch{_nocheck} fail to umount $SCRATCH_MNT since it checks mount
> point with SCRATCH_DEV only, and for sure it finds nothing to umount and the
> following tests complain about 'device still mounted' alike errors.
> 
> Introduce a helper to address this special case where both btrfs and scratch
> dev pool are in use.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Hmm, I didn't see this problem, I ran btrfs/011 then another tests that
uses $SCRATCH_DEV, and the second test ran fine too. Can you please
provide more details?

Anyway, I think we should fix btrfs/011 to either not use $SCRATCH_DEV
in replace operations (AFAIK, other btrfs replace tests do this) or
umount all devices before exit. And I noticed btrfs/011 does umount
$SCRATCH_MNT at the end of workout(), so usually all should be fine
(perhaps it would leave a device mounted if interrupted in the middle of
test run, because _cleanup() doesn't do umount).

Thanks,
Eryu

Re: [PATCH] fstests: common/rc: fix device still mounted error with SCRATCH_DEV_POOL

[David Sterba]

On Mon, Jan 15, 2018 at 02:22:28PM +0800, Eryu Guan wrote:
> On Fri, Jan 12, 2018 at 06:04:59PM -0700, Liu Bo wrote:
> > One of btrfs tests, btrfs/011, uses SCRATCH_DEV_POOL and puts a non-SCRATCH_DEV
> > device as the first one when doing mkfs, and this makes
> > _require_scratch{_nocheck} fail to umount $SCRATCH_MNT since it checks mount
> > point with SCRATCH_DEV only, and for sure it finds nothing to umount and the
> > following tests complain about 'device still mounted' alike errors.
> > 
> > Introduce a helper to address this special case where both btrfs and scratch
> > dev pool are in use.
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> 
> Hmm, I didn't see this problem, I ran btrfs/011 then another tests that
> uses $SCRATCH_DEV, and the second test ran fine too. Can you please
> provide more details?
> 
> Anyway, I think we should fix btrfs/011 to either not use $SCRATCH_DEV
> in replace operations (AFAIK, other btrfs replace tests do this) or
> umount all devices before exit. And I noticed btrfs/011 does umount
> $SCRATCH_MNT at the end of workout(), so usually all should be fine
> (perhaps it would leave a device mounted if interrupted in the middle of
> test run, because _cleanup() doesn't do umount).

In my case I saw lots of test failures (btrfs/ 012 068 071 074 116 136
138 152 154 155 ...), some of them repeatedly but not reliably. This
could have been triggered by a patch in my testing branch, but I can't
tell for sure due to the inaccurate fstest checks. The common problem
was that the scratch device appeared as mounted.

We discussed that with Bo, I was suspecting some of our changes that
could theoretically leave some data in flight after umount. Bo found the
potential problems in fstests so I'll redo all the testing again with
updated fstests.