linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] fstests: generic: Test if we can still do operations which don't take extra data space on a fs without data space
@ 2019-05-08  9:45 Qu Wenruo
  2019-05-08 10:38 ` Filipe Manana
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2019-05-08  9:45 UTC (permalink / raw)
  To: linux-btrfs, fstests

This test will test if we can still do the following operations when a
fs has no extra data space:
- buffered write into unpopulated preallocated extent
- clone the untouched preallocated extent
- fsync
Above operations in a row should not fail, as they takes no extra data space.

Xfs passes the test, while btrfs fails at fsync.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
- Change the comment and commit message to make it describe the test
  itself, not the btrfs specific part.
- Use $XFS_IO_PROG to replace xfs_io.
---
 tests/generic/545     | 70 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/545.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 73 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..9efaf58c
--- /dev/null
+++ b/tests/generic/545
@@ -0,0 +1,70 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 545
+#
+# Test when a fs is full we can still:
+# - Do buffered write into a unpopulated preallocated extent
+# - Clone the untouched part of that preallocated extent
+# - Fsync
+# All operations above should not fail.
+#
+# Xfs passes the test while btrfs fails. The fix for btrfs is:
+# "btrfs: Flush before reflinking any extent to prevent NOCOW write falling
+#  back to CoW without data reservation"
+#
+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
+_scratch_mount
+
+# Create preallocated extent where we can write into
+$XFS_IO_PROG -f -c 'falloc 8k 64m' "$SCRATCH_MNT/foobar" >> $seqres.full
+
+# Use up all data space, to test later write-into-preallocate behavior
+_pwrite_byte 0x00 0 512m "$SCRATCH_MNT/padding" >> $seqres.full 2>&1
+
+# This should not fail
+_pwrite_byte 0xcd 1m 16m "$SCRATCH_MNT/foobar" >> $seqres.full
+
+# Do reflink here, we shouldn't use extra data space, thus it should not fail
+$XFS_IO_PROG -c "reflink ${SCRATCH_MNT}/foobar 8k 0 4k" "$SCRATCH_MNT/foobar" \
+	>> $seqres.full
+
+# Fsync to check if writeback is ok
+$XFS_IO_PROG -c 'fsync'  "$SCRATCH_MNT/foobar"
+
+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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] fstests: generic: Test if we can still do operations which don't take extra data space on a fs without data space
  2019-05-08  9:45 [PATCH v2] fstests: generic: Test if we can still do operations which don't take extra data space on a fs without data space Qu Wenruo
@ 2019-05-08 10:38 ` Filipe Manana
  2019-05-08 10:44   ` Qu Wenruo
  0 siblings, 1 reply; 3+ messages in thread
From: Filipe Manana @ 2019-05-08 10:38 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, fstests

On Wed, May 8, 2019 at 11:06 AM Qu Wenruo <wqu@suse.com> wrote:
>
> This test will test if we can still do the following operations when a
> fs has no extra data space:
> - buffered write into unpopulated preallocated extent
> - clone the untouched preallocated extent
> - fsync
> Above operations in a row should not fail, as they takes no extra data space.
>
> Xfs passes the test, while btrfs fails at fsync.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v2:
> - Change the comment and commit message to make it describe the test
>   itself, not the btrfs specific part.
> - Use $XFS_IO_PROG to replace xfs_io.
> ---
>  tests/generic/545     | 70 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/545.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 73 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..9efaf58c
> --- /dev/null
> +++ b/tests/generic/545
> @@ -0,0 +1,70 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 545
> +#
> +# Test when a fs is full we can still:
> +# - Do buffered write into a unpopulated preallocated extent
> +# - Clone the untouched part of that preallocated extent
> +# - Fsync
> +# All operations above should not fail.
> +#
> +# Xfs passes the test while btrfs fails. The fix for btrfs is:
> +# "btrfs: Flush before reflinking any extent to prevent NOCOW write falling
> +#  back to CoW without data reservation"

Again, this part that says it currently fails on btrfs, passes on xfs
and which patch fixes the problem for btrfs, should be in the
changelog and not on the test description.

> +#
> +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

Not needed, because below you do _require_scratch_reflink

> +_require_scratch_reflink
> +
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
> +_scratch_mount
> +
> +# Create preallocated extent where we can write into
> +$XFS_IO_PROG -f -c 'falloc 8k 64m' "$SCRATCH_MNT/foobar" >> $seqres.full
> +
> +# Use up all data space, to test later write-into-preallocate behavior
> +_pwrite_byte 0x00 0 512m "$SCRATCH_MNT/padding" >> $seqres.full 2>&1
> +
> +# This should not fail
> +_pwrite_byte 0xcd 1m 16m "$SCRATCH_MNT/foobar" >> $seqres.full
> +
> +# Do reflink here, we shouldn't use extra data space, thus it should not fail
> +$XFS_IO_PROG -c "reflink ${SCRATCH_MNT}/foobar 8k 0 4k" "$SCRATCH_MNT/foobar" \
> +       >> $seqres.full
> +
> +# Fsync to check if writeback is ok
> +$XFS_IO_PROG -c 'fsync'  "$SCRATCH_MNT/foobar"

It would be more useful to not only check that fsync does not return
an error, but also to check that if a power failure happens, when we
mount the fs again all the data previously written is there.

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.”

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] fstests: generic: Test if we can still do operations which don't take extra data space on a fs without data space
  2019-05-08 10:38 ` Filipe Manana
@ 2019-05-08 10:44   ` Qu Wenruo
  0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2019-05-08 10:44 UTC (permalink / raw)
  To: fdmanana, Qu Wenruo; +Cc: linux-btrfs, fstests


[-- Attachment #1.1: Type: text/plain, Size: 4532 bytes --]



On 2019/5/8 下午6:38, Filipe Manana wrote:
> On Wed, May 8, 2019 at 11:06 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> This test will test if we can still do the following operations when a
>> fs has no extra data space:
>> - buffered write into unpopulated preallocated extent
>> - clone the untouched preallocated extent
>> - fsync
>> Above operations in a row should not fail, as they takes no extra data space.
>>
>> Xfs passes the test, while btrfs fails at fsync.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> changelog:
>> v2:
>> - Change the comment and commit message to make it describe the test
>>   itself, not the btrfs specific part.
>> - Use $XFS_IO_PROG to replace xfs_io.
>> ---
>>  tests/generic/545     | 70 +++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/545.out |  2 ++
>>  tests/generic/group   |  1 +
>>  3 files changed, 73 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..9efaf58c
>> --- /dev/null
>> +++ b/tests/generic/545
>> @@ -0,0 +1,70 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
>> +#
>> +# FS QA Test 545
>> +#
>> +# Test when a fs is full we can still:
>> +# - Do buffered write into a unpopulated preallocated extent
>> +# - Clone the untouched part of that preallocated extent
>> +# - Fsync
>> +# All operations above should not fail.
>> +#
>> +# Xfs passes the test while btrfs fails. The fix for btrfs is:
>> +# "btrfs: Flush before reflinking any extent to prevent NOCOW write falling
>> +#  back to CoW without data reservation"
> 
> Again, this part that says it currently fails on btrfs, passes on xfs
> and which patch fixes the problem for btrfs, should be in the
> changelog and not on the test description.
> 
>> +#
>> +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
> 
> Not needed, because below you do _require_scratch_reflink
> 
>> +_require_scratch_reflink
>> +
>> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
>> +_scratch_mount
>> +
>> +# Create preallocated extent where we can write into
>> +$XFS_IO_PROG -f -c 'falloc 8k 64m' "$SCRATCH_MNT/foobar" >> $seqres.full
>> +
>> +# Use up all data space, to test later write-into-preallocate behavior
>> +_pwrite_byte 0x00 0 512m "$SCRATCH_MNT/padding" >> $seqres.full 2>&1
>> +
>> +# This should not fail
>> +_pwrite_byte 0xcd 1m 16m "$SCRATCH_MNT/foobar" >> $seqres.full
>> +
>> +# Do reflink here, we shouldn't use extra data space, thus it should not fail
>> +$XFS_IO_PROG -c "reflink ${SCRATCH_MNT}/foobar 8k 0 4k" "$SCRATCH_MNT/foobar" \
>> +       >> $seqres.full
>> +
>> +# Fsync to check if writeback is ok
>> +$XFS_IO_PROG -c 'fsync'  "$SCRATCH_MNT/foobar"
> 
> It would be more useful to not only check that fsync does not return
> an error, but also to check that if a power failure happens, when we
> mount the fs again all the data previously written is there.

A quick glance into the group shows no enospc test with reflink/clone
and log/replay.

So it makes sense.

Thanks,
Qu

> 
> 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
>>
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-05-08 10:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08  9:45 [PATCH v2] fstests: generic: Test if we can still do operations which don't take extra data space on a fs without data space Qu Wenruo
2019-05-08 10:38 ` Filipe Manana
2019-05-08 10:44   ` Qu Wenruo

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 NNTP newsgroup(s).