All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
@ 2017-04-04  6:34 fdmanana
  2017-04-04  6:34 ` [PATCH v2 2/3] generic: test for number of bytes used by files after buffered writes fdmanana
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: fdmanana @ 2017-04-04  6:34 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, Filipe Manana

From: Filipe Manana <fdmanana@suse.com>

For example NFS 4.2 supports fallocate but it does not support its
KEEP_SIZE flag, so we want to skip tests that use fallocate with that
flag on filesystems that don't support it.

Suggested-by: Eryu Guan <eguan@redhat.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 common/rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index e1ab2c6..3d0f089 100644
--- a/common/rc
+++ b/common/rc
@@ -2021,8 +2021,8 @@ _require_xfs_io_command()
 	"chproj")
 		testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
 		;;
-	"falloc" )
-		testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
+	"falloc*" )
+		testio=`$XFS_IO_PROG -F -f -c "$command 0 1m" $testfile 2>&1`
 		;;
 	"fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
 		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
-- 
2.7.0.rc3


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

* [PATCH v2 2/3] generic: test for number of bytes used by files after buffered writes
  2017-04-04  6:34 [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags fdmanana
@ 2017-04-04  6:34 ` fdmanana
  2017-04-04  6:34 ` [PATCH 3/3] generic/071: check that the fs supports fallocate with the KEEP_SIZE flag fdmanana
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: fdmanana @ 2017-04-04  6:34 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, Filipe Manana

From: Filipe Manana <fdmanana@suse.com>

Test that a filesystem's implementation of the stat(2) system call
reports correct values for the number of blocks allocated for a file
when there are delayed allocations.

This test is motivated by a bug in btrfs which is fixed by the following
path for the linux kernel:

 "Btrfs: fix reported number of inode blocks"

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: Added missing _require_odirect and make the test check that the tested
    filesystem supports fallocate with the KEEP_SIZE flag.

 tests/generic/422     | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/422.out |  41 ++++++++++++++++
 tests/generic/group   |   1 +
 3 files changed, 169 insertions(+)
 create mode 100755 tests/generic/422
 create mode 100644 tests/generic/422.out

diff --git a/tests/generic/422 b/tests/generic/422
new file mode 100755
index 0000000..cae76a9
--- /dev/null
+++ b/tests/generic/422
@@ -0,0 +1,127 @@
+#! /bin/bash
+# FS QA Test No. generic/422
+#
+# Test that a filesystem's implementation of the stat(2) system call reports
+# correct values for the number of blocks allocated for a file when there are
+# delayed allocations.
+#
+#-----------------------------------------------------------------------
+#
+# Copyright (C) 2017 SUSE Linux Products GmbH. All Rights Reserved.
+# Author: Filipe Manana <fdmanana@suse.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+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
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_scratch
+_require_xfs_io_command "falloc -k"
+_require_odirect
+
+rm -f $seqres.full
+
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 64K" $SCRATCH_MNT/foo1 | _filter_xfs_io
+$XFS_IO_PROG -f \
+     -c "pwrite -S 0xaa 0 64K" \
+     -c "truncate 128K" \
+     $SCRATCH_MNT/foo2 | _filter_xfs_io
+$XFS_IO_PROG -f \
+     -c "falloc -k 0 128K" \
+     -c "pwrite -S 0xaa 0 64K" \
+     $SCRATCH_MNT/foo3 | _filter_xfs_io
+touch $SCRATCH_MNT/foo4
+
+# Make sure everything done so far is durably persisted.
+sync
+
+# Now overwrite the extent of the first file.
+$XFS_IO_PROG -c "pwrite -S 0xff 0 64K" $SCRATCH_MNT/foo1 | _filter_xfs_io
+
+# Write to a hole of the second file.
+$XFS_IO_PROG -c "pwrite -S 0xff 64K 64K" $SCRATCH_MNT/foo2 | _filter_xfs_io
+# Write again to the same location, just to test that the fs will not account
+# the same write twice.
+$XFS_IO_PROG -c "pwrite -S 0x20 64K 64K" $SCRATCH_MNT/foo2 | _filter_xfs_io
+
+# Write beyond eof of the third file into the pre-allocated extent.
+$XFS_IO_PROG -c "pwrite -S 0xff 64K 64K" $SCRATCH_MNT/foo3 | _filter_xfs_io
+
+# Do a buffered write immediately followed by a direct IO write, without a
+# fsync in between, just to test that page invalidation does not lead to an
+# incorrect number of file blocks reported.
+$XFS_IO_PROG -c "pwrite -S 0xab 0 64K" $SCRATCH_MNT/foo4 | _filter_xfs_io
+$XFS_IO_PROG -d -c "pwrite -S 0xef 0 64K" $SCRATCH_MNT/foo4 | _filter_xfs_io
+
+echo
+echo "Before writeback"
+echo
+
+echo "Space used by file foo1:"
+du -h $SCRATCH_MNT/foo1 | _filter_scratch
+
+echo "Space used by file foo2:"
+du -h $SCRATCH_MNT/foo2 | _filter_scratch
+
+echo "Space used by file foo3:"
+du -h $SCRATCH_MNT/foo3 | _filter_scratch
+
+echo "Space used by file foo4:"
+du -h $SCRATCH_MNT/foo4 | _filter_scratch
+
+sync
+
+# We expect the same file sizes reported by 'du' after writeback finishes.
+echo
+echo "After writeback"
+echo
+
+echo "Space used by file foo1:"
+du -h $SCRATCH_MNT/foo1 | _filter_scratch
+
+echo "Space used by file foo2:"
+du -h $SCRATCH_MNT/foo2 | _filter_scratch
+
+echo "Space used by file foo3:"
+du -h $SCRATCH_MNT/foo3 | _filter_scratch
+
+echo "Space used by file foo4:"
+du -h $SCRATCH_MNT/foo4 | _filter_scratch
+
+status=0
+exit
diff --git a/tests/generic/422.out b/tests/generic/422.out
new file mode 100644
index 0000000..3696088
--- /dev/null
+++ b/tests/generic/422.out
@@ -0,0 +1,41 @@
+QA output created by 422
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 65536
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 65536
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 65536
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Before writeback
+
+Space used by file foo1:
+64K	SCRATCH_MNT/foo1
+Space used by file foo2:
+128K	SCRATCH_MNT/foo2
+Space used by file foo3:
+128K	SCRATCH_MNT/foo3
+Space used by file foo4:
+64K	SCRATCH_MNT/foo4
+
+After writeback
+
+Space used by file foo1:
+64K	SCRATCH_MNT/foo1
+Space used by file foo2:
+128K	SCRATCH_MNT/foo2
+Space used by file foo3:
+128K	SCRATCH_MNT/foo3
+Space used by file foo4:
+64K	SCRATCH_MNT/foo4
diff --git a/tests/generic/group b/tests/generic/group
index 3c7c5e4..d747385 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -424,3 +424,4 @@
 419 auto quick encrypt
 420 auto quick punch
 421 auto quick encrypt dangerous
+422 auto quick
-- 
2.7.0.rc3


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

* [PATCH 3/3] generic/071: check that the fs supports fallocate with the KEEP_SIZE flag
  2017-04-04  6:34 [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags fdmanana
  2017-04-04  6:34 ` [PATCH v2 2/3] generic: test for number of bytes used by files after buffered writes fdmanana
@ 2017-04-04  6:34 ` fdmanana
  2017-04-06 14:18 ` [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags Eryu Guan
  2017-04-07  8:51 ` Eryu Guan
  3 siblings, 0 replies; 9+ messages in thread
From: fdmanana @ 2017-04-04  6:34 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, Filipe Manana

From: Filipe Manana <fdmanana@suse.com>

So that the test is skipped for filesystems that don't support it instead
of failing (like NFS 4.2 for example).

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/generic/071 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/071 b/tests/generic/071
index 65ed0c7..c1f90c6 100755
--- a/tests/generic/071
+++ b/tests/generic/071
@@ -46,7 +46,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fs generic
 _supported_os Linux
 _require_scratch
-_require_xfs_io_command "falloc"
+_require_xfs_io_command "falloc -k"
 
 rm -f $seqres.full
 
-- 
2.7.0.rc3


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

* Re: [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
  2017-04-04  6:34 [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags fdmanana
  2017-04-04  6:34 ` [PATCH v2 2/3] generic: test for number of bytes used by files after buffered writes fdmanana
  2017-04-04  6:34 ` [PATCH 3/3] generic/071: check that the fs supports fallocate with the KEEP_SIZE flag fdmanana
@ 2017-04-06 14:18 ` Eryu Guan
  2017-04-06 14:20   ` Filipe Manana
  2017-04-07  8:51 ` Eryu Guan
  3 siblings, 1 reply; 9+ messages in thread
From: Eryu Guan @ 2017-04-06 14:18 UTC (permalink / raw)
  To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana

On Tue, Apr 04, 2017 at 07:34:29AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> For example NFS 4.2 supports fallocate but it does not support its
> KEEP_SIZE flag, so we want to skip tests that use fallocate with that
> flag on filesystems that don't support it.
> 
> Suggested-by: Eryu Guan <eguan@redhat.com>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  common/rc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index e1ab2c6..3d0f089 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2021,8 +2021,8 @@ _require_xfs_io_command()
>  	"chproj")
>  		testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
>  		;;
> -	"falloc" )
> -		testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
> +	"falloc*" )

This doesn't work as expected with quotes. I can remove the quotes at
commit time though.

Thanks,
Eryu

> +		testio=`$XFS_IO_PROG -F -f -c "$command 0 1m" $testfile 2>&1`
>  		;;
>  	"fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
>  		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> -- 
> 2.7.0.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
  2017-04-06 14:18 ` [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags Eryu Guan
@ 2017-04-06 14:20   ` Filipe Manana
  2017-04-06 14:28     ` David Sterba
  0 siblings, 1 reply; 9+ messages in thread
From: Filipe Manana @ 2017-04-06 14:20 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-btrfs, Filipe Manana

On Thu, Apr 6, 2017 at 3:18 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Tue, Apr 04, 2017 at 07:34:29AM +0100, fdmanana@kernel.org wrote:
>> From: Filipe Manana <fdmanana@suse.com>
>>
>> For example NFS 4.2 supports fallocate but it does not support its
>> KEEP_SIZE flag, so we want to skip tests that use fallocate with that
>> flag on filesystems that don't support it.
>>
>> Suggested-by: Eryu Guan <eguan@redhat.com>
>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>> ---
>>  common/rc | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/rc b/common/rc
>> index e1ab2c6..3d0f089 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2021,8 +2021,8 @@ _require_xfs_io_command()
>>       "chproj")
>>               testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
>>               ;;
>> -     "falloc" )
>> -             testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
>> +     "falloc*" )
>
> This doesn't work as expected with quotes. I can remove the quotes at
> commit time though.

Hum, it did work for me, strange.

But please do, thanks.

>
> Thanks,
> Eryu
>
>> +             testio=`$XFS_IO_PROG -F -f -c "$command 0 1m" $testfile 2>&1`
>>               ;;
>>       "fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
>>               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
>> --
>> 2.7.0.rc3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
  2017-04-06 14:20   ` Filipe Manana
@ 2017-04-06 14:28     ` David Sterba
  0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2017-04-06 14:28 UTC (permalink / raw)
  To: Filipe Manana; +Cc: Eryu Guan, fstests, linux-btrfs, Filipe Manana

On Thu, Apr 06, 2017 at 03:20:43PM +0100, Filipe Manana wrote:
> On Thu, Apr 6, 2017 at 3:18 PM, Eryu Guan <eguan@redhat.com> wrote:
> > On Tue, Apr 04, 2017 at 07:34:29AM +0100, fdmanana@kernel.org wrote:
> >> From: Filipe Manana <fdmanana@suse.com>
> >>
> >> For example NFS 4.2 supports fallocate but it does not support its
> >> KEEP_SIZE flag, so we want to skip tests that use fallocate with that
> >> flag on filesystems that don't support it.
> >>
> >> Suggested-by: Eryu Guan <eguan@redhat.com>
> >> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> >> ---
> >>  common/rc | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/rc b/common/rc
> >> index e1ab2c6..3d0f089 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2021,8 +2021,8 @@ _require_xfs_io_command()
> >>       "chproj")
> >>               testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
> >>               ;;
> >> -     "falloc" )
> >> -             testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
> >> +     "falloc*" )
> >
> > This doesn't work as expected with quotes. I can remove the quotes at
> > commit time though.
> 
> Hum, it did work for me, strange.

Quoted globs don't work the same way as unquoted inside case, this

#!/bin/sh
i=abc
case $i in
	abc*) echo notquoted;;
	"abc*") echo quoted;;
esac
---

will return 'notquoted'

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

* Re: [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
  2017-04-04  6:34 [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags fdmanana
                   ` (2 preceding siblings ...)
  2017-04-06 14:18 ` [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags Eryu Guan
@ 2017-04-07  8:51 ` Eryu Guan
  2017-04-07  9:13   ` Filipe Manana
  3 siblings, 1 reply; 9+ messages in thread
From: Eryu Guan @ 2017-04-07  8:51 UTC (permalink / raw)
  To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana

On Tue, Apr 04, 2017 at 07:34:29AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> For example NFS 4.2 supports fallocate but it does not support its
> KEEP_SIZE flag, so we want to skip tests that use fallocate with that
> flag on filesystems that don't support it.
> 
> Suggested-by: Eryu Guan <eguan@redhat.com>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  common/rc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index e1ab2c6..3d0f089 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2021,8 +2021,8 @@ _require_xfs_io_command()
>  	"chproj")
>  		testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
>  		;;
> -	"falloc" )
> -		testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
> +	"falloc*" )
> +		testio=`$XFS_IO_PROG -F -f -c "$command 0 1m" $testfile 2>&1`

Sorry, I was wrong about this. It would break the subsequent
$XFS_IO_PROG -c "help $command" | grep ... command if another $param is
specified. Seems adding $param to falloc command is the right way, as
what Darrick did to fiemap in his new test.

-               testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
+               testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1`

Do you mind me updating these three patches accordingly? Or can you send
out new version if you like?

Thanks! And sorry again!

Eryu

P.S. I'm thinking of converting all the case switches (except the
default one) in _require_xfs_io_command() to actually run the $command
with $param, and doing other cleanups, but that won't block this patch
and I can do it in another patch.

>  		;;
>  	"fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
>  		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> -- 
> 2.7.0.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
  2017-04-07  8:51 ` Eryu Guan
@ 2017-04-07  9:13   ` Filipe Manana
  2017-04-07  9:45     ` Eryu Guan
  0 siblings, 1 reply; 9+ messages in thread
From: Filipe Manana @ 2017-04-07  9:13 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-btrfs, Filipe Manana

On Fri, Apr 7, 2017 at 9:51 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Tue, Apr 04, 2017 at 07:34:29AM +0100, fdmanana@kernel.org wrote:
>> From: Filipe Manana <fdmanana@suse.com>
>>
>> For example NFS 4.2 supports fallocate but it does not support its
>> KEEP_SIZE flag, so we want to skip tests that use fallocate with that
>> flag on filesystems that don't support it.
>>
>> Suggested-by: Eryu Guan <eguan@redhat.com>
>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>> ---
>>  common/rc | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/rc b/common/rc
>> index e1ab2c6..3d0f089 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2021,8 +2021,8 @@ _require_xfs_io_command()
>>       "chproj")
>>               testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
>>               ;;
>> -     "falloc" )
>> -             testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
>> +     "falloc*" )
>> +             testio=`$XFS_IO_PROG -F -f -c "$command 0 1m" $testfile 2>&1`
>
> Sorry, I was wrong about this. It would break the subsequent
> $XFS_IO_PROG -c "help $command" | grep ... command if another $param is
> specified.

Yeah I had noticed that because the following won't cause the return anymore:

test -z "$param" && return

> Seems adding $param to falloc command is the right way, as
> what Darrick did to fiemap in his new test.
>
> -               testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
> +               testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1`

But in that case grepping the help output, at the very end of the
function, will fail for falloc since its help output fails to match
the grep pattern (as highlighted in the thread you pointed before).
So that grep pattern would have to change as well.

>
> Do you mind me updating these three patches accordingly? Or can you send
> out new version if you like?

Sure, fell free to update it as you feel it's the best way.

Thanks!

>
> Thanks! And sorry again!
>
> Eryu
>
> P.S. I'm thinking of converting all the case switches (except the
> default one) in _require_xfs_io_command() to actually run the $command
> with $param, and doing other cleanups, but that won't block this patch
> and I can do it in another patch.
>
>>               ;;
>>       "fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
>>               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
>> --
>> 2.7.0.rc3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags
  2017-04-07  9:13   ` Filipe Manana
@ 2017-04-07  9:45     ` Eryu Guan
  0 siblings, 0 replies; 9+ messages in thread
From: Eryu Guan @ 2017-04-07  9:45 UTC (permalink / raw)
  To: Filipe Manana; +Cc: fstests, linux-btrfs, Filipe Manana

On Fri, Apr 07, 2017 at 10:13:09AM +0100, Filipe Manana wrote:
> On Fri, Apr 7, 2017 at 9:51 AM, Eryu Guan <eguan@redhat.com> wrote:
> > On Tue, Apr 04, 2017 at 07:34:29AM +0100, fdmanana@kernel.org wrote:
> >> From: Filipe Manana <fdmanana@suse.com>
> >>
> >> For example NFS 4.2 supports fallocate but it does not support its
> >> KEEP_SIZE flag, so we want to skip tests that use fallocate with that
> >> flag on filesystems that don't support it.
> >>
> >> Suggested-by: Eryu Guan <eguan@redhat.com>
> >> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> >> ---
> >>  common/rc | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/rc b/common/rc
> >> index e1ab2c6..3d0f089 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2021,8 +2021,8 @@ _require_xfs_io_command()
> >>       "chproj")
> >>               testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
> >>               ;;
> >> -     "falloc" )
> >> -             testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
> >> +     "falloc*" )
> >> +             testio=`$XFS_IO_PROG -F -f -c "$command 0 1m" $testfile 2>&1`
> >
> > Sorry, I was wrong about this. It would break the subsequent
> > $XFS_IO_PROG -c "help $command" | grep ... command if another $param is
> > specified.
> 
> Yeah I had noticed that because the following won't cause the return anymore:
> 
> test -z "$param" && return
> 
> > Seems adding $param to falloc command is the right way, as
> > what Darrick did to fiemap in his new test.
> >
> > -               testio=`$XFS_IO_PROG -F -f -c "falloc 0 1m" $testfile 2>&1`
> > +               testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1`
> 
> But in that case grepping the help output, at the very end of the
> function, will fail for falloc since its help output fails to match
> the grep pattern (as highlighted in the thread you pointed before).
> So that grep pattern would have to change as well.

This should be fixed by xfsprogs commit 0c2ed80a3590 ("xfs_io: provide
long-format help for falloc"). But I agreed searching the help message
again for it would be redundant. But perhaps that needs wider review and
doesn't belong to this patch. I'll try to do that in my cleanup patch
for _require_xfs_io_command().

Thanks,
Eryu

> 
> >
> > Do you mind me updating these three patches accordingly? Or can you send
> > out new version if you like?
> 
> Sure, fell free to update it as you feel it's the best way.
> 
> Thanks!
> 
> >
> > Thanks! And sorry again!
> >
> > Eryu
> >
> > P.S. I'm thinking of converting all the case switches (except the
> > default one) in _require_xfs_io_command() to actually run the $command
> > with $param, and doing other cleanups, but that won't block this patch
> > and I can do it in another patch.
> >
> >>               ;;
> >>       "fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
> >>               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> >> --
> >> 2.7.0.rc3
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe fstests" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2017-04-07  9:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-04  6:34 [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags fdmanana
2017-04-04  6:34 ` [PATCH v2 2/3] generic: test for number of bytes used by files after buffered writes fdmanana
2017-04-04  6:34 ` [PATCH 3/3] generic/071: check that the fs supports fallocate with the KEEP_SIZE flag fdmanana
2017-04-06 14:18 ` [PATCH 1/3] common/rc: test that xfs_io's falloc command supports specific flags Eryu Guan
2017-04-06 14:20   ` Filipe Manana
2017-04-06 14:28     ` David Sterba
2017-04-07  8:51 ` Eryu Guan
2017-04-07  9:13   ` Filipe Manana
2017-04-07  9:45     ` Eryu Guan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.