Re: [PATCH 09/13] xfs_repair: better checking of v5 attributes

[Brian Foster <bfoster@redhat.com>]

On Wed, Sep 09, 2015 at 02:34:07PM -0500, Eric Sandeen wrote:
> The commit:
> 
> 0519f66 xfs_repair: better checking of v5 metadata fields
> 
> added new corruption checks to dir2.c but missed the similar
> code in attr_repair.c; add that here.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  repair/attr_repair.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index 2aafdf6..c8ba484 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -201,6 +201,15 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  			goto error_out;
>  		}
>  
> +		/* corrupt node; rebuild the dir. */
> +		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
> +			libxfs_putbuf(bp);
> +			do_warn(
> +_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
> +				bno, da_cursor->ino);
> +			goto error_out;
> +		}
> +

Hmm, well this certainly looks similar, but is it the right thing to do
for xattrs? I haven't followed through how exactly directories are
rebuilt, but there does appear to be a recovery path in the dir2
context. A failure there simply puts the inode on a "bad" list to be
rebuilt later, presumably from data collected from all of the inodes.

If we fail here, it looks like we just clear the attribute fork. So are
we failing too hard, too soon here if a dablock crc happens to be
incorrect?

Brian

>  		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>  				  "count = %d, max = %d\n"),
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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