Re: [PATCH 9/9] repair: detect and handle attribute tree CRC errors

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 29, 2014 at 10:06:51AM -0400, Brian Foster wrote:
> On Tue, Apr 29, 2014 at 07:04:59AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Currently the attribute code will not detect and correct errors in
> > the attribute tree. It also fails to validate the CRCs and headers
> > on remote attribute blocks. Ensure that all the attribute blocks are
> > CRC checked and that the processing functions understand the correct
> > block formats for decoding.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  repair/attr_repair.c | 45 +++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 37 insertions(+), 8 deletions(-)
> > 
> > diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> > index ba85ac2..9b57960 100644
> > --- a/repair/attr_repair.c
> > +++ b/repair/attr_repair.c
> > @@ -604,6 +604,7 @@ verify_da_path(xfs_mount_t	*mp,
> >  			libxfs_putbuf(bp);
> >  			return(1);
> >  		}
> > +
> >  		/*
> >  		 * update cursor, write out the *current* level if
> >  		 * required.  don't write out the descendant level
> > @@ -615,6 +616,8 @@ verify_da_path(xfs_mount_t	*mp,
> >  			libxfs_writebuf(cursor->level[this_level].bp, 0);
> >  		else
> >  			libxfs_putbuf(cursor->level[this_level].bp);
> > +
> > +		/* switch cursor to point at the new buffer we just read */
> >  		cursor->level[this_level].bp = bp;
> >  		cursor->level[this_level].dirty = 0;
> >  		cursor->level[this_level].bno = dabno;
> > @@ -624,6 +627,14 @@ verify_da_path(xfs_mount_t	*mp,
> >  		cursor->level[this_level].n = newnode;
> >  #endif
> >  		entry = cursor->level[this_level].index = 0;
> > +
> > +		/*
> > +		 * We want to rewrite the buffer a CRC error seeing as it
> Nit:						^ "on a CRC error ..." ?

Will fix.

> > @@ -1555,7 +1584,7 @@ process_longform_attr(
> >  	case XFS_DA_NODE_MAGIC:		/* btree-form attribute */
> >  	case XFS_DA3_NODE_MAGIC:
> >  		/* must do this now, to release block 0 before the traversal */
> > -		if (repairlinks) {
> > +		if (*repair || repairlinks) {
> >  			*repair = 1;
> >  			libxfs_writebuf(bp, 0);
> >  		} else
> 
> repairlinks incorporates a !no_modify check, but *repair does not. It's
> incremented unconditionally if we find a CRC error. I suspect this means
> we now need a !no_modify check for the writebuf/putbuf check here, as is
> done in the alternate path at the end of the function.

I'll fix it. Christoph is right, we need a helper function for this.
Thanks!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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