Abort tests on mkfs failure
diff mbox

Message ID 1440945888-32220-1-git-send-email-chandan@linux.vnet.ibm.com
State New
Headers show

Commit Message

Chandan Rajendra Aug. 30, 2015, 2:44 p.m. UTC
When creating small Btrfs filesystem instances (i.e. filesystem size <= 1GiB),
mkfs.btrfs can fail if "data block size" does not match "metadata block
size". In such cases this commit aborts the test instead of letting it to
continue and report misleading results.

Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
---
 tests/generic/027 | 3 ++-
 tests/generic/081 | 3 ++-
 tests/generic/085 | 3 ++-
 tests/generic/096 | 3 ++-
 tests/generic/102 | 3 ++-
 tests/generic/204 | 4 ++++
 tests/generic/226 | 3 ++-
 tests/generic/269 | 3 ++-
 tests/generic/270 | 3 ++-
 tests/generic/300 | 3 ++-
 10 files changed, 22 insertions(+), 9 deletions(-)

Comments

Dave Chinner Aug. 30, 2015, 11:20 p.m. UTC | #1
On Sun, Aug 30, 2015 at 08:14:48PM +0530, Chandan Rajendra wrote:
> When creating small Btrfs filesystem instances (i.e. filesystem size <= 1GiB),
> mkfs.btrfs can fail if "data block size" does not match "metadata block
> size". In such cases this commit aborts the test instead of letting it to
> continue and report misleading results.

NACK. Running the test, even with the wrong mkfs parameters, is more
valuable to us than not running the test at all. This has been
explained several times in the past 18 months when similar patches
have been posted.

What you need to do is prevent mkfs from failing in your new corner
case on small filesystems, or fail the test in the the filesystem
specific mkfs routine.

e.g. Filesystems like xfs have custom functions (i.e.
_scratch_mkfs_xfs) that prevent mkfs from failing in weird corner
cases or when conflicting options are given (e.g. test option
conflict with MKFS_OPTIONS given on the CLI).

Or, you can make the btrfs mkfs command in _scratch_mkfs_sized()
specially handle this specific btrfs configuration correctly, and if
it can't then it should fail there.

Cheers,

Dave.
Chandan Rajendra Aug. 31, 2015, 10:41 a.m. UTC | #2
On Monday 31 Aug 2015 09:20:24 Dave Chinner wrote:
> On Sun, Aug 30, 2015 at 08:14:48PM +0530, Chandan Rajendra wrote:
> > When creating small Btrfs filesystem instances (i.e. filesystem size <=
> > 1GiB), mkfs.btrfs can fail if "data block size" does not match "metadata
> > block size". In such cases this commit aborts the test instead of letting
> > it to continue and report misleading results.
> 
> NACK. Running the test, even with the wrong mkfs parameters, is more
> valuable to us than not running the test at all. This has been
> explained several times in the past 18 months when similar patches
> have been posted.
> 
> What you need to do is prevent mkfs from failing in your new corner
> case on small filesystems, or fail the test in the the filesystem
> specific mkfs routine.
> 
> e.g. Filesystems like xfs have custom functions (i.e.
> _scratch_mkfs_xfs) that prevent mkfs from failing in weird corner
> cases or when conflicting options are given (e.g. test option
> conflict with MKFS_OPTIONS given on the CLI).
> 
> Or, you can make the btrfs mkfs command in _scratch_mkfs_sized()
> specially handle this specific btrfs configuration correctly, and if
> it can't then it should fail there.
>
Dave, Thanks for the guidance.

For Btrfs filesystem instances <= 1GiB in size, Btrfs creates "mixed"
block-groups i.e. block groups that are capable of holding data and metadata
blocks. Hence the requirement for such instances to have the same block sizes
for "data" and "metadata". On such mismatched block sizes being provided as
input, mkfs.btrfs exits without making any changes to the underlying disk
(which may actually have a different filesystem created on it).

Based on this reasoning, I will write a patch that detects the conflict and
fails in btrfs specific command in _scratch_mkfs_sized() function.

Patch
diff mbox

diff --git a/tests/generic/027 b/tests/generic/027
index d2e59d6..3aef914 100755
--- a/tests/generic/027
+++ b/tests/generic/027
@@ -65,7 +65,8 @@  _require_scratch
 rm -f $seqres.full
 echo "Silence is golden"
 
-_scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1
+_scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount
 
 echo "Reserve 2M space" >>$seqres.full
diff --git a/tests/generic/081 b/tests/generic/081
index 34da1ac..4044d12 100755
--- a/tests/generic/081
+++ b/tests/generic/081
@@ -63,7 +63,8 @@  mkdir -p $mnt
 
 # make sure there's enough disk space for 256M lv, test for 300M here in case
 # lvm uses some space for metadata
-_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1
+_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1
 $LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1
 # wait for lvcreation to fully complete
diff --git a/tests/generic/085 b/tests/generic/085
index 8398752..ec543be 100755
--- a/tests/generic/085
+++ b/tests/generic/085
@@ -76,7 +76,8 @@  echo "Silence is golden"
 
 size=$((256 * 1024 * 1024))
 size_in_sector=$((size / 512))
-_scratch_mkfs_sized $size >>$seqres.full 2>&1
+_scratch_mkfs_sized $size >>$seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 
 node=$seq-test
 lvdev=/dev/mapper/$node
diff --git a/tests/generic/096 b/tests/generic/096
index daf9981..f9bdc81 100755
--- a/tests/generic/096
+++ b/tests/generic/096
@@ -53,7 +53,8 @@  rm -f $seqres.full
 echo "Silence is golden"
 
 # Use smaller scratch fs to shorten the test time
-_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount
 
 testfile=$SCRATCH_MNT/$seq.$$
diff --git a/tests/generic/102 b/tests/generic/102
index abc3994..d7f598b 100755
--- a/tests/generic/102
+++ b/tests/generic/102
@@ -49,7 +49,8 @@  _require_scratch
 rm -f $seqres.full
 
 dev_size=$((512 * 1024 * 1024))     # 512MB filesystem
-_scratch_mkfs_sized $dev_size >>$seqres.full 2>&1
+_scratch_mkfs_sized $dev_size >>$seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount
 
 for ((i = 0; i < 10; i++)); do
diff --git a/tests/generic/204 b/tests/generic/204
index 42985ab..79de712 100755
--- a/tests/generic/204
+++ b/tests/generic/204
@@ -60,6 +60,10 @@  _scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
 SIZE=`expr 106 \* 1024 \* 1024`
 _scratch_mkfs_sized $SIZE $dbsize 2> /dev/null \
 		| _filter_mkfs 2> $tmp.mkfs > /dev/null
+if [[ ${PIPESTATUS[0]} != 0 ]]; then
+	_fail "mkfs failed"
+fi
+
 _scratch_mount
 
 # Source $tmp.mkfs to get geometry
diff --git a/tests/generic/226 b/tests/generic/226
index 4ad56a5..a83ee0f 100755
--- a/tests/generic/226
+++ b/tests/generic/226
@@ -44,7 +44,8 @@  rm -f $seqres.full
 
 umount $SCRATCH_DEV 2>/dev/null
 echo "--> mkfs 256m filesystem"
-_scratch_mkfs_sized `expr 256 \* 1024 \* 1024` >> $seqres.full 2>&1
+_scratch_mkfs_sized `expr 256 \* 1024 \* 1024` >> $seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount
 
 loops=16
diff --git a/tests/generic/269 b/tests/generic/269
index fe648b5..5ceedba 100755
--- a/tests/generic/269
+++ b/tests/generic/269
@@ -67,7 +67,8 @@  _need_to_be_root
 _require_scratch
 
 rm -f $seqres.full
-_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount
 
 if ! _workout; then
diff --git a/tests/generic/270 b/tests/generic/270
index 255ebf2..6d1c5f1 100755
--- a/tests/generic/270
+++ b/tests/generic/270
@@ -82,7 +82,8 @@  _need_to_be_root
 _require_scratch
 
 rm -f $seqres.full
-_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount "-o usrquota,grpquota"
 chmod 777 $SCRATCH_MNT
 quotacheck -u -g $SCRATCH_MNT 2>/dev/null
diff --git a/tests/generic/300 b/tests/generic/300
index d7523aa..72f1ab3 100755
--- a/tests/generic/300
+++ b/tests/generic/300
@@ -143,7 +143,8 @@  _workout()
 
 _require_fio $fio_config
 
-_scratch_mkfs_sized $FS_SIZE >> $seqres.full 2>&1
+_scratch_mkfs_sized $FS_SIZE >> $seqres.full 2>&1 \
+	|| _fail "mkfs failed"
 _scratch_mount
 
 if ! _workout; then