* [PATCH 2/4 v2] fstests: cleanup test btrfs/029
2015-12-23 4:43 [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda fdmanana
@ 2015-12-23 4:43 ` fdmanana
2015-12-23 4:43 ` [PATCH 3/4 v2] fstests: cleanup test btrfs/031 fdmanana
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: fdmanana @ 2015-12-23 4:43 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, Filipe Manana
From: Filipe Manana <fdmanana@suse.com>
The test was using $SCRATCH_MNT as a mountpoint for $SCRATCH_DEV, which
is counter intuitive and not expected by the fstests framework - this
made the test fail after commit 27d077ec0bda (common: use mount/umount
helpers everywhere). So rewrite the test to use the scratch device for
all data and use a test specific directory inside $TEST_DIR to use as a
mount point for a cross mount of $SCRATCH_DEV.
This test was also overriding $seqres.full, through the redirect ">"
operator, if a call to cp failed. Fix that by using instead the operator
">>". Also make the test use the function _mount() instead of calling
the mount program directly.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: First version of the change, introduced in the v2 of the corresponding
patchset.
tests/btrfs/029 | 45 +++++++++++++++++++--------------------------
tests/btrfs/029.out | 12 ++++++------
2 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/tests/btrfs/029 b/tests/btrfs/029
index cdce6e1..87c289b 100755
--- a/tests/btrfs/029
+++ b/tests/btrfs/029
@@ -39,7 +39,6 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_cleanup()
{
- _scratch_unmount &>/dev/null
cd /
rm -f $tmp.*
}
@@ -56,55 +55,49 @@ _supported_os Linux
_require_test
_require_scratch
_require_cp_reflink
+_need_to_be_root
-SOURCE_DIR=$TEST_DIR/test-$seq
-CROSS_DEV_DIR=$SCRATCH_MNT/test-$seq
-# mount point & target for twice-mounted device
-TEST_DIR2=$TEST_DIR/mount2
-DUAL_MOUNT_DIR=$SCRATCH_MNT/test-bis-$seq
-
-rm -rf $SOURCE_DIR
-mkdir $SOURCE_DIR
+reflink_test_dir=$TEST_DIR/test-$seq
+rm -rf $reflink_test_dir
+mkdir $reflink_test_dir
rm -f $seqres.full
_scratch_mkfs > /dev/null 2>&1
-$XFS_IO_PROG -f -c 'pwrite -S 0x61 0 9000' $SOURCE_DIR/original \
- >> $seqres.full
+_scratch_mount
+$XFS_IO_PROG -f -c 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/original >> $seqres.full
_filter_testdirs()
{
_filter_test_dir | _filter_scratch
}
-_create_reflinks_to()
+_create_reflinks()
{
# auto reflink, should fall back to non-reflink
- rm -rf $1; mkdir $1
+ rm -rf $2
echo "reflink=auto:"
- cp --reflink=auto $SOURCE_DIR/original $1/copy
- md5sum $SOURCE_DIR/original | _filter_testdirs
- md5sum $1/copy | _filter_testdirs
+ cp --reflink=auto $1 $2
+ md5sum $1 | _filter_testdirs
+ md5sum $2 | _filter_testdirs
# always reflink, should fail outright
- rm -rf $1; mkdir $1
+ rm -rf $2
echo "reflink=always:"
- cp --reflink=always $SOURCE_DIR/original $1/copyfail > $seqres.full 2>&1 \
- || echo "cp reflink failed"
+ cp --reflink=always $1 $2 >> $seqres.full 2>&1 || echo "cp reflink failed"
# The failed target actually gets created by cp:
- ls $1/copyfail | _filter_testdirs
+ ls $2 | _filter_testdirs
}
echo "test reflinks across different devices"
-_scratch_mount
-_create_reflinks_to $CROSS_DEV_DIR
-_scratch_unmount
+_create_reflinks $SCRATCH_MNT/original $reflink_test_dir/copy
echo "test reflinks across different mountpoints of same device"
-mount $TEST_DEV $SCRATCH_MNT || _fail "Couldn't double-mount $TEST_DEV"
-_create_reflinks_to $DUAL_MOUNT_DIR
-_scratch_unmount
+rm -rf $reflink_test_dir/*
+_mount $SCRATCH_DEV $reflink_test_dir
+_create_reflinks $SCRATCH_MNT/original $reflink_test_dir/copy
+$UMOUNT_PROG $reflink_test_dir
# success, all done
status=0
diff --git a/tests/btrfs/029.out b/tests/btrfs/029.out
index 9390d95..0547d28 100644
--- a/tests/btrfs/029.out
+++ b/tests/btrfs/029.out
@@ -1,15 +1,15 @@
QA output created by 029
test reflinks across different devices
reflink=auto:
-42d69d1a6d333a7ebdf64792a555e392 TEST_DIR/test-029/original
-42d69d1a6d333a7ebdf64792a555e392 SCRATCH_MNT/test-029/copy
+42d69d1a6d333a7ebdf64792a555e392 SCRATCH_MNT/original
+42d69d1a6d333a7ebdf64792a555e392 TEST_DIR/test-029/copy
reflink=always:
cp reflink failed
-SCRATCH_MNT/test-029/copyfail
+TEST_DIR/test-029/copy
test reflinks across different mountpoints of same device
reflink=auto:
-42d69d1a6d333a7ebdf64792a555e392 TEST_DIR/test-029/original
-42d69d1a6d333a7ebdf64792a555e392 SCRATCH_MNT/test-bis-029/copy
+42d69d1a6d333a7ebdf64792a555e392 SCRATCH_MNT/original
+42d69d1a6d333a7ebdf64792a555e392 TEST_DIR/test-029/copy
reflink=always:
cp reflink failed
-SCRATCH_MNT/test-bis-029/copyfail
+TEST_DIR/test-029/copy
--
2.1.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/4 v2] fstests: cleanup test btrfs/031
2015-12-23 4:43 [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda fdmanana
2015-12-23 4:43 ` [PATCH 2/4 v2] fstests: cleanup test btrfs/029 fdmanana
@ 2015-12-23 4:43 ` fdmanana
2015-12-23 4:43 ` [PATCH 4/4 v2] fstests: fix cleanup of test btrfs/003 fdmanana
2016-01-11 3:41 ` [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda Dave Chinner
3 siblings, 0 replies; 5+ messages in thread
From: fdmanana @ 2015-12-23 4:43 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, Filipe Manana
From: Filipe Manana <fdmanana@suse.com>
The test was using $SCRATCH_MNT as a mountpoint for $SCRATCH_DEV, which
is counter intuitive and not expected by the fstests framework - this
made the test fail after commit 27d077ec0bda (common: use mount/umount
helpers everywhere). So rewrite the test to use the scratch device for
all data and use a test specific directory inside $TEST_DIR to use as a
mount point for a cross mount of $SCRATCH_DEV.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: First version of the change, introduced in the v2 of the corresponding
patchset.
tests/btrfs/031 | 102 ++++++++++++++++++++++++++++----------------------------
1 file changed, 51 insertions(+), 51 deletions(-)
diff --git a/tests/btrfs/031 b/tests/btrfs/031
index 0159c95..fe8cd4e 100755
--- a/tests/btrfs/031
+++ b/tests/btrfs/031
@@ -36,11 +36,6 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_cleanup()
{
- _scratch_unmount
- rm -rf $TESTDIR1
- rm -rf $TESTDIR2
- $BTRFS_UTIL_PROG subvolume delete $SUBVOL1 >> $seqres.full
- $BTRFS_UTIL_PROG subvolume delete $SUBVOL2 >> $seqres.full
cd /
rm -f $tmp.*
}
@@ -57,84 +52,89 @@ _supported_os Linux
_require_test
_require_scratch
_require_cp_reflink
+_need_to_be_root
-_checksum_files() {
- for F in file1 file2 file3
- do
- echo "$F:"
- for D in $TESTDIR1 $SCRATCH_MNT $SUBVOL2
- do
- _md5_checksum $D/$F
- done
- done
+_checksum_files()
+{
+ for f in file1 file2 file3; do
+ echo "$f:"
+ for d in $testdir1 $cross_mount_test_dir $subvol2; do
+ _md5_checksum $d/$f
+ done
+ done
}
-TESTDIR1=$TEST_DIR/test-$seq-1
-TESTDIR2=$TEST_DIR/test-$seq-2
-SUBVOL1=$TEST_DIR/subvol-$seq-1
-SUBVOL2=$TEST_DIR/subvol-$seq-2
+testdir1=$SCRATCH_MNT/test-$seq-1
+testdir2=$SCRATCH_MNT/test-$seq-2
+subvol1=$SCRATCH_MNT/subvol-$seq-1
+subvol2=$SCRATCH_MNT/subvol-$seq-2
+cross_mount_test_dir=$TEST_DIR/test-$seq
+
+rm -f $seqres.full
-_scratch_unmount 2>/dev/null
-rm -rf $seqres.full
-rm -rf $TESTDIR1 $TESTDIR2
-$BTRFS_UTIL_PROG subvolume delete $SUBVOL1 >/dev/null 2>&1
-$BTRFS_UTIL_PROG subvolume delete $SUBVOL2 >/dev/null 2>&1
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
-mkdir $TESTDIR1
-mkdir $TESTDIR2
-$BTRFS_UTIL_PROG subvolume create $SUBVOL1 >> $seqres.full
-$BTRFS_UTIL_PROG subvolume create $SUBVOL2 >> $seqres.full
-_mount -t btrfs -o subvol=subvol-$seq-1 $TEST_DEV $SCRATCH_MNT
+mkdir $testdir1
+mkdir $testdir2
+$BTRFS_UTIL_PROG subvolume create $subvol1 >> $seqres.full
+$BTRFS_UTIL_PROG subvolume create $subvol2 >> $seqres.full
+rm -rf $cross_mount_test_dir
+mkdir $cross_mount_test_dir
+
+_mount -t btrfs -o subvol=subvol-$seq-1 $SCRATCH_DEV $cross_mount_test_dir
echo "Create initial files"
-# TESTDIR1/file1 is very small and will be inlined
-$XFS_IO_PROG -f -c 'pwrite -S 0x61 0 10' $TESTDIR1/file1 \
+# #testdir1/file1 is very small and will be inlined
+$XFS_IO_PROG -f -c 'pwrite -S 0x61 0 10' $testdir1/file1 \
>> $seqres.full
-$XFS_IO_PROG -f -c 'pwrite -S 0x62 0 13000' $SCRATCH_MNT/file2 \
+$XFS_IO_PROG -f -c 'pwrite -S 0x62 0 13000' $cross_mount_test_dir/file2 \
>> $seqres.full
-$XFS_IO_PROG -f -c 'pwrite -S 0x63 0 17000' $SUBVOL2/file3 \
+$XFS_IO_PROG -f -c 'pwrite -S 0x63 0 17000' $subvol2/file3 \
>> $seqres.full
echo "Create reflinks to the initial files on other subvolumes"
-cp --reflink=always $TESTDIR1/file1 $SUBVOL1
-cp --reflink=always $TESTDIR1/file1 $SUBVOL2
-cp --reflink=always $SUBVOL1/file2 $TESTDIR1/
-cp --reflink=always $SUBVOL1/file2 $SUBVOL2
-cp --reflink=always $SUBVOL2/file3 $TESTDIR1/
-cp --reflink=always $SUBVOL2/file3 $SUBVOL1
+cp --reflink=always $testdir1/file1 $subvol1
+cp --reflink=always $testdir1/file1 $subvol2
+cp --reflink=always $subvol1/file2 $testdir1/
+cp --reflink=always $subvol1/file2 $subvol2
+cp --reflink=always $subvol2/file3 $testdir1/
+cp --reflink=always $subvol2/file3 $subvol1
echo "Verify the reflinks"
-_verify_reflink $SCRATCH_MNT/file2 $TESTDIR1/file2
-_verify_reflink $SCRATCH_MNT/file2 $SUBVOL2/file2
-_verify_reflink $SUBVOL2/file3 $TESTDIR1/file3
-_verify_reflink $SUBVOL2/file3 $SCRATCH_MNT/file3
+_verify_reflink $cross_mount_test_dir/file2 $testdir1/file2
+_verify_reflink $cross_mount_test_dir/file2 $subvol2/file2
+_verify_reflink $subvol2/file3 $testdir1/file3
+_verify_reflink $subvol2/file3 $cross_mount_test_dir/file3
echo "Verify the file contents:"
_checksum_files
echo -e "---\nOverwrite some files with new content"
-$XFS_IO_PROG -c 'pwrite -S 0x64 0 20' $TESTDIR1/file1 >> $seqres.full
-$XFS_IO_PROG -c 'pwrite -S 0x66 0 21000' $SUBVOL2/file2 >> $seqres.full
-$XFS_IO_PROG -c 'pwrite -S 0x65 5000 5000' $SCRATCH_MNT/file3 \
+$XFS_IO_PROG -c 'pwrite -S 0x64 0 20' $testdir1/file1 >> $seqres.full
+$XFS_IO_PROG -c 'pwrite -S 0x66 0 21000' $subvol2/file2 >> $seqres.full
+$XFS_IO_PROG -c 'pwrite -S 0x65 5000 5000' $cross_mount_test_dir/file3 \
>> $seqres.full
echo -n "Verify that non-overwritten reflinks "
echo "still have the same data blocks"
-_verify_reflink $TESTDIR1/file2 $SCRATCH_MNT/file2
-_verify_reflink $TESTDIR1/file3 $SUBVOL2/file3
+_verify_reflink $testdir1/file2 $cross_mount_test_dir/file2
+_verify_reflink $testdir1/file3 $subvol2/file3
echo "Verify the file contents:"
_checksum_files
echo -e "---\nShuffle files between directories"
-mv $TESTDIR1/file* $TESTDIR2
-mv $SCRATCH_MNT/file* $TESTDIR1/
-mv $SUBVOL2/file* $SCRATCH_MNT/
-mv $TESTDIR2/file* $SUBVOL2/
+mv $testdir1/file* $testdir2
+mv $cross_mount_test_dir/file* $testdir1/
+mv $subvol2/file* $cross_mount_test_dir/
+mv $testdir2/file* $subvol2/
# No _verify_reflink here as data is copied when moving files
# between subvols
echo "Verify the file contents:"
_checksum_files
+$UMOUNT_PROG $cross_mount_test_dir
+
# success, all done
status=0
exit
--
2.1.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/4 v2] fstests: fix cleanup of test btrfs/003
2015-12-23 4:43 [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda fdmanana
2015-12-23 4:43 ` [PATCH 2/4 v2] fstests: cleanup test btrfs/029 fdmanana
2015-12-23 4:43 ` [PATCH 3/4 v2] fstests: cleanup test btrfs/031 fdmanana
@ 2015-12-23 4:43 ` fdmanana
2016-01-11 3:41 ` [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda Dave Chinner
3 siblings, 0 replies; 5+ messages in thread
From: fdmanana @ 2015-12-23 4:43 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, Filipe Manana
From: Filipe Manana <fdmanana@suse.com>
If the test fails after removing a device and before adding it back, it
attempts to add back the device in its _cleanup() function. However this
is broken because the device identifier is stored in a variable local to
the function _test_replace() and not in a global variable. So make the
variable global instead of local.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: No changes from v1. Only added to V2 of the corresponding patchset.
tests/btrfs/003 | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/btrfs/003 b/tests/btrfs/003
index 353cb48..ae7e377 100755
--- a/tests/btrfs/003
+++ b/tests/btrfs/003
@@ -29,6 +29,7 @@ here=`pwd`
tmp=/tmp/$$
status=1 # failure is the default!
dev_removed=0
+removed_dev_htl=""
trap "_cleanup; exit \$status" 0 1 2 3 15
_cleanup()
@@ -37,7 +38,7 @@ _cleanup()
rm -f $tmp.*
if [ $dev_removed == 1 ]; then
_scratch_unmount
- _devmgt_add "${DEVHTL}"
+ _devmgt_add "${removed_dev_htl}"
fi
}
@@ -125,7 +126,6 @@ _test_replace()
local n=${#devs[@]}
local ds
local d
- local DEVHTL=""
# exclude the first and the last disk in the disk pool
n=$(($n-1))
@@ -142,10 +142,10 @@ _test_replace()
# retrive the HTL for this scsi disk
d=`echo $ds|cut -d"/" -f3`
- DEVHTL=`ls -l /sys/class/block/${d} | rev | cut -d "/" -f 3 | rev`
+ removed_dev_htl=`ls -l /sys/class/block/${d} | rev | cut -d "/" -f 3 | rev`
#fail disk
- _devmgt_remove ${DEVHTL} $ds
+ _devmgt_remove ${removed_dev_htl} $ds
dev_removed=1
$BTRFS_UTIL_PROG filesystem show $SCRATCH_DEV | grep "Some devices missing" >> $seqres.full || _fail \
@@ -162,7 +162,7 @@ _test_replace()
# cleaup. add the removed disk
_scratch_unmount
- _devmgt_add "${DEVHTL}"
+ _devmgt_add "${removed_dev_htl}"
dev_removed=0
}
--
2.1.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda
2015-12-23 4:43 [PATCH 1/4 v2] fstests: fix btrfs test failures after commit 27d077ec0bda fdmanana
` (2 preceding siblings ...)
2015-12-23 4:43 ` [PATCH 4/4 v2] fstests: fix cleanup of test btrfs/003 fdmanana
@ 2016-01-11 3:41 ` Dave Chinner
3 siblings, 0 replies; 5+ messages in thread
From: Dave Chinner @ 2016-01-11 3:41 UTC (permalink / raw)
To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana
On Wed, Dec 23, 2015 at 04:43:12AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> 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@suse.com>
> ---
>
> 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@fromorbit.com
^ permalink raw reply [flat|nested] 5+ messages in thread