On 22.11.2017 08:47, Eryu Guan wrote:
> On Tue, Nov 21, 2017 at 05:16:56PM +0200, Nikolay Borisov wrote:
>> Fiemap gained support for passing in optional offset len
>> which denote the range requested, so this patch adds
>> testcases for this functionality. Aditionally, a special "ranged"
>> argument is added to the require_xfs_io_command which checks
>> for the presence of fiemap range support.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
>> ---
>> V5:
>> * Drop changes to existing generic punch hole tests since
>> the new fiemap implementation don't require them
>> * Merge the common/rc change with this patch
>> * Added Data + Hole + Data and Hole + Data tests as per Eryu's request
>> * Adjusted output of some tests which fall in a hole, since holes are
>> truncated to passed range
>> * Simplified the logic to check for fiemap range support (Eryu)
>
> Thanks for the 5th version, looks good overall, though I have more
> comments, sorry for not bringing them up in previous reviews...
>
>>
>> V4:
>> * Added test description
>> * Added new test for past-eof behavior
>> * Removed supported_generic_fs line
>> * Switched to using the "ranged" param require
>> * Revert v3 changes
>>
>> V3:
>> * Adjusted tests for '-r' fiemap param
>> * Tests for invalid -r combination
>>
>> V2: No change
>> V1: No change
>> common/rc | 7 +++
>> tests/xfs/900 | 101 +++++++++++++++++++++++++++++++++++++++++++
>> tests/xfs/900.out | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 233 insertions(+)
>> create mode 100755 tests/xfs/900
>> create mode 100644 tests/xfs/900.out
>>
>> diff --git a/common/rc b/common/rc
>> index e2a8229..d0dd32d 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2053,6 +2053,13 @@ _require_xfs_io_command()
>> -c "$command 4k 8k" $testfile 2>&1`
>> ;;
>> "fiemap")
>> + # If 'ranged' is passed as argument then we check to see if fiemap supports
>> + # ranged query params
>> + if echo "$param" | grep -q "ranged"; then
>> + param=$(echo "$param" | sed "s/ranged//")
>> + $XFS_IO_PROG -c "help fiemap" | grep -q "\[offset \[len\]\]"
>> + [ $? -eq 0 ] || _notrun "xfs_io $command ranged support is missing"
>> + fi
>> testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
>> -c "fiemap -v $param" $testfile 2>&1`
>> param_checked=1
>> diff --git a/tests/xfs/900 b/tests/xfs/900
>> new file mode 100755
>> index 0000000..07f4f40
>> --- /dev/null
>> +++ b/tests/xfs/900
>> @@ -0,0 +1,101 @@
>> +#! /bin/bash
>> +# FS QA Test No. 900
>> +
>> +# Test for the new ranged query functionality in xfs_io's fiemap command.
>> +# This tests various combinations of hole + data layout being printed.
>> +# Also the test used 16k holes to be compatible with 16k block filesystems
>> +
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2017 SUSE Linux Products GmbH. All Rights Reserved.
>> +# Author: Nikolay Borisov <nborisov@xxxxxxxx>
>> +#
>> +# 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"
>> +
>> +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/punch
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +
>> +# Modify as appropriate.
>> +_supported_os Linux
>> +_require_scratch
>
> This test is in xfs dir and that means it's an xfs-specific test, better
> to have "_supported_fs xfs" for consistency.
>
> But I'm wondering why not make it a generic test? There seems nothing
> xfs-specific. And I hacked the test and tested with ext4/btrfs, tests
> all passed.
>
> Interesting thing is, test failed on XFS as:
>
> Hole + Data
> 0: [0..31]: hole
> -1: [32..63]: unwritten
> +1: [32..33]: unwritten
> Hole + Data + Hole
> 0: [64..95]: hole
> 1: [96..127]: unwritten
> @@ -14,7 +14,7 @@
> Data + Hole + Data
> 0: [32..63]: unwritten
> 1: [64..95]: hole
> -2: [96..127]: unwritten
> +2: [96..97]: unwritten
This is due to xfs always trimming extents. I've sent a patch:
xfs: Don't trim extents for fiemap
That harmonizes xfs' behavior in that regard to the other major
filesystems by not trimming extents when called on behalf of iomap. For
once it wasn't a problem in xfs_io's fiemap code :D
>
> The data extents were truncated. I was using Eric's v6 patch and v4.14+
> kernel, which contains the xfs updates for 4.15-rc1.
>
>> +_require_xfs_io_command "falloc"
>
> fpunch is used as well, maybe need "_require_xfs_io_command fpunch".
>
> But I'd like to avoid falloc and fpunch, so test runs on filesystems
> that don't support falloc/fpunch but support fiemap, e.g. ext3, to get
> better test coverage.
>
>> +# ranged is a special argument which checks if fiemap supports
>> +# [offset [len]] args
>> +_require_xfs_io_command "fiemap" "ranged"
>> +
>> +_scratch_mkfs > $seqres.full 2>&1
>> +_scratch_mount || _fail "mount failure"
>> +
>> +file=$SCRATCH_MNT/testfile
>> +$XFS_IO_PROG -f -c "falloc 0 1m" $file
>> +for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*32))k 16k" $file; done
>
> So I'd like to create $file with the following command:
>
> # Create a file with 16k hole followed by 16k data, and this pattern
> # repeats till it reaches 1M file size, so each extent has 16k data.
> # But truncate file to its final size first, otherwise XFS would merge
> # some extents due to speculative preallocation.
> $XFS_IO_PROG -fc "truncate 1m" $file
> for i in {0..31}; do
> $XFS_IO_PROG -c "pwrite $(($i*32+16))k 16k" $file >/dev/null
> done
>
> Then update .out file, replace all "unwritten" with "data".
>
>
> And test is missing an entry in group file.
>
> Thanks,
> Eryu
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
