[Top] [All Lists]

Re: [PATCH] Add test 248: Check filesystem FITRIM implementation

To: Lukas Czerner <lczerner@xxxxxxxxxx>
Subject: Re: [PATCH] Add test 248: Check filesystem FITRIM implementation
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 20 Dec 2010 12:41:46 -0500
Cc: xfs@xxxxxxxxxxx, esandeen@xxxxxxxxxx
In-reply-to: <1291996407-26251-1-git-send-email-lczerner@xxxxxxxxxx>
References: <1291996407-26251-1-git-send-email-lczerner@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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.

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