[PATCH 0/6] Btrfs test updates and fixups

[PATCH 0/6] Btrfs test updates and fixups

[David Sterba]

Hi,

another batch of changes that we had for our CI. This adds requirements
for some test cases or adjusts conditions for running them successfuly
on btrfs (no affecting other filesystems).

Please add them to fstests, thanks.

Josef Bacik (6):
  common: udev settle before _scratch_pool_mkfs
  generic/352: require no compression
  generic/027: require no compression
  generic/269: require no compression
  btrfs/011: mkfs the scratch dev before exiting
  btrfs/{140,141}: verify read-repair test data by md5sum

 common/rc           |  6 ++++++
 tests/btrfs/011     |  5 +++++
 tests/btrfs/140     | 15 ++++++++++++++-
 tests/btrfs/140.out | 34 ----------------------------------
 tests/btrfs/141     | 16 +++++++++++++++-
 tests/btrfs/141.out | 34 ----------------------------------
 tests/generic/027   |  4 ++++
 tests/generic/269   |  3 +++
 tests/generic/352   |  4 ++++
 9 files changed, 51 insertions(+), 70 deletions(-)

-- 
2.45.0

[PATCH 1/6] common: udev settle before _scratch_pool_mkfs

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

There are some btrfs tests that do _scratch_pool_mkfs in a loop.
Sometimes this fails with EBUSY.  Tracing revealed that udevd will
sometimes write to /sys/block/device/uevent to make sure an event
triggers to rules get written.  However these events will not get sent
to user space until after an O_EXCL open as been closed.  The general
flow is something like

mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
mount /dev/sda /mnt/test
<things>
umount /mnt/test

in a loop.  The problem is udevd will add uevents for the devices and
they won't get delivered until after the umount.  If we're doing the
above sequence in a loop the next mkfs.btrfs will fail because udev is
touching the devices to consume the KOBJ_CHANGE event.

Fix this by doing a udev settle before _scratch_pool_mkfs.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 common/rc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/rc b/common/rc
index 0fe56382a6a497..5d38571ffe87eb 100644
--- a/common/rc
+++ b/common/rc
@@ -903,6 +903,12 @@ _scratch_pool_mkfs()
 {
     case $FSTYP in
     btrfs)
+	# For multi-disk file systems udev can queue up events on the device
+	# when we mkfs the device, and thus tie up the device after we've
+	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
+	# trip over udev trying to do the updates after the umount, so make sure
+	# we settle before we try mkfs'ing so we don't get an EBUSY
+	$UDEV_SETTLE_PROG >/dev/null 2>&1
         $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
         ;;
     *)
-- 
2.45.0

Re: [PATCH 1/6] common: udev settle before _scratch_pool_mkfs

[Anand Jain]

On 5/17/24 06:12, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> There are some btrfs tests that do _scratch_pool_mkfs in a loop.
> Sometimes this fails with EBUSY.  Tracing revealed that udevd will
> sometimes write to /sys/block/device/uevent to make sure an event
> triggers to rules get written.  However these events will not get sent
> to user space until after an O_EXCL open as been closed.  The general
> flow is something like
> 
> mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
> mount /dev/sda /mnt/test
> <things>
> umount /mnt/test
> 
> in a loop.  The problem is udevd will add uevents for the devices and
> they won't get delivered until after the umount.  If we're doing the
> above sequence in a loop the next mkfs.btrfs will fail because udev is
> touching the devices to consume the KOBJ_CHANGE event.
> 
> Fix this by doing a udev settle before _scratch_pool_mkfs.
> 



> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   common/rc | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 0fe56382a6a497..5d38571ffe87eb 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -903,6 +903,12 @@ _scratch_pool_mkfs()
>   {
>       case $FSTYP in
>       btrfs)
> +	# For multi-disk file systems udev can queue up events on the device
> +	# when we mkfs the device, and thus tie up the device after we've
> +	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
> +	# trip over udev trying to do the updates after the umount, so make sure
> +	# we settle before we try mkfs'ing so we don't get an EBUSY
> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
>           $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
>           ;;


Just curious: have we seen this issue even after the btrfs-progs commit 
below?

   e54514aaeab6 btrfs-progs: fix stray fd close in open_ctree_fs_info()

Thanks, Anand

>       *)

Re: [PATCH 1/6] common: udev settle before _scratch_pool_mkfs

[David Sterba]

On Sat, May 18, 2024 at 05:57:12AM +0800, Anand Jain wrote:
> On 5/17/24 06:12, David Sterba wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> > 
> > There are some btrfs tests that do _scratch_pool_mkfs in a loop.
> > Sometimes this fails with EBUSY.  Tracing revealed that udevd will
> > sometimes write to /sys/block/device/uevent to make sure an event
> > triggers to rules get written.  However these events will not get sent
> > to user space until after an O_EXCL open as been closed.  The general
> > flow is something like
> > 
> > mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
> > mount /dev/sda /mnt/test
> > <things>
> > umount /mnt/test
> > 
> > in a loop.  The problem is udevd will add uevents for the devices and
> > they won't get delivered until after the umount.  If we're doing the
> > above sequence in a loop the next mkfs.btrfs will fail because udev is
> > touching the devices to consume the KOBJ_CHANGE event.
> > 
> > Fix this by doing a udev settle before _scratch_pool_mkfs.
> > 
> 
> 
> 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   common/rc | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 0fe56382a6a497..5d38571ffe87eb 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -903,6 +903,12 @@ _scratch_pool_mkfs()
> >   {
> >       case $FSTYP in
> >       btrfs)
> > +	# For multi-disk file systems udev can queue up events on the device
> > +	# when we mkfs the device, and thus tie up the device after we've
> > +	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
> > +	# trip over udev trying to do the updates after the umount, so make sure
> > +	# we settle before we try mkfs'ing so we don't get an EBUSY
> > +	$UDEV_SETTLE_PROG >/dev/null 2>&1
> >           $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
> >           ;;
> 
> 
> Just curious: have we seen this issue even after the btrfs-progs commit 
> below?
> 
>    e54514aaeab6 btrfs-progs: fix stray fd close in open_ctree_fs_info()

That's a good question, I don't know but I can revert the change and see
if things break. This may take time as triggering the udev/mkfs race is
not reliable.

Re: [PATCH 1/6] common: udev settle before _scratch_pool_mkfs

[Anand Jain]

On 5/21/24 03:21, David Sterba wrote:
> On Sat, May 18, 2024 at 05:57:12AM +0800, Anand Jain wrote:
>> On 5/17/24 06:12, David Sterba wrote:
>>> From: Josef Bacik <josef@toxicpanda.com>
>>>
>>> There are some btrfs tests that do _scratch_pool_mkfs in a loop.
>>> Sometimes this fails with EBUSY.  Tracing revealed that udevd will
>>> sometimes write to /sys/block/device/uevent to make sure an event
>>> triggers to rules get written.  However these events will not get sent
>>> to user space until after an O_EXCL open as been closed.  The general
>>> flow is something like
>>>
>>> mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
>>> mount /dev/sda /mnt/test
>>> <things>
>>> umount /mnt/test
>>>
>>> in a loop.  The problem is udevd will add uevents for the devices and
>>> they won't get delivered until after the umount.  If we're doing the
>>> above sequence in a loop the next mkfs.btrfs will fail because udev is
>>> touching the devices to consume the KOBJ_CHANGE event.
>>>
>>> Fix this by doing a udev settle before _scratch_pool_mkfs.
>>>
>>
>>
>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>> ---
>>>    common/rc | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/common/rc b/common/rc
>>> index 0fe56382a6a497..5d38571ffe87eb 100644
>>> --- a/common/rc
>>> +++ b/common/rc
>>> @@ -903,6 +903,12 @@ _scratch_pool_mkfs()
>>>    {
>>>        case $FSTYP in
>>>        btrfs)
>>> +	# For multi-disk file systems udev can queue up events on the device
>>> +	# when we mkfs the device, and thus tie up the device after we've
>>> +	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
>>> +	# trip over udev trying to do the updates after the umount, so make sure
>>> +	# we settle before we try mkfs'ing so we don't get an EBUSY
>>> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
>>>            $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
>>>            ;;
>>
>>
>> Just curious: have we seen this issue even after the btrfs-progs commit
>> below?
>>
>>     e54514aaeab6 btrfs-progs: fix stray fd close in open_ctree_fs_info()
> 
> That's a good question, I don't know but I can revert the change and see
> if things break. This may take time as triggering the udev/mkfs race is
> not reliable.

Agreed. The udev/mkfs race is tricky and will take a while to report,
but it's good to experiment. IMO.

Thanks! Anand

[PATCH 2/6] generic/352: require no compression

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

Our CI has been failing on this test for compression since 0fc226e7
("fstests: generic/352 should accomodate other pwrite behaviors").  This
is because we changed the size of the initial write down to 4k, and we
write a repeatable pattern.  With compression on btrfs this results in
an inline extent, and when you reflink an inline extent this just turns
it into full on copies instead of a reflink.

As this isn't a bug with compression, it's just not well aligned with
how compression interacts with the allocation of space, simply exclude
this test from running when you have compression enabled.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 tests/generic/352 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/generic/352 b/tests/generic/352
index acc17dac38d061..3a18f07601de96 100755
--- a/tests/generic/352
+++ b/tests/generic/352
@@ -25,6 +25,10 @@ _supported_fs generic
 _require_scratch_reflink
 _require_xfs_io_command "fiemap"
 
+# The size is too small, this will result in an inline extent and then reflink
+# will simply be a copy on btrfs, so exclude compression.
+ _require_no_compress
+
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 
-- 
2.45.0

Re: [PATCH 2/6] generic/352: require no compression

[Anand Jain]

On 5/17/24 06:12, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> Our CI has been failing on this test for compression since 0fc226e7
> ("fstests: generic/352 should accomodate other pwrite behaviors").  This
> is because we changed the size of the initial write down to 4k, and we
> write a repeatable pattern.  With compression on btrfs this results in
> an inline extent, and when you reflink an inline extent this just turns
> it into full on copies instead of a reflink.
> 
> As this isn't a bug with compression, it's just not well aligned with
> how compression interacts with the allocation of space, simply exclude
> this test from running when you have compression enabled.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---




>   tests/generic/352 | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/tests/generic/352 b/tests/generic/352
> index acc17dac38d061..3a18f07601de96 100755
> --- a/tests/generic/352
> +++ b/tests/generic/352
> @@ -25,6 +25,10 @@ _supported_fs generic
>   _require_scratch_reflink
>   _require_xfs_io_command "fiemap"
>   
> +# The size is too small, this will result in an inline extent and then reflink
> +# will simply be a copy on btrfs, so exclude compression.
> + _require_no_compress
> +


Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand
>   _scratch_mkfs > /dev/null 2>&1
>   _scratch_mount
>   

[PATCH 3/6] generic/027: require no compression

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

This test creates a small file and then a giant file and then tries to
create a bunch of small files in a loop to exercise ENOPSC.  The problem
is that with compression the giant file isn't actually giant, so it can
make this test take forever.  Simply disable it for compression.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 tests/generic/027 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/generic/027 b/tests/generic/027
index 47f1981d4e51b7..eace1b358cdf34 100755
--- a/tests/generic/027
+++ b/tests/generic/027
@@ -33,6 +33,10 @@ _supported_fs generic
 
 _require_scratch
 
+# This takes forever with compression because we don't actually fill up the
+# disk, simply disable it if we have compression enabled.
+_require_no_compress
+
 echo "Silence is golden"
 
 _scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1
-- 
2.45.0

Re: [PATCH 3/6] generic/027: require no compression

[Anand Jain]

On 5/17/24 06:12, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> This test creates a small file and then a giant file and then tries to
> create a bunch of small files in a loop to exercise ENOPSC.  The problem
> is that with compression the giant file isn't actually giant, so it can
> make this test take forever.  Simply disable it for compression.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   tests/generic/027 | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/tests/generic/027 b/tests/generic/027
> index 47f1981d4e51b7..eace1b358cdf34 100755
> --- a/tests/generic/027
> +++ b/tests/generic/027
> @@ -33,6 +33,10 @@ _supported_fs generic
>   
>   _require_scratch
>   
> +# This takes forever with compression because we don't actually fill up the
> +# disk, simply disable it if we have compression enabled.
> +_require_no_compress
> +
>   echo "Silence is golden"
>   
>   _scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

[PATCH 4/6] generic/269: require no compression

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

This is meant to test ENOSPC, but we're dd'ing /dev/zero, which won't
fill up anything with compression on.

Additionally we're killing dd and then immediately trying to unmount.
With compression we could have references to the inode being held by the
async compression workers, so sometimes this will fail with EBUSY on the
unmount.

Make it easier on us and just skip this if we have compression enabled.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 tests/generic/269 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/generic/269 b/tests/generic/269
index b7cdecd94f219a..6ee0cb04581632 100755
--- a/tests/generic/269
+++ b/tests/generic/269
@@ -11,6 +11,9 @@ _begin_fstest auto rw prealloc ioctl enospc stress
 
 # Import common functions.
 . ./common/filter
+
+_require_no_compress
+
 # Disable all sync operations to get higher load
 FSSTRESS_AVOID="$FSSTRESS_AVOID -ffsync=0 -fsync=0 -ffdatasync=0"
 _workout()
-- 
2.45.0

Re: [PATCH 4/6] generic/269: require no compression

[Anand Jain]

On 5/17/24 06:12, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> This is meant to test ENOSPC, but we're dd'ing /dev/zero, which won't
> fill up anything with compression on.
> 
> Additionally we're killing dd and then immediately trying to unmount.
> With compression we could have references to the inode being held by the
> async compression workers, so sometimes this will fail with EBUSY on the
> unmount.
> 
> Make it easier on us and just skip this if we have compression enabled.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   tests/generic/269 | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tests/generic/269 b/tests/generic/269
> index b7cdecd94f219a..6ee0cb04581632 100755
> --- a/tests/generic/269
> +++ b/tests/generic/269
> @@ -11,6 +11,9 @@ _begin_fstest auto rw prealloc ioctl enospc stress
>   
>   # Import common functions.
>   . ./common/filter
> +
> +_require_no_compress
> +
>   # Disable all sync operations to get higher load
>   FSSTRESS_AVOID="$FSSTRESS_AVOID -ffsync=0 -fsync=0 -ffdatasync=0"
>   _workout()


Can we use _ddt to fill the file with partially compressible data?
On a laptop, with compression, it takes 63s (otherwise 45s).
If we're ok with this, I'll sort it out here. Git diff below.

Thanks, Anand


------
MOUNT_OPTIONS -- -o compress /dev/sdc1 /mnt/scratch

generic/269 45s ...  63s
Ran: generic/269
Passed all 1 tests
------


diff --git a/tests/generic/269 b/tests/generic/269
index b7cdecd94f21..29f453735fc2 100755
--- a/tests/generic/269
+++ b/tests/generic/269
@@ -29,7 +29,7 @@ _workout()
         for ((i=0; i < num_iterations; i++))
         do
                 # File will be opened with O_TRUNC each time
-               dd if=/dev/zero of=$SCRATCH_MNT/SPACE_CONSUMER bs=1M \
+               _ddt of=$SCRATCH_MNT/SPACE_CONSUMER bs=1M \
                         >> $seqres.full 2>&1
                 sleep $enospc_time
         done

Re: [PATCH 4/6] generic/269: require no compression

[David Sterba]

On Tue, May 21, 2024 at 08:37:48AM +0800, Anand Jain wrote:
> On 5/17/24 06:12, David Sterba wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> > 
> > This is meant to test ENOSPC, but we're dd'ing /dev/zero, which won't
> > fill up anything with compression on.
> > 
> > Additionally we're killing dd and then immediately trying to unmount.
> > With compression we could have references to the inode being held by the
> > async compression workers, so sometimes this will fail with EBUSY on the
> > unmount.
> > 
> > Make it easier on us and just skip this if we have compression enabled.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   tests/generic/269 | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/tests/generic/269 b/tests/generic/269
> > index b7cdecd94f219a..6ee0cb04581632 100755
> > --- a/tests/generic/269
> > +++ b/tests/generic/269
> > @@ -11,6 +11,9 @@ _begin_fstest auto rw prealloc ioctl enospc stress
> >   
> >   # Import common functions.
> >   . ./common/filter
> > +
> > +_require_no_compress
> > +
> >   # Disable all sync operations to get higher load
> >   FSSTRESS_AVOID="$FSSTRESS_AVOID -ffsync=0 -fsync=0 -ffdatasync=0"
> >   _workout()
> 
> 
> Can we use _ddt to fill the file with partially compressible data?
> On a laptop, with compression, it takes 63s (otherwise 45s).
> If we're ok with this, I'll sort it out here. Git diff below.

Yeah the incompressible data would be better. According to the changelog
the async workers could block umount but I'm not sure this actually
happens, umount should wait until the IO is done and all inodes closed.

[PATCH 5/6] btrfs/011: mkfs the scratch dev before exiting

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

When testing encryption I started getting failures because the scratch
dev didn't have a valid fs at the end of the test.  This is because for
encryption we have to disable raid5/6, which changes how the test is
run.

Normally with raid6 we end up cancelling the device replace, and thus
$SCRATCH_DEV has a valid file system on it.  However with raid5/6
disabled we end with a normal DUP profile, and the replace doesn't end
up cancelled, so $SCRATCH_DEV is wiped.  Then when the test finishes we
do the normal fsck and see that there's no fs on the $SCRATCH_DEV and
error.

This test does all the fsck'ing during the workout period, so we don't
need the final scratch check, simply re-make the $SCRATCH_DEV at the end
as it could have been replaced during the test.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 tests/btrfs/011 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/btrfs/011 b/tests/btrfs/011
index bf63a72b11c42f..d99624fb941cce 100755
--- a/tests/btrfs/011
+++ b/tests/btrfs/011
@@ -258,6 +258,11 @@ for t in "-m single -d single:1 no 64" \
 	fi
 done
 
+# If we exclude certain RAID profiles we can end up where the scratch dev
+# doesn't have a valid fs on it because it was replaced during workout, so mkfs
+# the scratch device so we don't get _check_btrfs_filesystem errors
+_scratch_mkfs > /dev/null 2>&1
+
 echo "*** done"
 status=0
 exit
-- 
2.45.0

Re: [PATCH 5/6] btrfs/011: mkfs the scratch dev before exiting

[Darrick J. Wong]

On Fri, May 17, 2024 at 12:12:50AM +0200, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> When testing encryption I started getting failures because the scratch
> dev didn't have a valid fs at the end of the test.  This is because for
> encryption we have to disable raid5/6, which changes how the test is
> run.
> 
> Normally with raid6 we end up cancelling the device replace, and thus
> $SCRATCH_DEV has a valid file system on it.  However with raid5/6
> disabled we end with a normal DUP profile, and the replace doesn't end
> up cancelled, so $SCRATCH_DEV is wiped.  Then when the test finishes we
> do the normal fsck and see that there's no fs on the $SCRATCH_DEV and
> error.
> 
> This test does all the fsck'ing during the workout period, so we don't
> need the final scratch check, simply re-make the $SCRATCH_DEV at the end
> as it could have been replaced during the test.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  tests/btrfs/011 | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/btrfs/011 b/tests/btrfs/011
> index bf63a72b11c42f..d99624fb941cce 100755
> --- a/tests/btrfs/011
> +++ b/tests/btrfs/011
> @@ -258,6 +258,11 @@ for t in "-m single -d single:1 no 64" \
>  	fi
>  done
>  
> +# If we exclude certain RAID profiles we can end up where the scratch dev
> +# doesn't have a valid fs on it because it was replaced during workout, so mkfs
> +# the scratch device so we don't get _check_btrfs_filesystem errors
> +_scratch_mkfs > /dev/null 2>&1

/me wonders why the_require_scratch_nocheck at the top doesn't shut off
the post-test fsck?

--D

> +
>  echo "*** done"
>  status=0
>  exit
> -- 
> 2.45.0
> 
> 

Re: [PATCH 5/6] btrfs/011: mkfs the scratch dev before exiting

[David Sterba]

On Fri, May 17, 2024 at 08:37:54AM -0700, Darrick J. Wong wrote:
> On Fri, May 17, 2024 at 12:12:50AM +0200, David Sterba wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> > 
> > When testing encryption I started getting failures because the scratch
> > dev didn't have a valid fs at the end of the test.  This is because for
> > encryption we have to disable raid5/6, which changes how the test is
> > run.
> > 
> > Normally with raid6 we end up cancelling the device replace, and thus
> > $SCRATCH_DEV has a valid file system on it.  However with raid5/6
> > disabled we end with a normal DUP profile, and the replace doesn't end
> > up cancelled, so $SCRATCH_DEV is wiped.  Then when the test finishes we
> > do the normal fsck and see that there's no fs on the $SCRATCH_DEV and
> > error.
> > 
> > This test does all the fsck'ing during the workout period, so we don't
> > need the final scratch check, simply re-make the $SCRATCH_DEV at the end
> > as it could have been replaced during the test.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  tests/btrfs/011 | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/tests/btrfs/011 b/tests/btrfs/011
> > index bf63a72b11c42f..d99624fb941cce 100755
> > --- a/tests/btrfs/011
> > +++ b/tests/btrfs/011
> > @@ -258,6 +258,11 @@ for t in "-m single -d single:1 no 64" \
> >  	fi
> >  done
> >  
> > +# If we exclude certain RAID profiles we can end up where the scratch dev
> > +# doesn't have a valid fs on it because it was replaced during workout, so mkfs
> > +# the scratch device so we don't get _check_btrfs_filesystem errors
> > +_scratch_mkfs > /dev/null 2>&1
> 
> /me wonders why the_require_scratch_nocheck at the top doesn't shut off
> the post-test fsck?

I think Josef did this when adding encryption support and tests and had
a reason for that, but I don't know why or what was the error.  This
patch can be skipped in case the rest is OK, we'll keep in our local
branch and reevaluate it again before sending.

[PATCH 6/6] btrfs/{140,141}: verify read-repair test data by md5sum

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

For validating that read repair works properly we corrupt one mirror and
then read back the physical location after we do a direct or buffered
read on the mounted file system and then unmount the file system.  The
golden output expects all a's, however with encryption this will
obviously not be the case.

However I still broke read repair, so these tests are quite valuable.
Fix them to dump the on disk values to a temporary file and then md5sum
the files, and then validate the md5sum to make sure the read repair
worked properly.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 tests/btrfs/140     | 15 ++++++++++++++-
 tests/btrfs/140.out | 34 ----------------------------------
 tests/btrfs/141     | 16 +++++++++++++++-
 tests/btrfs/141.out | 34 ----------------------------------
 4 files changed, 29 insertions(+), 70 deletions(-)

diff --git a/tests/btrfs/140 b/tests/btrfs/140
index f1bee5d3841234..eb0fbc13b2513b 100755
--- a/tests/btrfs/140
+++ b/tests/btrfs/140
@@ -74,6 +74,13 @@ devpath=$(get_device_path ${devid})
 
 _scratch_unmount
 
+# Grab the contents of the the area so we can compare to the final part
+orig=$(mktemp)
+$XFS_IO_PROG -d -c "pread -v -b 512 $physical 512" $devpath |\
+	 _filter_xfs_io_offset > $orig
+origcsum=$(_md5_checksum $orig)
+rm -f $orig
+
 echo " corrupt stripe #1, devid $devid devpath $devpath physical $physical" \
 	>> $seqres.full
 $XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $devpath > /dev/null
@@ -88,10 +95,16 @@ _btrfs_direct_read_on_mirror 1 2 "$SCRATCH_MNT/foobar" 0 128K
 _scratch_unmount
 
 # check if the repair works
+final=$(mktemp)
 $XFS_IO_PROG -d -c "pread -v -b 512 $physical 512" $devpath |\
-	_filter_xfs_io_offset
+	_filter_xfs_io_offset > $final
+finalcsum=$(_md5_checksum $final)
+rm -f $final
 
 _scratch_dev_pool_put
+
+[ "$origcsum" == "$finalcsum" ] || _fail "repair failed, csums don't match"
+
 # success, all done
 status=0
 exit
diff --git a/tests/btrfs/140.out b/tests/btrfs/140.out
index fb5aa1087235ac..58dfb24ec1a565 100644
--- a/tests/btrfs/140.out
+++ b/tests/btrfs/140.out
@@ -1,37 +1,3 @@
 QA output created by 140
 wrote 131072/131072 bytes
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-read 512/512 bytes
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/btrfs/141 b/tests/btrfs/141
index c78ee33e484cad..d46301a5220ba3 100755
--- a/tests/btrfs/141
+++ b/tests/btrfs/141
@@ -72,6 +72,14 @@ devid=$(get_devid ${logical_in_btrfs} 1)
 devpath=$(get_device_path ${devid})
 
 _scratch_unmount
+
+# Grab the contents of the area so we can compare to the final part
+orig=$(mktemp)
+$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $devpath |\
+	_filter_xfs_io_offset > $orig
+origcsum=$(_md5_checksum $orig)
+rm -f $orig
+
 echo " corrupt stripe #1, devid $devid devpath $devpath physical $physical" \
 	>> $seqres.full
 $XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $devpath > /dev/null
@@ -86,10 +94,16 @@ _btrfs_buffered_read_on_mirror 1 2 "$SCRATCH_MNT/foobar" 0 128K
 _scratch_unmount
 
 # check if the repair works
+final=$(mktemp)
 $XFS_IO_PROG -c "pread -v -b 512 $physical 512" $devpath |\
-	_filter_xfs_io_offset
+	_filter_xfs_io_offset > $final
+finalcsum=$(_md5_checksum $final)
+rm -f $final
 
 _scratch_dev_pool_put
+
+[ "$origcsum" == "$finalcsum" ] || _fail "repair failed, csums don't match"
+
 # success, all done
 status=0
 exit
diff --git a/tests/btrfs/141.out b/tests/btrfs/141.out
index 4b8be189144e36..d8c6940f109b1d 100644
--- a/tests/btrfs/141.out
+++ b/tests/btrfs/141.out
@@ -1,37 +1,3 @@
 QA output created by 141
 wrote 131072/131072 bytes
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-read 512/512 bytes
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
2.45.0

Re: [PATCH 6/6] btrfs/{140,141}: verify read-repair test data by md5sum

[Anand Jain]

Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thx, Anand

Re: [PATCH 0/6] Btrfs test updates and fixups

[Anand Jain]

On 17/05/2024 06:12, David Sterba wrote:
> Hi,
> 
> another batch of changes that we had for our CI. This adds requirements
> for some test cases or adjusts conditions for running them successfuly
> on btrfs (no affecting other filesystems).
> 
> Please add them to fstests, thanks.
> 


> Josef Bacik (6):
>    common: udev settle before _scratch_pool_mkfs
>    generic/352: require no compression
>    generic/027: require no compression
>    generic/269: require no compression
>    btrfs/011: mkfs the scratch dev before exiting
>    btrfs/{140,141}: verify read-repair test data by md5sum

Applied without 1/6 and 5/6.

Thanks, Anand

> 
>   common/rc           |  6 ++++++
>   tests/btrfs/011     |  5 +++++
>   tests/btrfs/140     | 15 ++++++++++++++-
>   tests/btrfs/140.out | 34 ----------------------------------
>   tests/btrfs/141     | 16 +++++++++++++++-
>   tests/btrfs/141.out | 34 ----------------------------------
>   tests/generic/027   |  4 ++++
>   tests/generic/269   |  3 +++
>   tests/generic/352   |  4 ++++
>   9 files changed, 51 insertions(+), 70 deletions(-)
>