xfs
[Top] [All Lists]

Re: [patch 2/2] xfstests: xfs_tosspages() test addition

To: Andrew Dahl <adahl@xxxxxxx>
Subject: Re: [patch 2/2] xfstests: xfs_tosspages() test addition
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 9 Nov 2012 10:27:26 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20121108222315.709683665@xxxxxxx>
References: <20121108222315.505370321@xxxxxxx> <20121108222315.709683665@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Nov 08, 2012 at 04:23:17PM -0600, Andrew Dahl wrote:
> Tests xfs_tosspages() to ensure the expected output of tossing various 
> ranges. It uses the XFS_IOC_ZERO_RANGE ioctl by way of "xfs_io zero"
> 
> The ranges tested are [0,4095], [0,4096], [4096,8192], [1024,5120]

I'd add [0,1] [0,4097], [4095,8191] [4095,8192] and [4095,8193] as
well for good measure, as they exercise other off-by-one corner
cases not tested here.

> Signed-off-by: Andrew Dahl <adahl@xxxxxxx>
> 
> --- 
> 
> Index: xfstests/290
> ===================================================================
> --- /dev/null
> +++ xfstests/290
> @@ -0,0 +1,86 @@
> +#! /bin/bash
> +# FS QA Test No. 290
> +#
> +# check xfs_tosspages to ensure it's tossing proper pages

A better description about the corner cases it is test woul dbe
helpful.

> +_cleanup()
> +{
> +    rm -f $TEST_DIR/290.*
> +}

Leave them there - the test dir is supposed to grow and age over
time. Also, removing them here means on a failure the files re
removed and there is no corpse left to dissect. Besides, you are
already using O_TRUNC for opening the files, so if they already
exist the test handles it just fine.

> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_supported_os Linux
> +
> +_test_io_zero()
> +{
> +        $XFS_IO_PROG -c "zero help" 2>&1 | \
> +                        grep 'command "zero" not found' > /dev/null
> +        echo $?
> +}
> +
> +[ $(_test_io_zero) -eq 0 ] && _notrun "zero command not supported"

You need to test whether the filesystem supports it. See, for
example, _require_xfs_io_falloc. Also, moving it to common.rc and
naming it _require_xfs_io_zero woul dbe a good idea.

Yes, I know you copied this from test 242, and that was probably my
fault in the first place, but rather than propagate something that
is wrong, let's fix it properly before it becomes widespread....


> +testfile=$TEST_DIR/290.$$
> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 0 4095" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique

Nice! Someone's been listening to me about filtering. :)

> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 0 4096" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique
> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 4096 4096" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique
> +
> +$XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
> +                      -c "pwrite -S 0x42 4096 4096" \
> +                      -c "zero 1024 4096" \
> +                      -c "pread -v 0 8192" \
> +                      $testfile | _filter_xfs_io_unique

A little wrapper function might be in order here - it makes it much
easier to see what is being tested. i.e.:

test_zero()
{
        zero_start=$1
        zero_len=$2

        $XFS_IO_PROG -F -f -t -c "pwrite -S 0x41 0 4096" \
                              -c "pwrite -S 0x42 4096 4096" \
                              -c "zero $zero_start $zero_end" \
                              -c "pread -v 0 8192" \
                              $testfile | _filter_xfs_io_unique
}

#         Zero Range
test_zero    0 4095
test_zero    0 4096
test_zero 4096 4096
test_zero 1024 4096

That also makes it much easier to add more ranges to be tested. :)

> Index: xfstests/group
> ===================================================================
> --- xfstests.orig/group
> +++ xfstests/group
> @@ -408,3 +408,4 @@ deprecated
>  287 auto dump quota quick
>  288 auto quick ioctl trim
>  289 auto quick
> +290 auto quick ioctl

prealloc and rw groups make sense, too.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>