[Top] [All Lists]

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

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH 07/16] xfs: add CRC checks for quota blocks
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 6 Mar 2013 09:49:36 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130305203615.GK22182@xxxxxxx>
References: <1361755901-12453-1-git-send-email-david@xxxxxxxxxxxxx> <1361755901-12453-8-git-send-email-david@xxxxxxxxxxxxx> <20130305203615.GK22182@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxx>
> > 
> > Use the reserved space in struct xfs_dqblk to store a UUID and a crc
> > for the quota blocks.
> > 
> > [dchinner@xxxxxxxxxx] Add a LSN field and update for current verifier
> > infrastructure.
> > 
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> 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.


Dave Chinner

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