[PATCH 07/16] xfs: add CRC checks for quota blocks

[Dave Chinner]

On Tue, Mar 05, 2013 at 02:36:15PM -0600, Ben Myers wrote:
> Hi Dave,
> 
> On Mon, Feb 25, 2013 at 12:31:32PM +1100, Dave Chinner wrote:
> > From: Christoph Hellwig <hch at lst.de>
> > 
> > Use the reserved space in struct xfs_dqblk to store a UUID and a crc
> > for the quota blocks.
> > 
> > [dchinner at redhat.com] Add a LSN field and update for current verifier
> > infrastructure.
> > 
> > Signed-off-by: Christoph Hellwig <hch at lst.de>
> > Signed-off-by: Dave Chinner <dchinner at redhat.com>
> 
> Been over this and it looks fine.
> 
> > @@ -897,6 +910,10 @@ xfs_qm_dqiter_bufs(
> >  		if (error)
> >  			break;
> >  
> > +		/*
> > +		 * XXX(hch): need to figure out if it makes sense to validate
> > +		 *	     the CRC here.
> > +		 */
> 
> What's you're opinion on this?

I did actually change the code to validate the CRC here via
the call to xfs_trans_read_buf(.... &xfs_dquot_buf_ops) above. i.e.
the read verifier will do CRC validation of the buffer.

The comment is still relevant, however, because it's not exactly
clear what the best approach to do here if we get a CRC failure.
That would indicate some kind of corruption that we weren't
expecting, and quite possibly a corruption that rewriting the dquot
can't fix. So without knowing what kind of corruption is typical
here, I don't know what the best approach to take here is.

So effectively what I've done is add CRC validation so that we'll
get people telling us about problems (because the quotacheck will
abort and there will be a stack trace in the log). If it turns out
that corrupted quota files is a common problem that quotacheck
encounters we can gather the corpses, do post-mortem analysis of the
failures and then revisit the code appropriately with that knowledge
in hand.

Cheers,

Dave.
-- 
Dave Chinner
david at fromorbit.com