[PATCH] fstests: Update generic/077 for newest version of btrfs progs

[PATCH] fstests: Update generic/077 for newest version of btrfs progs

[Zhaolei]

From: Zhao Lei <zhaolei@cn.fujitsu.com>

generic/077 fails on btrfs progs v4.3:
 # ./check generic/077
 FSTYP         -- btrfs
 PLATFORM      -- Linux/x86_64 lenovo 4.4.0-rc2_HEAD_1ec218373b8ebda821aec00bb156a9c94fad9cd4_
 MKFS_OPTIONS  -- /dev/sdb6
 MOUNT_OPTIONS -- /dev/sdb6 /var/ltf/tester/scratch_mnt

 generic/077 344s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//generic/077.out.bad)
     --- tests/generic/077.out   2015-11-23 17:06:27.144983112 +0800
     +++ /var/lib/xfstests/results//generic/077.out.bad  2015-11-23 17:41:25.187062895 +0800
     @@ -1,7 +1,5 @@
      QA output created by 077
      *** create filesystem
     -*** set default ACL
     -*** populate filesystem, pass #1
     -*** populate filesystem, pass #2
     -*** all done
     +mkfs failed
     +(see /var/lib/xfstests/results//generic/077.full for details)
      *** unmount
 Ran: generic/077
 Failures: generic/077
 Failed 1 of 1 tests

Reason:
 btrfs progs v4.3 use non-mixed blockgroup for small volume as default,
 it need at least 100M to build a filesystem.

Fix:
 We can force mixed block group for btrfs, or increase filesystem
 size to btrfs's least requirement to make test works, the first
 way create a non-common filesystem in btrfs case, so this patch
 use the second way.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 tests/generic/077 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/077 b/tests/generic/077
index 8405b02..c646997 100755
--- a/tests/generic/077
+++ b/tests/generic/077
@@ -65,7 +65,7 @@ rm -f $seqres.full
 umount $SCRATCH_DEV >/dev/null 2>&1
 echo "*** MKFS ***"                         >>$seqres.full
 echo ""                                     >>$seqres.full
-SIZE=`expr 50 \* 1024 \* 1024`
+SIZE=`expr 100 \* 1024 \* 1024`
 _scratch_mkfs_sized $SIZE                   >>$seqres.full 2>&1 \
 	|| _fail "mkfs failed"
 _scratch_mount                              >>$seqres.full 2>&1 \
-- 
1.8.5.1

Re: [PATCH] fstests: Update generic/077 for newest version of btrfs progs

[Dave Chinner]

On Mon, Nov 23, 2015 at 05:55:58PM +0800, Zhaolei wrote:
> From: Zhao Lei <zhaolei@cn.fujitsu.com>
> 
> generic/077 fails on btrfs progs v4.3:
>  # ./check generic/077
>  FSTYP         -- btrfs
>  PLATFORM      -- Linux/x86_64 lenovo 4.4.0-rc2_HEAD_1ec218373b8ebda821aec00bb156a9c94fad9cd4_
>  MKFS_OPTIONS  -- /dev/sdb6
>  MOUNT_OPTIONS -- /dev/sdb6 /var/ltf/tester/scratch_mnt
> 
>  generic/077 344s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//generic/077.out.bad)
>      --- tests/generic/077.out   2015-11-23 17:06:27.144983112 +0800
>      +++ /var/lib/xfstests/results//generic/077.out.bad  2015-11-23 17:41:25.187062895 +0800
>      @@ -1,7 +1,5 @@
>       QA output created by 077
>       *** create filesystem
>      -*** set default ACL
>      -*** populate filesystem, pass #1
>      -*** populate filesystem, pass #2
>      -*** all done
>      +mkfs failed
>      +(see /var/lib/xfstests/results//generic/077.full for details)
>       *** unmount
>  Ran: generic/077
>  Failures: generic/077
>  Failed 1 of 1 tests
> 
> Reason:
>  btrfs progs v4.3 use non-mixed blockgroup for small volume as default,
>  it need at least 100M to build a filesystem.

<sigh>

btrfs got broken again.

> Fix:
>  We can force mixed block group for btrfs, or increase filesystem
>  size to btrfs's least requirement to make test works, the first
>  way create a non-common filesystem in btrfs case, so this patch
>  use the second way.

No. This is a clear mkfs.btrfs regression, so the mkfs.btrfs default
behaviour needs to be changed back to something that works for small
filesystems.  Anyone who makes a <100MB btrfs filesytsem is going to
need to use that mixed block group option, so that needs to be what
the test uses here.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] fstests: Update generic/077 for newest version of btrfs progs

[Qu Wenruo]

Dave Chinner wrote on 2015/11/24 15:41 +1100:
> On Mon, Nov 23, 2015 at 05:55:58PM +0800, Zhaolei wrote:
>> From: Zhao Lei <zhaolei@cn.fujitsu.com>
>>
>> generic/077 fails on btrfs progs v4.3:
>>   # ./check generic/077
>>   FSTYP         -- btrfs
>>   PLATFORM      -- Linux/x86_64 lenovo 4.4.0-rc2_HEAD_1ec218373b8ebda821aec00bb156a9c94fad9cd4_
>>   MKFS_OPTIONS  -- /dev/sdb6
>>   MOUNT_OPTIONS -- /dev/sdb6 /var/ltf/tester/scratch_mnt
>>
>>   generic/077 344s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//generic/077.out.bad)
>>       --- tests/generic/077.out   2015-11-23 17:06:27.144983112 +0800
>>       +++ /var/lib/xfstests/results//generic/077.out.bad  2015-11-23 17:41:25.187062895 +0800
>>       @@ -1,7 +1,5 @@
>>        QA output created by 077
>>        *** create filesystem
>>       -*** set default ACL
>>       -*** populate filesystem, pass #1
>>       -*** populate filesystem, pass #2
>>       -*** all done
>>       +mkfs failed
>>       +(see /var/lib/xfstests/results//generic/077.full for details)
>>        *** unmount
>>   Ran: generic/077
>>   Failures: generic/077
>>   Failed 1 of 1 tests
>>
>> Reason:
>>   btrfs progs v4.3 use non-mixed blockgroup for small volume as default,
>>   it need at least 100M to build a filesystem.
>
> <sigh>
>
> btrfs got broken again.
>
>> Fix:
>>   We can force mixed block group for btrfs, or increase filesystem
>>   size to btrfs's least requirement to make test works, the first
>>   way create a non-common filesystem in btrfs case, so this patch
>>   use the second way.
>
> No. This is a clear mkfs.btrfs regression, so the mkfs.btrfs default
> behaviour needs to be changed back to something that works for small
> filesystems.  Anyone who makes a <100MB btrfs filesytsem is going to
> need to use that mixed block group option, so that needs to be what
> the test uses here.
>
> Cheers,
>
> Dave.
>
Hi Dave,

I'm a little curious about fstests support for make small fs.

It's not strange that all filesystems have a requirement on the 
filesystem size, for btrfs it's a little larger than normal fs anyway.

Yes, this bug reported by Zhao is definitely a regression of mkfs.btrfs, 
and I'll enhance the size checking part of mkfs.btrfs.

But I hope fstests can have a generic API to make small fs other than 
current mkfs_sized without any good check on filesystem size.

What about the following idea?
1) Do normal mkfs_size
    But save the error output (it's saved anyway)

2) If mkfs failed, check mkfs dependent output
    For example, for mkfs.xfs, it will output like "agsize (256 blocks)
    too small, need at least 4096 blocks" and we can calculate the
    fs needs to be at least 16M for xfs.

    For btrfs, mkfs.btrfs will also output things like "Minimum size for
    each btrfs device is 41943040." and we can use it to create a small
    fs. (Although the output is totally wrong for non-mixed-bg case)

3) If mkfs didn't provide that size, use a fallback value
    Like old mkfs.btrfs, which doesn't provide such thing (and will just
    crash), use a per-file-system value as fallback.

Personally speaking, if the filesystem is a little larger than 
mkfs_sized parameter, it should not affect the testcases much, will only 
increase the time needed.

Thanks,
Qu

-- 
This message has been scanned for viruses and
dangerous content by Fujitsu, and is believed to be clean.

Re: [PATCH] fstests: Update generic/077 for newest version of btrfs progs

[Dave Chinner]

On Wed, Nov 25, 2015 at 09:22:24AM +0800, Qu Wenruo wrote:
> Dave Chinner wrote on 2015/11/24 15:41 +1100:
> >On Mon, Nov 23, 2015 at 05:55:58PM +0800, Zhaolei wrote:
> >>From: Zhao Lei <zhaolei@cn.fujitsu.com>
> >>
> >>generic/077 fails on btrfs progs v4.3:
....
> >>      +mkfs failed
> >>      +(see /var/lib/xfstests/results//generic/077.full for details)
> >>       *** unmount
> >>  Ran: generic/077
> >>  Failures: generic/077
> >>  Failed 1 of 1 tests
> >>
> >>Reason:
> >>  btrfs progs v4.3 use non-mixed blockgroup for small volume as default,
> >>  it need at least 100M to build a filesystem.
> >
> ><sigh>
> >
> >btrfs got broken again.
> >
> >>Fix:
> >>  We can force mixed block group for btrfs, or increase filesystem
> >>  size to btrfs's least requirement to make test works, the first
> >>  way create a non-common filesystem in btrfs case, so this patch
> >>  use the second way.
> >
> >No. This is a clear mkfs.btrfs regression, so the mkfs.btrfs default
> >behaviour needs to be changed back to something that works for small
> >filesystems.  Anyone who makes a <100MB btrfs filesytsem is going to
> >need to use that mixed block group option, so that needs to be what
> >the test uses here.
> 
> I'm a little curious about fstests support for make small fs.
> 
> It's not strange that all filesystems have a requirement on the
> filesystem size, for btrfs it's a little larger than normal fs
> anyway.

No it isn't. btrfs can quite easily make a 50MB filesystem.
mkfs.btrfs got changed, and broke it's ability to create a 50MB
filesystem.

> Yes, this bug reported by Zhao is definitely a regression of
> mkfs.btrfs, and I'll enhance the size checking part of mkfs.btrfs.
> 
> But I hope fstests can have a generic API to make small fs other
> than current mkfs_sized without any good check on filesystem size.

Why? You're trying to invent a solution to a problem that doesn't
exist.

If a filesystem in an existing has become too small in a test to
exercise the necessary functionality, then it needs to be discussed
on the list, not worked around by trying to guess what size a
filesystem might need. Indeed, the test may very weel require a
specifically sized filesystem to exercise the particular code path
that a bug existed in, and so silently changing the filesystem size
because someone broke mkfs is exactly the wrong thing to be doing.

> Personally speaking, if the filesystem is a little larger than
> mkfs_sized parameter, it should not affect the testcases much, will
> only increase the time needed.

that's where you are wrong - there are plenty of ENOSPC tests where
the test is extremely specific about layout of files, the number and
size of them to create exact free space patterns and/or consumption.
Silently increasing the filesystem size because of mkfs suddenly
doesn't work properly means the tests no longer exercise the code
they were designed to test.

xfstests is not jsut for testing kernel changes - it tests all of
the filesystem utilities for regressions, too. And so when
inadvertant changes in default behaviour occur, it detects those
regressions too. We don't change tests just because they found
a regression...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com