Re: [PATCH, RFC] xfs_repair - clear inodes in incorrect btree format

[Christoph Hellwig <hch@infradead.org>]

Adding this check is certainly better than having nothing, but I would
be much happier if we could do something.

On Sat, Jul 11, 2009 at 12:17:36AM -0500, Eric Sandeen wrote:
> 1) How'd it get into this state? ... but maybe more importantly...

End of last year lachlan had case that looked a bit like this where
we had problems resetting the fork state.

> 2) Should these really get cleared?  It's possibly a sane extent list,
> it's just that it -could- be in extents rather than btree format...

That is indeed the the most likely case.  Do you still have a metadump
with this problem around?  We should probably sanity-check for a valid
looking extent format inode and then process it as such.

> 3) By the same token, should the kernel really be choking on it?

Well, not choking could cause all kinds of harm by treating it as
a btree inode while it's not.  We could try to apply a very careful
variant of 2) above, but I'd really rather leave that kind of thing
to repair.

> +	if (*nex <= XFS_DFORK_SIZE(dip, mp, whichfork) / sizeof(xfs_bmbt_rec_t)) {
> +		do_warn(_("extent count for ino %lld %s fork too low "
> +			  "(%d) for file format\n"),
> +				lino,
> +				whichfork == XFS_DATA_FORK ? _("data") : _("attr"),
> +				*nex);
> +		return(1);
> +	}

Well, you'll get my ok in the sense of this looks good and better than
nothing, but I'd really prefer a real fixup for this issues.  Also the
code above looks a bit unreadable, why not:


	if (*nex <= XFS_DFORK_SIZE(dip, mp, whichfork) /
			sizeof(xfs_bmbt_rec_t)) {
		do_warn(
	_("extent count for ino %lld %s fork too low (%d) for file format\n"),
			lino, forkname, *nex);
		return 1;
	}

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