On Wed, May 8, 2019 at 8:48 AM Qu Wenruo <wqu@xxxxxxxx> wrote: > > This test case is going to check if btrfs will fail fsync after NOCOW > buffered write and reflink. > > Btrfs' back reference only has extent level granularity, so if we do > buffered write which can be done NOCOW, then reflink, that buffered > write will be forced CoW. > > And if we have no data space left, CoW will fail and cause fsync > failure. The changelog, for a generic test, should describe in general terms what the test is testing, not how/why btrfs fails, neither the btrfs implementation details. Here in changelog you can say the test currently fails on btrfs and then mention which patch/commit fixes it. > > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx> > --- > tests/generic/545 | 82 +++++++++++++++++++++++++++++++++++++++++++ > tests/generic/545.out | 2 ++ > tests/generic/group | 1 + > 3 files changed, 85 insertions(+) > create mode 100755 tests/generic/545 > create mode 100644 tests/generic/545.out > > diff --git a/tests/generic/545 b/tests/generic/545 > new file mode 100755 > index 00000000..b6e1a0ae > --- /dev/null > +++ b/tests/generic/545 > @@ -0,0 +1,82 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved. > +# > +# FS QA Test 545 > +# > +# Test if btrfs fails fsync due to ENOSPC. > +# > +# Btrfs' back reference only has extent level granularity, thus if reflink of > +# an preallocated extent happens, any write into that extent must be CoWed. > +# > +# We could craft a case, where btrfs reserve no data space at buffered write > +# time but are forced to do CoW at writeback, and fail fsync. > +# > +# This is fixed by "btrfs: Flush before reflinking any extent to prevent NOCOW > +# write falling back to CoW without data reservation" Same as before, it's a generic test, you should describe what we are testing, not that btrfs fails and why/how, nor btrfs' specific implementation details. I've been pointed about doing similar in the past, see https://marc.info/?l=linux-btrfs&m=142482279823445&w=2 I would just say: Test that if we do a buffered write against a section of an unwritten extent, reflink a different section of the extent and then fsync the file, the fsync will succeed and the buffered write is persisted. > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > +. ./common/reflink > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > + > +# Modify as appropriate. > +_supported_fs generic > +_supported_os Linux > +_require_scratch > +_require_scratch_reflink > + > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1 > + > +# Space cache will use some data space and may cause interference. > +# Disable space cache here. > +_scratch_mount -o nospace_cache Have you tested this on other filesystems? This will fail, nospace_cache is btrfs specific... > + > +# Create preallocated extent where we can do NOCOW write > +xfs_io -f -c 'falloc 8k 64m' "$SCRATCH_MNT/foobar" >> $seqres.full xfs_io -> $XFS_IO_PROG > + > +# Use up all remaining space, so that later write will go through NOCOW check > +# We ignore the ENOSPC error here Again that's a very specific btrfs detail - that btrfs will only "enter" NOCOW mode when there's not enough available data space at the time of the buffered write. > +_pwrite_byte 0x00 0 512m "$SCRATCH_MNT/padding" >> $seqres.full 2>&1 > + > +# Now setup is all done. > +sync Is the sync needed? Why? I don't think it's needed. > + > +# This buffered write will go through and pass NOCOW check thus no > +# data space is reserved. Again, very btrfs specific . > +_pwrite_byte 0xcd 1m 16m "$SCRATCH_MNT/foobar" >> $seqres.full > + > +# Reflink the the unused part of the preallocated extent to increase > +# its reference, so for btrfs any write into that preallocated extent > +# must be CoWed. Missing 'count' after 'reference'. Also too much btrfs specific. > +xfs_io -c "reflink ${SCRATCH_MNT}/foobar 8k 0 4k" "$SCRATCH_MNT/foobar" \ > + >> $seqres.full xfs_io -> $XFS_IO_PROG > + > +# Now fsync will fail due to we must CoW previous NOCOW write, but we have > +# now data space left, it will fail with ENOSPC Again, describing the btrfs specific implementation details/failure, instead of what we are testing (that the fsync succeeds). > +xfs_io -c 'fsync' "$SCRATCH_MNT/foobar" xfs_io -> $XFS_IO_PROG Thanks. > + > +echo "Silence is golden" > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/545.out b/tests/generic/545.out > new file mode 100644 > index 00000000..920d7244 > --- /dev/null > +++ b/tests/generic/545.out > @@ -0,0 +1,2 @@ > +QA output created by 545 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index 40deb4d0..f26b91fe 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -547,3 +547,4 @@ > 542 auto quick clone > 543 auto quick clone > 544 auto quick clone > +545 auto quick clone enospc > -- > 2.21.0 > -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.”
