On Mon, Feb 19, 2024 at 8:18 AM Qu Wenruo <wqu@xxxxxxxx> 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]
> Fix the false alerts by disable XATTR checks.
>
> Furthermore 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.
>
> 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@xxxxxxxx>
> ---
> Changelog:
> v2:
> - Add -T option to avoid xattrs checks
> Since this test case is only verify the hole punching behavior, XATTR
> is not our interest.
> ---
> tests/btrfs/016 | 53 ++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/tests/btrfs/016 b/tests/btrfs/016
> index 35609329..37119363 100755
> --- a/tests/btrfs/016
> +++ b/tests/btrfs/016
> @@ -12,58 +12,48 @@ _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"
>
> _scratch_mkfs > /dev/null 2>&1
> -
> -#receive needs to be able to setxattrs, including the selinux context, if we use
> -#the normal nfs context thing it screws up our ability to set the
> -#security.selinux xattrs so we need to disable this for this test
> -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 \
> +# -A disable access time check.
> +# And -T disable xattrs to prevent SELinux changes causing false alerts, and the
> +# test case only cares about hole punching.
> +$FSSUM_PROG -AT -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 -AT -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 \
> @@ -75,4 +65,5 @@ $FSSUM_PROG -r $tmp/fssum.snap1 $SCRATCH_MNT/snap1 >> $seqres.full 2>&1 \
> || _fail "fssum failed"
>
> echo "Silence is golden"
> -status=0 ; exit
> +status=0
> +exit
This hunk is unrelated and unnecessary.
But other than that, it looks good to me:
Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
Thanks.
> --
> 2.43.1
>
>
