diff mbox series

btrfs/016: fix a false alert due to xattrs mismatch

Message ID ae441bd376587124becd9141ed690598d4ed281a.1703741660.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs/016: fix a false alert due to xattrs mismatch | expand

Commit Message

Qu Wenruo Dec. 28, 2023, 5:36 a.m. UTC
[BUG]
When running btrfs/016 after any other test case, it would fail on a
SELinux enabled environment:

  btrfs/015 1s ...  0s
  btrfs/016 1s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/016.out.bad)
      --- tests/btrfs/016.out	2023-12-28 10:39:36.481027970 +1030
      +++ ~/xfstests-dev/results//btrfs/016.out.bad	2023-12-28 15:53:10.745436664 +1030
      @@ -1,2 +1,3 @@
       QA output created by 016
      -Silence is golden
      +fssum failed
      +(see ~/xfstests-dev/results//btrfs/016.full for details)
      ...
      (Run 'diff -u ~/xfstests-dev/tests/btrfs/016.out ~/xfstests-dev/results//btrfs/016.out.bad'  to see the entire diff)
  Ran: btrfs/015 btrfs/016
  Failures: btrfs/016
  Failed 1 of 2 tests

[CAUSE]
The test case itself would try to use a blank SELinux context for the
SCRATCH_MNT, to control the xattrs.

But the initial send stream is generated from $TEST_DIR, which may still
have the default SELinux mount context.

And such mismatch in the SELinux xattr (source on $TEST_DIR still has
the extra xattr, meanwhile the receve end on $SCRATCH_MNT doesn't) would
lead to above mismatch.

[FIX]
Instead of doing all the edge juggling using $TEST_DIR, this time we do
all the work on $SCRATCH_MNT.

This means we would generate the initial send stream from $SCRATCH_MNT,
then reformat the fs, mount scratch again, receive and verify.

This does not only fix the above false alerts, but also simplify the
cleanup.
We no longer needs to cleanup the extra file for the initial send
stream, as they are on the scratch device and would be formatted anyway.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/016 | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

Comments

Qu Wenruo Feb. 17, 2024, 9:31 a.m. UTC | #1
A gentle ping.

As the test case still failed in the latest for-next branch due to the 
SELinux xattr mismatch.

Thanks,
Qu

在 2023/12/28 16:06, Qu Wenruo 写道:
> [BUG]
> When running btrfs/016 after any other test case, it would fail on a
> SELinux enabled environment:
> 
>    btrfs/015 1s ...  0s
>    btrfs/016 1s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/016.out.bad)
>        --- tests/btrfs/016.out	2023-12-28 10:39:36.481027970 +1030
>        +++ ~/xfstests-dev/results//btrfs/016.out.bad	2023-12-28 15:53:10.745436664 +1030
>        @@ -1,2 +1,3 @@
>         QA output created by 016
>        -Silence is golden
>        +fssum failed
>        +(see ~/xfstests-dev/results//btrfs/016.full for details)
>        ...
>        (Run 'diff -u ~/xfstests-dev/tests/btrfs/016.out ~/xfstests-dev/results//btrfs/016.out.bad'  to see the entire diff)
>    Ran: btrfs/015 btrfs/016
>    Failures: btrfs/016
>    Failed 1 of 2 tests
> 
> [CAUSE]
> The test case itself would try to use a blank SELinux context for the
> SCRATCH_MNT, to control the xattrs.
> 
> But the initial send stream is generated from $TEST_DIR, which may still
> have the default SELinux mount context.
> 
> And such mismatch in the SELinux xattr (source on $TEST_DIR still has
> the extra xattr, meanwhile the receve end on $SCRATCH_MNT doesn't) would
> lead to above mismatch.
> 
> [FIX]
> Instead of doing all the edge juggling using $TEST_DIR, this time we do
> all the work on $SCRATCH_MNT.
> 
> This means we would generate the initial send stream from $SCRATCH_MNT,
> then reformat the fs, mount scratch again, receive and verify.
> 
> This does not only fix the above false alerts, but also simplify the
> cleanup.
> We no longer needs to cleanup the extra file for the initial send
> stream, as they are on the scratch device and would be formatted anyway.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   tests/btrfs/016 | 46 ++++++++++++++++++++++------------------------
>   1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/tests/btrfs/016 b/tests/btrfs/016
> index 35609329ba0e..9371b3316332 100755
> --- a/tests/btrfs/016
> +++ b/tests/btrfs/016
> @@ -12,22 +12,11 @@ _begin_fstest auto quick send prealloc
>   tmp=`mktemp -d`
>   tmp_dir=send_temp_$seq
>   
> -# Override the default cleanup function.
> -_cleanup()
> -{
> -	$BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap > /dev/null 2>&1
> -	$BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap1 > /dev/null 2>&1
> -	$BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/send > /dev/null 2>&1
> -	rm -rf $TEST_DIR/$tmp_dir
> -	rm -f $tmp.*
> -}
> -
>   # Import common functions.
>   . ./common/filter
>   
>   # real QA test starts here
>   _supported_fs btrfs
> -_require_test
>   _require_scratch
>   _require_fssum
>   _require_xfs_io_command "falloc"
> @@ -41,29 +30,33 @@ export SELINUX_MOUNT_OPTIONS=""
>   
>   _scratch_mount
>   
> -mkdir $TEST_DIR/$tmp_dir
> -$BTRFS_UTIL_PROG subvolume create $TEST_DIR/$tmp_dir/send \
> +mkdir $SCRATCH_MNT/$tmp_dir
> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/$tmp_dir/send \
>   	> $seqres.full 2>&1 || _fail "failed subvolume create"
>   
> -_ddt of=$TEST_DIR/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
> +_ddt of=$SCRATCH_MNT/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
>   	2>&1 || _fail "dd failed"
> -$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
> -	$TEST_DIR/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
> -$XFS_IO_PROG -c "fpunch 1m 1m" $TEST_DIR/$tmp_dir/send/foo
> -$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
> -	$TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
> +	$SCRATCH_MNT/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
> +$XFS_IO_PROG -c "fpunch 1m 1m" $SCRATCH_MNT/$tmp_dir/send/foo
> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
> +	$SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
>   
> -$FSSUM_PROG -A -f -w $tmp/fssum.snap $TEST_DIR/$tmp_dir/snap >> $seqres.full \
> +$FSSUM_PROG -A -f -w $tmp/fssum.snap $SCRATCH_MNT/$tmp_dir/snap >> $seqres.full \
>   	2>&1 || _fail "fssum gen failed"
> -$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $TEST_DIR/$tmp_dir/snap1 >> $seqres.full \
> +$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full \
>   	2>&1 || _fail "fssum gen failed"
>   
> -$BTRFS_UTIL_PROG send -f $tmp/send.snap $TEST_DIR/$tmp_dir/snap >> \
> +$BTRFS_UTIL_PROG send -f $tmp/send.snap $SCRATCH_MNT/$tmp_dir/snap >> \
>   	$seqres.full 2>&1 || _fail "failed send"
> -$BTRFS_UTIL_PROG send -p $TEST_DIR/$tmp_dir/snap \
> -	-f $tmp/send.snap1 $TEST_DIR/$tmp_dir/snap1 \
> +$BTRFS_UTIL_PROG send -p $SCRATCH_MNT/$tmp_dir/snap \
> +	-f $tmp/send.snap1 $SCRATCH_MNT/$tmp_dir/snap1 \
>   	>> $seqres.full 2>&1 || _fail "failed send"
>   
> +_scratch_unmount
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
>   $BTRFS_UTIL_PROG receive -f $tmp/send.snap $SCRATCH_MNT >> $seqres.full 2>&1 \
>   	|| _fail "failed recv"
>   $BTRFS_UTIL_PROG receive -f $tmp/send.snap1 $SCRATCH_MNT >> $seqres.full 2>&1 \
> @@ -74,5 +67,10 @@ $FSSUM_PROG -r $tmp/fssum.snap $SCRATCH_MNT/snap >> $seqres.full 2>&1 \
>   $FSSUM_PROG -r $tmp/fssum.snap1 $SCRATCH_MNT/snap1 >> $seqres.full 2>&1 \
>   	|| _fail "fssum failed"
>   
> +# Unset the selinux mount options and restore whatever the default one for
> +# test device.
> +unset SELINUX_MOUNT_OPTIONS
> +_test_cycle_mount
> +
>   echo "Silence is golden"
>   status=0 ; exit
Filipe Manana Feb. 17, 2024, 12:25 p.m. UTC | #2
On Thu, Dec 28, 2023 at 5:36 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> When running btrfs/016 after any other test case, it would fail on a
> SELinux enabled environment:
>
>   btrfs/015 1s ...  0s
>   btrfs/016 1s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/016.out.bad)
>       --- tests/btrfs/016.out   2023-12-28 10:39:36.481027970 +1030
>       +++ ~/xfstests-dev/results//btrfs/016.out.bad     2023-12-28 15:53:10.745436664 +1030
>       @@ -1,2 +1,3 @@
>        QA output created by 016
>       -Silence is golden
>       +fssum failed
>       +(see ~/xfstests-dev/results//btrfs/016.full for details)
>       ...
>       (Run 'diff -u ~/xfstests-dev/tests/btrfs/016.out ~/xfstests-dev/results//btrfs/016.out.bad'  to see the entire diff)
>   Ran: btrfs/015 btrfs/016
>   Failures: btrfs/016
>   Failed 1 of 2 tests
>
> [CAUSE]
> The test case itself would try to use a blank SELinux context for the
> SCRATCH_MNT, to control the xattrs.
>
> But the initial send stream is generated from $TEST_DIR, which may still
> have the default SELinux mount context.
>
> And such mismatch in the SELinux xattr (source on $TEST_DIR still has
> the extra xattr, meanwhile the receve end on $SCRATCH_MNT doesn't) would
> lead to above mismatch.
>
> [FIX]
> Instead of doing all the edge juggling using $TEST_DIR, this time we do
> all the work on $SCRATCH_MNT.
>
> This means we would generate the initial send stream from $SCRATCH_MNT,
> then reformat the fs, mount scratch again, receive and verify.
>
> This does not only fix the above false alerts, but also simplify the
> cleanup.
> We no longer needs to cleanup the extra file for the initial send
> stream, as they are on the scratch device and would be formatted anyway.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/016 | 46 ++++++++++++++++++++++------------------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/tests/btrfs/016 b/tests/btrfs/016
> index 35609329ba0e..9371b3316332 100755
> --- a/tests/btrfs/016
> +++ b/tests/btrfs/016
> @@ -12,22 +12,11 @@ _begin_fstest auto quick send prealloc
>  tmp=`mktemp -d`
>  tmp_dir=send_temp_$seq
>
> -# Override the default cleanup function.
> -_cleanup()
> -{
> -       $BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap > /dev/null 2>&1
> -       $BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap1 > /dev/null 2>&1
> -       $BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/send > /dev/null 2>&1
> -       rm -rf $TEST_DIR/$tmp_dir
> -       rm -f $tmp.*
> -}
> -
>  # Import common functions.
>  . ./common/filter
>
>  # real QA test starts here
>  _supported_fs btrfs
> -_require_test
>  _require_scratch
>  _require_fssum
>  _require_xfs_io_command "falloc"
> @@ -41,29 +30,33 @@ export SELINUX_MOUNT_OPTIONS=""
>
>  _scratch_mount
>
> -mkdir $TEST_DIR/$tmp_dir
> -$BTRFS_UTIL_PROG subvolume create $TEST_DIR/$tmp_dir/send \
> +mkdir $SCRATCH_MNT/$tmp_dir
> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/$tmp_dir/send \
>         > $seqres.full 2>&1 || _fail "failed subvolume create"
>
> -_ddt of=$TEST_DIR/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
> +_ddt of=$SCRATCH_MNT/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
>         2>&1 || _fail "dd failed"
> -$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
> -       $TEST_DIR/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
> -$XFS_IO_PROG -c "fpunch 1m 1m" $TEST_DIR/$tmp_dir/send/foo
> -$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
> -       $TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
> +       $SCRATCH_MNT/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
> +$XFS_IO_PROG -c "fpunch 1m 1m" $SCRATCH_MNT/$tmp_dir/send/foo
> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
> +       $SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
>
> -$FSSUM_PROG -A -f -w $tmp/fssum.snap $TEST_DIR/$tmp_dir/snap >> $seqres.full \
> +$FSSUM_PROG -A -f -w $tmp/fssum.snap $SCRATCH_MNT/$tmp_dir/snap >> $seqres.full \
>         2>&1 || _fail "fssum gen failed"
> -$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $TEST_DIR/$tmp_dir/snap1 >> $seqres.full \
> +$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full \
>         2>&1 || _fail "fssum gen failed"
>
> -$BTRFS_UTIL_PROG send -f $tmp/send.snap $TEST_DIR/$tmp_dir/snap >> \
> +$BTRFS_UTIL_PROG send -f $tmp/send.snap $SCRATCH_MNT/$tmp_dir/snap >> \
>         $seqres.full 2>&1 || _fail "failed send"
> -$BTRFS_UTIL_PROG send -p $TEST_DIR/$tmp_dir/snap \
> -       -f $tmp/send.snap1 $TEST_DIR/$tmp_dir/snap1 \
> +$BTRFS_UTIL_PROG send -p $SCRATCH_MNT/$tmp_dir/snap \
> +       -f $tmp/send.snap1 $SCRATCH_MNT/$tmp_dir/snap1 \
>         >> $seqres.full 2>&1 || _fail "failed send"
>
> +_scratch_unmount
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
>  $BTRFS_UTIL_PROG receive -f $tmp/send.snap $SCRATCH_MNT >> $seqres.full 2>&1 \
>         || _fail "failed recv"
>  $BTRFS_UTIL_PROG receive -f $tmp/send.snap1 $SCRATCH_MNT >> $seqres.full 2>&1 \
> @@ -74,5 +67,10 @@ $FSSUM_PROG -r $tmp/fssum.snap $SCRATCH_MNT/snap >> $seqres.full 2>&1 \
>  $FSSUM_PROG -r $tmp/fssum.snap1 $SCRATCH_MNT/snap1 >> $seqres.full 2>&1 \
>         || _fail "fssum failed"
>
> +# Unset the selinux mount options and restore whatever the default one for
> +# test device.
> +unset SELINUX_MOUNT_OPTIONS
> +_test_cycle_mount

Why do we need to _test_cycle_mount?

We're not using the test device anymore after this change, so
unsetting SELINUX_MOUNT_OPTIONS should be enough.

A simpler alternative fix would be just to pass -T to fssum, so that
it ignores xattrs when computing/verifying checksums, which is ok
since the tests' goal is to verify file data and that the hole
punching worked. Or just not use fssum and compare md5sum in the
original fs and the new fs.

Either way, it looks good to me except that confusing part of the
_test_cycle_mount which seems irrelevant to me.

Thanks.

> +
>  echo "Silence is golden"
>  status=0 ; exit
> --
> 2.43.0
>
>
Qu Wenruo Feb. 17, 2024, 8:39 p.m. UTC | #3
在 2024/2/17 22:55, Filipe Manana 写道:
> On Thu, Dec 28, 2023 at 5:36 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> [BUG]
>> When running btrfs/016 after any other test case, it would fail on a
>> SELinux enabled environment:
>>
>>    btrfs/015 1s ...  0s
>>    btrfs/016 1s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/016.out.bad)
>>        --- tests/btrfs/016.out   2023-12-28 10:39:36.481027970 +1030
>>        +++ ~/xfstests-dev/results//btrfs/016.out.bad     2023-12-28 15:53:10.745436664 +1030
>>        @@ -1,2 +1,3 @@
>>         QA output created by 016
>>        -Silence is golden
>>        +fssum failed
>>        +(see ~/xfstests-dev/results//btrfs/016.full for details)
>>        ...
>>        (Run 'diff -u ~/xfstests-dev/tests/btrfs/016.out ~/xfstests-dev/results//btrfs/016.out.bad'  to see the entire diff)
>>    Ran: btrfs/015 btrfs/016
>>    Failures: btrfs/016
>>    Failed 1 of 2 tests
>>
>> [CAUSE]
>> The test case itself would try to use a blank SELinux context for the
>> SCRATCH_MNT, to control the xattrs.
>>
>> But the initial send stream is generated from $TEST_DIR, which may still
>> have the default SELinux mount context.
>>
>> And such mismatch in the SELinux xattr (source on $TEST_DIR still has
>> the extra xattr, meanwhile the receve end on $SCRATCH_MNT doesn't) would
>> lead to above mismatch.
>>
>> [FIX]
>> Instead of doing all the edge juggling using $TEST_DIR, this time we do
>> all the work on $SCRATCH_MNT.
>>
>> This means we would generate the initial send stream from $SCRATCH_MNT,
>> then reformat the fs, mount scratch again, receive and verify.
>>
>> This does not only fix the above false alerts, but also simplify the
>> cleanup.
>> We no longer needs to cleanup the extra file for the initial send
>> stream, as they are on the scratch device and would be formatted anyway.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   tests/btrfs/016 | 46 ++++++++++++++++++++++------------------------
>>   1 file changed, 22 insertions(+), 24 deletions(-)
>>
>> diff --git a/tests/btrfs/016 b/tests/btrfs/016
>> index 35609329ba0e..9371b3316332 100755
>> --- a/tests/btrfs/016
>> +++ b/tests/btrfs/016
>> @@ -12,22 +12,11 @@ _begin_fstest auto quick send prealloc
>>   tmp=`mktemp -d`
>>   tmp_dir=send_temp_$seq
>>
>> -# Override the default cleanup function.
>> -_cleanup()
>> -{
>> -       $BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap > /dev/null 2>&1
>> -       $BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap1 > /dev/null 2>&1
>> -       $BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/send > /dev/null 2>&1
>> -       rm -rf $TEST_DIR/$tmp_dir
>> -       rm -f $tmp.*
>> -}
>> -
>>   # Import common functions.
>>   . ./common/filter
>>
>>   # real QA test starts here
>>   _supported_fs btrfs
>> -_require_test
>>   _require_scratch
>>   _require_fssum
>>   _require_xfs_io_command "falloc"
>> @@ -41,29 +30,33 @@ export SELINUX_MOUNT_OPTIONS=""
>>
>>   _scratch_mount
>>
>> -mkdir $TEST_DIR/$tmp_dir
>> -$BTRFS_UTIL_PROG subvolume create $TEST_DIR/$tmp_dir/send \
>> +mkdir $SCRATCH_MNT/$tmp_dir
>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/$tmp_dir/send \
>>          > $seqres.full 2>&1 || _fail "failed subvolume create"
>>
>> -_ddt of=$TEST_DIR/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
>> +_ddt of=$SCRATCH_MNT/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
>>          2>&1 || _fail "dd failed"
>> -$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
>> -       $TEST_DIR/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
>> -$XFS_IO_PROG -c "fpunch 1m 1m" $TEST_DIR/$tmp_dir/send/foo
>> -$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
>> -       $TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
>> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
>> +       $SCRATCH_MNT/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
>> +$XFS_IO_PROG -c "fpunch 1m 1m" $SCRATCH_MNT/$tmp_dir/send/foo
>> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
>> +       $SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
>>
>> -$FSSUM_PROG -A -f -w $tmp/fssum.snap $TEST_DIR/$tmp_dir/snap >> $seqres.full \
>> +$FSSUM_PROG -A -f -w $tmp/fssum.snap $SCRATCH_MNT/$tmp_dir/snap >> $seqres.full \
>>          2>&1 || _fail "fssum gen failed"
>> -$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $TEST_DIR/$tmp_dir/snap1 >> $seqres.full \
>> +$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full \
>>          2>&1 || _fail "fssum gen failed"
>>
>> -$BTRFS_UTIL_PROG send -f $tmp/send.snap $TEST_DIR/$tmp_dir/snap >> \
>> +$BTRFS_UTIL_PROG send -f $tmp/send.snap $SCRATCH_MNT/$tmp_dir/snap >> \
>>          $seqres.full 2>&1 || _fail "failed send"
>> -$BTRFS_UTIL_PROG send -p $TEST_DIR/$tmp_dir/snap \
>> -       -f $tmp/send.snap1 $TEST_DIR/$tmp_dir/snap1 \
>> +$BTRFS_UTIL_PROG send -p $SCRATCH_MNT/$tmp_dir/snap \
>> +       -f $tmp/send.snap1 $SCRATCH_MNT/$tmp_dir/snap1 \
>>          >> $seqres.full 2>&1 || _fail "failed send"
>>
>> +_scratch_unmount
>> +_scratch_mkfs > /dev/null 2>&1
>> +_scratch_mount
>> +
>>   $BTRFS_UTIL_PROG receive -f $tmp/send.snap $SCRATCH_MNT >> $seqres.full 2>&1 \
>>          || _fail "failed recv"
>>   $BTRFS_UTIL_PROG receive -f $tmp/send.snap1 $SCRATCH_MNT >> $seqres.full 2>&1 \
>> @@ -74,5 +67,10 @@ $FSSUM_PROG -r $tmp/fssum.snap $SCRATCH_MNT/snap >> $seqres.full 2>&1 \
>>   $FSSUM_PROG -r $tmp/fssum.snap1 $SCRATCH_MNT/snap1 >> $seqres.full 2>&1 \
>>          || _fail "fssum failed"
>>
>> +# Unset the selinux mount options and restore whatever the default one for
>> +# test device.
>> +unset SELINUX_MOUNT_OPTIONS
>> +_test_cycle_mount
>
> Why do we need to _test_cycle_mount?
>
> We're not using the test device anymore after this change, so
> unsetting SELINUX_MOUNT_OPTIONS should be enough.
>
> A simpler alternative fix would be just to pass -T to fssum, so that
> it ignores xattrs when computing/verifying checksums, which is ok
> since the tests' goal is to verify file data and that the hole
> punching worked.

That makes sense. In that case we can completely remove the
SELINUX_MOUNT_OPTIONS related setup.

> Or just not use fssum and compare md5sum in the
> original fs and the new fs.
>
> Either way, it looks good to me except that confusing part of the
> _test_cycle_mount which seems irrelevant to me.

Thanks for the review, would update this fix soon.

Thanks,
Qu

>
> Thanks.
>
>> +
>>   echo "Silence is golden"
>>   status=0 ; exit
>> --
>> 2.43.0
>>
>>
>
diff mbox series

Patch

diff --git a/tests/btrfs/016 b/tests/btrfs/016
index 35609329ba0e..9371b3316332 100755
--- a/tests/btrfs/016
+++ b/tests/btrfs/016
@@ -12,22 +12,11 @@  _begin_fstest auto quick send prealloc
 tmp=`mktemp -d`
 tmp_dir=send_temp_$seq
 
-# Override the default cleanup function.
-_cleanup()
-{
-	$BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap > /dev/null 2>&1
-	$BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/snap1 > /dev/null 2>&1
-	$BTRFS_UTIL_PROG subvolume delete $TEST_DIR/$tmp_dir/send > /dev/null 2>&1
-	rm -rf $TEST_DIR/$tmp_dir
-	rm -f $tmp.*
-}
-
 # Import common functions.
 . ./common/filter
 
 # real QA test starts here
 _supported_fs btrfs
-_require_test
 _require_scratch
 _require_fssum
 _require_xfs_io_command "falloc"
@@ -41,29 +30,33 @@  export SELINUX_MOUNT_OPTIONS=""
 
 _scratch_mount
 
-mkdir $TEST_DIR/$tmp_dir
-$BTRFS_UTIL_PROG subvolume create $TEST_DIR/$tmp_dir/send \
+mkdir $SCRATCH_MNT/$tmp_dir
+$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/$tmp_dir/send \
 	> $seqres.full 2>&1 || _fail "failed subvolume create"
 
-_ddt of=$TEST_DIR/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
+_ddt of=$SCRATCH_MNT/$tmp_dir/send/foo bs=1M count=10 >> $seqres.full \
 	2>&1 || _fail "dd failed"
-$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
-	$TEST_DIR/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
-$XFS_IO_PROG -c "fpunch 1m 1m" $TEST_DIR/$tmp_dir/send/foo
-$BTRFS_UTIL_PROG subvolume snapshot -r $TEST_DIR/$tmp_dir/send \
-	$TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
+$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
+	$SCRATCH_MNT/$tmp_dir/snap >> $seqres.full 2>&1 || _fail "failed snap"
+$XFS_IO_PROG -c "fpunch 1m 1m" $SCRATCH_MNT/$tmp_dir/send/foo
+$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/$tmp_dir/send \
+	$SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap"
 
-$FSSUM_PROG -A -f -w $tmp/fssum.snap $TEST_DIR/$tmp_dir/snap >> $seqres.full \
+$FSSUM_PROG -A -f -w $tmp/fssum.snap $SCRATCH_MNT/$tmp_dir/snap >> $seqres.full \
 	2>&1 || _fail "fssum gen failed"
-$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $TEST_DIR/$tmp_dir/snap1 >> $seqres.full \
+$FSSUM_PROG -A -f -w $tmp/fssum.snap1 $SCRATCH_MNT/$tmp_dir/snap1 >> $seqres.full \
 	2>&1 || _fail "fssum gen failed"
 
-$BTRFS_UTIL_PROG send -f $tmp/send.snap $TEST_DIR/$tmp_dir/snap >> \
+$BTRFS_UTIL_PROG send -f $tmp/send.snap $SCRATCH_MNT/$tmp_dir/snap >> \
 	$seqres.full 2>&1 || _fail "failed send"
-$BTRFS_UTIL_PROG send -p $TEST_DIR/$tmp_dir/snap \
-	-f $tmp/send.snap1 $TEST_DIR/$tmp_dir/snap1 \
+$BTRFS_UTIL_PROG send -p $SCRATCH_MNT/$tmp_dir/snap \
+	-f $tmp/send.snap1 $SCRATCH_MNT/$tmp_dir/snap1 \
 	>> $seqres.full 2>&1 || _fail "failed send"
 
+_scratch_unmount
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
 $BTRFS_UTIL_PROG receive -f $tmp/send.snap $SCRATCH_MNT >> $seqres.full 2>&1 \
 	|| _fail "failed recv"
 $BTRFS_UTIL_PROG receive -f $tmp/send.snap1 $SCRATCH_MNT >> $seqres.full 2>&1 \
@@ -74,5 +67,10 @@  $FSSUM_PROG -r $tmp/fssum.snap $SCRATCH_MNT/snap >> $seqres.full 2>&1 \
 $FSSUM_PROG -r $tmp/fssum.snap1 $SCRATCH_MNT/snap1 >> $seqres.full 2>&1 \
 	|| _fail "fssum failed"
 
+# Unset the selinux mount options and restore whatever the default one for
+# test device.
+unset SELINUX_MOUNT_OPTIONS
+_test_cycle_mount
+
 echo "Silence is golden"
 status=0 ; exit