[RESEND,6/7] xfstest: Add test case to test multiple collapse range call

From: Namjae Jeon <namjae.jeon@samsung.com>

We execute collapse range multiple times on same file.
Each collapse range call collapses a single alternate block.
After the test execution, file will be left with 80 blocks and
as much number of extents.
We also check for file system consistency after the completion.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
---
 tests/shared/317     |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/317.out |   85 ++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/group   |    1 +
 3 files changed, 182 insertions(+)
 create mode 100644 tests/shared/317
 create mode 100644 tests/shared/317.out

On Mon, Oct 07, 2013 at 05:14:06AM +0900, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon@samsung.com>
> 
> We execute collapse range multiple times on same file.
> Each collapse range call collapses a single alternate block.
> After the test execution, file will be left with 80 blocks and
> as much number of extents.
> We also check for file system consistency after the completion.
.....
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs xfs ext4
> +_supported_os Linux
> +
> +_require_scratch
> +_require_xfs_io_fiemap
> +_require_xfs_io_falloc_collapse
> +_do_die_on_error=y
> +test=$SCRATCH_MNT/test

Not used.

> +testfile=$SCRATCH_MNT/317.$$
> +BSIZE=4096
> +BLOCKS=10240
> +
> +# Filters fiemap output
> +_filter_fiemap()
> +{
> +	awk --posix '
> +		$3 ~ /hole/ {
> +			print $1, $2, $3;
> +			next;
> +		}
> +		$5 ~ /0x[[:xdigit:]]+/ {
> +			print $1, $2, "extent";
> +		}'
> +}

There's already a function in common/punch of this name, and it does
pretty much the same thing. Why not use that?

> +
> +case $FSTYP in
> +	ext4)
> +		export MKFS_OPTIONS="-F -b $BSIZE"
> +		;;
> +	xfs)
> +		export MKFS_OPTIONS="-f -b size=$BSIZE"
> +		;;
> +esac

_scratch_mkfs takes options on the command line - there is no need
to do this.

In fact, this test needs to run on all block sizes that filesystems
are capable of using, not just 4k and different architectures
exercise different code paths and so we must be able to test the
case where block size is smaller than page size on x86-64 so when
the code is run on an ia64 or ppc64 box with a 64k page size we know
that it's not completely broken...

Anyway, if you really need to make a 4k block size filesystem, then
_scratch_mkfs_sized() is the generic way of doing this.

> +# make filesystem on scratch with 4KB blocksize
> +_do 'make filesystem on $SCRATCH_DEV' '_scratch_mkfs'
> +_do 'mount filesytem' '_scratch_mount'

I really dislike this "_do" wrapper. The text does not add anything
to the test, and it makes it hard to see the command being run and
harder to modify it when necessary. It is used only by a couple of
old tests, and we'd do better to remove it than to propagate it
further.  This:

_scratch_mkfs >> $seqres.full 2>&1 || _fail "scratch_mkfs failed."
_scratch_mount >> $seqres.full 2>&1 || _fail "scratch_mount failed."

does everything that the _do wrapper does.

> +
> +# Write file
> +length=$(($BLOCKS*$BSIZE))
> +$XFS_IO_PROG -f -c "pwrite 0 $length" -c fsync $testfile > /dev/null
> +
> +# Collapse alternate blocks
> +for (( i = 1; i <= 7; i++ )); do
> +	for(( j=0 ; j < $(($BLOCKS/(2**$i))) ; j++ )); do
> +		offset=$(($j*$BSIZE))
> +		$XFS_IO_PROG -c "fcollapse $offset $BSIZE" $testfile > /dev/null
> +	done
> +done
> +
> +# Check if 80 extents are present
> +$XFS_IO_PROG -c "fiemap -v" $testfile | _filter_fiemap

If all you care about is that there are 80 extents, then why not
just something like:

$XFS_IO_PROG -c "fiemap -v" $testfile |grep "^ *[0-9]*:" |wc -l

> +
> +_do 'unmount $SCRATCH_DEV' 'umount $SCRATCH_DEV'
> +_do 'repair filesystem' '_check_scratch_fs'

_check_scratch_fs is all you need to call here.

> index 3a69294..80ff7ec 100644
> --- a/tests/shared/group
> +++ b/tests/shared/group
> @@ -12,3 +12,4 @@
>  298 auto trim
>  305 aio dangerous enospc rw stress
>  316 auto quick collapse
> +317 auto collapse

Again, I think the prealloc group is better for this.

Cheers,

Dave.

>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_supported_fs xfs ext4
>> +_supported_os Linux
>> +
>> +_require_scratch
>> +_require_xfs_io_fiemap
>> +_require_xfs_io_falloc_collapse
>> +_do_die_on_error=y
>> +test=$SCRATCH_MNT/test
>
> Not used.
Okay.

>
>> +testfile=$SCRATCH_MNT/317.$$
>> +BSIZE=4096
>> +BLOCKS=10240
>> +
>> +# Filters fiemap output
>> +_filter_fiemap()
>> +{
>> +     awk --posix '
>> +             $3 ~ /hole/ {
>> +                     print $1, $2, $3;
>> +                     next;
>> +             }
>> +             $5 ~ /0x[[:xdigit:]]+/ {
>> +                     print $1, $2, "extent";
>> +             }'
>> +}
>
> There's already a function in common/punch of this name, and it does
> pretty much the same thing. Why not use that?
Ah, Okay, I will check.

>
>> +
>> +case $FSTYP in
>> +     ext4)
>> +             export MKFS_OPTIONS="-F -b $BSIZE"
>> +             ;;
>> +     xfs)
>> +             export MKFS_OPTIONS="-f -b size=$BSIZE"
>> +             ;;
>> +esac
>
> _scratch_mkfs takes options on the command line - there is no need
> to do this.
Okay.

>
> In fact, this test needs to run on all block sizes that filesystems
> are capable of using, not just 4k and different architectures
> exercise different code paths and so we must be able to test the
> case where block size is smaller than page size on x86-64 so when
> the code is run on an ia64 or ppc64 box with a 64k page size we know
> that it's not completely broken...
Okay, I will update to test block size is smaller than page size.

>
> Anyway, if you really need to make a 4k block size filesystem, then
> _scratch_mkfs_sized() is the generic way of doing this.
>
>> +# make filesystem on scratch with 4KB blocksize
>> +_do 'make filesystem on $SCRATCH_DEV' '_scratch_mkfs'
>> +_do 'mount filesytem' '_scratch_mount'
>
> I really dislike this "_do" wrapper. The text does not add anything
> to the test, and it makes it hard to see the command being run and
> harder to modify it when necessary. It is used only by a couple of
> old tests, and we'd do better to remove it than to propagate it
> further.  This:
>
> _scratch_mkfs >> $seqres.full 2>&1 || _fail "scratch_mkfs failed."
> _scratch_mount >> $seqres.full 2>&1 || _fail "scratch_mount failed."
>
> does everything that the _do wrapper does.
Okay.

>
>> +
>> +# Write file
>> +length=$(($BLOCKS*$BSIZE))
>> +$XFS_IO_PROG -f -c "pwrite 0 $length" -c fsync $testfile > /dev/null
>> +
>> +# Collapse alternate blocks
>> +for (( i = 1; i <= 7; i++ )); do
>> +     for(( j=0 ; j < $(($BLOCKS/(2**$i))) ; j++ )); do
>> +             offset=$(($j*$BSIZE))
>> +             $XFS_IO_PROG -c "fcollapse $offset $BSIZE" $testfile > /dev/null
>> +     done
>> +done
>> +
>> +# Check if 80 extents are present
>> +$XFS_IO_PROG -c "fiemap -v" $testfile | _filter_fiemap
>
> If all you care about is that there are 80 extents, then why not
> just something like:
>
> $XFS_IO_PROG -c "fiemap -v" $testfile |grep "^ *[0-9]*:" |wc -l
Okay, I will check.

>
>> +
>> +_do 'unmount $SCRATCH_DEV' 'umount $SCRATCH_DEV'
>> +_do 'repair filesystem' '_check_scratch_fs'
>
> _check_scratch_fs is all you need to call here.
Yes, right. I will update.

>
>> index 3a69294..80ff7ec 100644
>> --- a/tests/shared/group
>> +++ b/tests/shared/group
>> @@ -12,3 +12,4 @@
>>  298 auto trim
>>  305 aio dangerous enospc rw stress
>>  316 auto quick collapse
>> +317 auto collapse
>
> Again, I think the prealloc group is better for this.
Okay, I will add collpase range cases to prealloc group.

Thanks for review.
I will post patches included your all review points soon.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html