From: Filipe Manana <fdmanana@kernel.org>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH] btrfs/016: fix a false alert due to xattrs mismatch
Date: Mon, 19 Feb 2024 12:11:58 +0000 [thread overview]
Message-ID: <CAL3q7H69SsWkmvysP-mDm1h6DJ1YMFSRkzs87yyQG2YbgRt26Q@mail.gmail.com> (raw)
In-Reply-To: <20240219080007.70318-1-wqu@suse.com>
On Mon, Feb 19, 2024 at 8:18 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]
> 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@suse.com>
> ---
> 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@suse.com>
Thanks.
> --
> 2.43.1
>
>
next prev parent reply other threads:[~2024-02-19 12:12 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 8:00 [PATCH] btrfs/016: fix a false alert due to xattrs mismatch Qu Wenruo
2024-02-19 12:11 ` Filipe Manana [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-12-28 5:36 Qu Wenruo
2024-02-17 9:31 ` Qu Wenruo
2024-02-17 12:25 ` Filipe Manana
2024-02-17 20:39 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAL3q7H69SsWkmvysP-mDm1h6DJ1YMFSRkzs87yyQG2YbgRt26Q@mail.gmail.com \
--to=fdmanana@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).