Re: [PATCH 08/12] xfs: remove xfs_ifork_ops

Brian Foster
2020-05-01T18:23:34Z

On Fri, May 01, 2020 at 06:50:17PM +0200, Christoph Hellwig wrote:
> On Fri, May 01, 2020 at 06:38:09PM +0200, Christoph Hellwig wrote:
> > That being said my approach here was a little too dumb. Once we are
> > all in the same code base we can stop the stupid patching of the
> > parent and just handle the case directly. Something like this
> > incremental diff on top of the sent out version (not actually tested).
> >
> > Total diffstate with the original patch is:
> >
> > 4 files changed, 37 insertions(+), 35 deletions(-)
> >
> > and this should also help with online repair while killing a horrible
> > kludge.
>
> Btw, І wonder if for repair / online repair just skipping the verifiers
> entirely would make more sense. But I think we can go there
> incrementally and just keep the existing repair behavior for now.
>

Can we use another dummy parent inode value in xfs_repair? It looks to
me that we set it to zero in phase 4 if it fails verification and set
the parent to NULLFSINO (i.e. unknown) in repair's in-core tracking.
Phase 6 walks the directory entries and explicitly sets the parent inode
number of entries with an unknown parent (according to the in-core
tracking). IOW, I don't see where we actually rely on the directory
header having a parent inode of zero outside of detecting it in the
custom verifier. If that's the only functional purpose, I wonder if we
could do something like set the bogus parent field of a sf dir to the
root inode or to itself, that way the default verifier wouldn't trip
over it..

Brian