public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Lukas Czerner <lczerner@redhat.com>
Cc: esandeen@redhat.com, xfs@oss.sgi.com
Subject: Re: [PATCH] Add test 248: Check filesystem FITRIM implementation
Date: Mon, 20 Dec 2010 12:41:46 -0500	[thread overview]
Message-ID: <20101220174146.GA32680@infradead.org> (raw)
In-Reply-To: <1291996407-26251-1-git-send-email-lczerner@redhat.com>

Hi Lukas,

I've looked over it a bit again, and I can't really comment on the code
too much, as my bash fu is nowhere near a level to actually make sense
of most of the testcase.   I have however ran it in a few setups and
can comment based on that.

First your current first discard to check if we actually support it is
not handled correctly.  Running the test on a filesystem that doesn't
support it currently fails the test for me with:

@@ -1,3 +1,2 @@
QA output created by 249
-Checking FITRIM support: done.
-Running the test: done.
+Checking FITRIM support: fstrim: FSTRIM: Inappropriate ioctl for device

This should be easily fixable by redirecting the output of the test
command to /dev/null and do a _notrun if it exited with an error value,
just like other tests that require non-standard behaviour.

Second using data from /usr/share/doc seems a bit non-deterministic to
me, as the content will be different on every system.  Any chance you
could just use the xfstests source code instead?

Third the testcase runs forever, e.g. 93 minutes in my KVM setup, even
using a no-op discard implementation.  While this is useful for burn in
testing having a shorter run time version that can be run automatically
would be really useful.  I tried to figure out what paramters control
the runtime, but as mentioned above my bash skill really aren't enough
to do that.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2010-12-20 17:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-10 15:53 [PATCH] Add test 248: Check filesystem FITRIM implementation Lukas Czerner
2010-12-17  9:32 ` Lukas Czerner
2010-12-20 17:41 ` Christoph Hellwig [this message]
2010-12-21 10:41   ` Lukas Czerner
2010-12-21 11:03     ` Phil Karn
2010-12-21 11:22       ` Christoph Hellwig
2010-12-21 15:04 ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2010-11-26 14:12 Lukas Czerner
2010-12-06 10:20 ` Lukas Czerner
2010-12-09  5:15 ` Eric Sandeen
2010-12-09 11:02   ` Lukas Czerner
2010-12-09 11:11     ` Lukas Czerner

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=20101220174146.GA32680@infradead.org \
    --to=hch@infradead.org \
    --cc=esandeen@redhat.com \
    --cc=lczerner@redhat.com \
    --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