Re: [PATCH] Add test 257: Check proper FITRIM argument handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 26 Sep 2011, Alex Elder wrote:

> On Mon, 2011-09-26 at 09:14 +0200, Lukas Czerner wrote:
> > This test suppose to validate that file systems are using the fitrim
> > arguments right. It checks that the fstrim returns EINVAl in case that
> > the start of the range is beyond the end of the file system, and also
> > that the fstrim works without an error if the length of the range is
> > bigger than the file system (it should be truncated to the file system
> > length automatically within the fitrim implementation).
> > 
> > This test should also catch common problem with overflow of start+len.
> > Some file systems (ext4,xfs) had overflow problems in the past so there
> > is a specific test for it (for ext4 and xfs) as well as generic test for
> > other file systems, but it would be nice if other fs can add their
> > specific checks if this problem does apply to them as well.
> > 
> > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
> 
> OK, I know I suggested it, and I do like the result, but
> there could be a problem with the use of things like
> "2^32 - 1" being passed to the _math() function.
> 
> The problem lies in the way _math() backs off to try
> to use shell built-in arithmetic, which interprets the
> '^' as a bitwise XOR operator.  (Note, _math() was
> defined in an earlier patch.)
> 
> I think the use of "bc" to do certain math operations
> has some value, and as such I think the right fix is
> just to require "bc" in order for xfstests, or at least
> for any that use the _math() function, and drop the
> fall-back logic out of the definition of _math().
> 
> What do you think?

Yes I had the same concern, but I guess I was just lazy to look at it
:). So if we can require "bc" for xfstests we can simply remove the
fallback. Also maybe adding helper _require_bc, or maybe even better
adding helper _require <whatever> so we can check for <whatever> tool
in any test.

Thanks!
-Lukas


> 
> Assuming we resolve that, this test now looks fine to me.
> 
> Reviewed-by: Alex Elder <aelder@xxxxxxx>
> 
> 
> 

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


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux