xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH, RFC] xfs_repair - clear inodes in incorrect btree format
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 15 Jul 2009 09:07:05 -0400
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <4A582070.9040907@xxxxxxxxxxx>
References: <4A582070.9040907@xxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
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;
        }

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