linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Lukas Czerner <lczerner@redhat.com>
Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	xfs@oss.sgi.com
Subject: Re: [PATCH 1/6] generic/290: Add test for fallocate zero range
Date: Wed, 26 Feb 2014 07:01:17 +1100	[thread overview]
Message-ID: <20140225200117.GY13647@dastard> (raw)
In-Reply-To: <1393355728-12056-1-git-send-email-lczerner@redhat.com>

On Tue, Feb 25, 2014 at 08:15:23PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

You need a commit message that describes the test....

> ---
>  common/rc             |  14 +++++++
>  tests/generic/290     |  92 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/290.out | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/group   |   1 +
>  4 files changed, 216 insertions(+)
>  create mode 100755 tests/generic/290
>  create mode 100644 tests/generic/290.out
> 
> diff --git a/common/rc b/common/rc
> index f2c3c3a..d3ec4db 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1359,6 +1359,20 @@ _require_xfs_io_fiemap()
>  		_notrun "xfs_io fiemap command failed (no fs support?)"
>  }
>  
> +# check that xfs_io, kernel and filesystem all support fallocate with zero
> +# range
> +_require_xfs_io_falloc_zero()
> +{
> +	testfile=$TEST_DIR/$$.falloc
> +	testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> +		-c "fzero 4k 8k" $testfile 2>&1`
> +	rm -f $testfile 2>&1 > /dev/null
> +	echo $testio | grep -q "not found" && \
> +		_notrun "xfs_io fallocate zero range support is missing"
> +	echo $testio | grep -q "Operation not supported" && \
> +		_notrun "xfs_io fallocate fzero command failed (no fs support?)"
> +}
> +

Ok, we've now got 4 or 5 copies of this same set of tests for
different fallocate commands. Please factor.

>  # Check that a fs has enough free space (in 1024b blocks)
>  #
>  _require_fs_space()
> diff --git a/tests/generic/290 b/tests/generic/290
> new file mode 100755
> index 0000000..90f560a
> --- /dev/null
> +++ b/tests/generic/290
> @@ -0,0 +1,92 @@
> +#! /bin/bash
> +# FS QA Test No. 290
> +#
> +# Makes calls to fallocate zero range and checks tossed ranges
> +#
> +# Nothing should be tossed unless the range includes a page boundry
> +#
> +# Primarily tests page boundries and boundries that are
> +#  off-by-one to ensure we're only tossing what's expected
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2012 SGI.  All Rights Reserved.

Really?

Oh, it's a direct copy-n-paste of the XFS_IOC_ZERO_RANGE test with
s/zero/fzero/.

Please factor along the same lines as _generic_test_punch so we
don't have duplicated code in the tests.

Also, I haven't seen patches 3, 4 or 6 on the list, and they haven't
made it to the archive, either. Can you please resend them?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2014-02-25 20:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-25 19:15 [PATCH 1/6] generic/290: Add test for fallocate zero range Lukas Czerner
2014-02-25 19:15 ` [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress Lukas Czerner
2014-02-25 20:15   ` Dave Chinner
2014-02-25 19:15 ` [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set Lukas Czerner
2014-02-25 20:18   ` Dave Chinner
2014-02-26 13:04     ` Lukáš Czerner
2014-02-25 19:15 ` [PATCH 4/6] xfstests: Add fallocate zero range operation to fsx Lukas Czerner
2014-02-25 20:31   ` Dave Chinner
2014-02-25 19:15 ` [PATCH 5/6] xfstests: Define fallocate flags locally in fsx Lukas Czerner
2014-02-25 20:39   ` Dave Chinner
2014-02-25 21:56     ` Christoph Hellwig
2014-02-26  6:07       ` Dave Chinner
2014-02-25 19:15 ` [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range Lukas Czerner
2014-02-25 20:53   ` Dave Chinner
2014-02-25 21:01     ` Lukáš Czerner
2014-02-25 21:27       ` Lukáš Czerner
2014-02-25 21:50       ` Dave Chinner
2014-02-26 14:24         ` Lukáš Czerner
2014-02-26 22:01           ` Dave Chinner
2014-02-27 12:03             ` Lukáš Czerner
2014-02-27 19:35               ` Dave Chinner
2014-02-28 12:38                 ` Lukáš Czerner
2014-02-26 14:58         ` Lukáš Czerner
2014-02-26 22:17           ` Dave Chinner
2014-02-27 11:48             ` Lukáš Czerner
2014-02-25 20:01 ` Dave Chinner [this message]
2014-02-25 20:55   ` [PATCH 1/6] generic/290: Add " Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140225200117.GY13647@dastard \
    --to=david@fromorbit.com \
    --cc=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).