Re: [PATCH] xfs: fix delalloc quota accounting on failure

[Dave Chinner <david@fromorbit.com>]

On Tue, May 08, 2012 at 09:28:55AM -0500, Eric Sandeen wrote:
> On 5/8/12 5:48 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > xfstest 270 was causing quota reservations way beyond what was sane
> > (ten to hundreds of TB) for a 4GB filesystem. There's a sign problem
> > in the error handling path of xfs_bmapi_reserve_delalloc() because
> > xfs_trans_unreserve_quota_nblks() simple negates the value passed -
> > which doesn't work for an unsigned variable. This causes
> > reservations of close to 2^32 block instead of removing a
> > reservation of a handful of blocks.
> > 
> > Fix the same problem in the other xfs_trans_unreserve_quota_nblks()
> > callers where unsigned integer variables are used, too.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> 
> Ouch!
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> as far as it goes, but a couple thoughts:
> 
> 1) Should the cast be done in the macro so new callers don't get tripped up?
> 2) Should we just remove the ninos argument from the macro?  It's always passed as 0 (and could potentially suffer the same problem)
> 
> something like:

3) I'm going to remove the problematic macro altogether.

I just wanted to get this fix out there while I look at the other
quota problems I've found. Hint: xfstests _check_quota_usage()
doesn't actually work *at all* on XFS (always passes without
actually checking quotas), and so it has been hiding problems with
either the tests that use it or the quota code for some time....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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