xfs
[Top] [All Lists]

Re: [PATCH][RESEND] xfstests: only run generic/300 on filesystems suppo

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH][RESEND] xfstests: only run generic/300 on filesystems supporting
From: Eric Whitney <enwlinux@xxxxxxxxx>
Date: Sat, 21 Sep 2013 17:59:27 -0400
Cc: Eric Whitney <enwlinux@xxxxxxxxx>, xfs@xxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=/gO4TdCZ5o1+9L3o+gcD/bsjJ1+kygJ+99Ym4QTbIfQ=; b=CX4UM2hshX2/cSi3oKtUoSICmw2ruJt56kv/Sp1GQGa0P7M39Iay42LRYvLvhx1n25 qAfs1j+zhe9BQc0qRQz4KH7tJ60GbAIMuGYa3FvIbbsv8AagKD0Xcf8SQwIdm2ezI21H ffnLnYHucrKn1ZNWV0aWwhqOBb9uE8x1Hpd7ZbmmqaYOdTCIXsBe/hkEOz3qEIfVVQjs 17NqxZ2JIw8RdIbxKf7wc8aYImPKuz+PlfT/U37kbqSwrb/dmO0AiNI1/6Lq28F4Isvi +bsA+l1Y09LaEbbhmAyKtNsUgsYw8TeNk92rPFAfid/08jDyO3wMjjGwy/Y1ziPH5btD GVRA==
In-reply-to: <522A3254.3090704@xxxxxxxxxx>
References: <20130906194243.GA19286@wallace> <522A3254.3090704@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
* Eric Sandeen <sandeen@xxxxxxxxxx>:
> On 9/6/13 2:42 PM, Eric Whitney wrote:
> > Generic/300 fails when run on a test filesystem that does not support
> > fallocate(), as in the case of an ext4 filesystem created without the
> > extent feature.  It uses fio's falloc ioengine to generate part of its
> > I/O load.
> > 
> > Verify that the test filesystem supports fallocate() before proceeding
> > with the test.  Also, delete any pre-existing test output to avoid
> > confusion with old results.
> 
> Hey Eric - sorry this got missed for review.
> 
> Since the test doesn't actually use xfs_io it seems like slightly the
> wrong check, maybe we need a new _require_fio_falloc()?
> 
> But xfs_io will almost always be installed for someone running
> xfstests, and the check as you have it will indeed test that the fs
> can do fallocate; it's just not the most targeted test.
> 
> It's probably ok, though - a comment about why you _require_xfs_io
> when xfs_io isn't used might be good.
> 
> What do you think?
> 

Hi Eric:

Sorry for my late reply.

I agree that the scope of _require_xfs_io_falloc is larger than what we
really need here.  However, after looking for other possible users of a
_require_fio_falloc and not finding others, I think adding the comment you
suggest sounds like the way to go for now.  I'm willing to add a function
tailored to fio (implemented and tested same), but that looks like it would
add some fairly redundant code that would only be used by one caller.  And,
you're quite right that you really need xfsprogs in place to use xfstest 
anyway - there are more than 20 tests that require xfs_io to run generic
xfstests on ext4.

More work revealed that I also need to add a call to
_require_xfs_io_falloc_punch.  Ext4 filesystems created with the bigalloc
feature don't support hole punching but do support allocation with fallocate().

I've got a tested V2 patch I'll post shortly.

Also, I discovered that generic/299 needs some similar treatment.  It's
using the fallocate command to generate a background load, but is failing
silently while reporting success when run against an ext4 filesystem created
without the extent feature (ext3 emulation).  This tends to point out the
problem with implementing a _require_<command>_falloc function for each
new command that test scripts want to use to generate I/O - they could
multiply quickly.

At any rate, I'm planning to get out a patch for generic/299 as well.

Thanks,
Eric

> 
> > Signed-off-by: Eric Whitney <enwlinux@xxxxxxxxx>
> > ---
> >  tests/generic/300 |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tests/generic/300 b/tests/generic/300
> > index 7c60728..1ac763b 100755
> > --- a/tests/generic/300
> > +++ b/tests/generic/300
> > @@ -43,6 +43,9 @@ _supported_fs generic
> >  _supported_os Linux
> >  _need_to_be_root
> >  _require_scratch
> > +_require_xfs_io_falloc
> > +
> > +rm -f $seqres.full
> >  
> >  NUM_JOBS=$((4*LOAD_FACTOR))
> >  BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> > 
> 

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