Re: [PATCH 2/6] xfs_repair: clear DIFLAG2_NREXT64 when filesystem doesn't support nrext64

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jun 29, 2022 at 08:58:57AM +1000, Dave Chinner wrote:
> On Tue, Jun 28, 2022 at 01:49:25PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Clear the nrext64 inode flag if the filesystem doesn't have the nrext64
> > feature enabled in the superblock.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  repair/dinode.c |   19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > 
> > diff --git a/repair/dinode.c b/repair/dinode.c
> > index 00de31fb..547c5833 100644
> > --- a/repair/dinode.c
> > +++ b/repair/dinode.c
> > @@ -2690,6 +2690,25 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"),
> >  			}
> >  		}
> >  
> > +		if (xfs_dinode_has_large_extent_counts(dino) &&
> > +		    !xfs_has_large_extent_counts(mp)) {
> > +			if (!uncertain) {
> > +				do_warn(
> > +	_("inode %" PRIu64 " is marked large extent counts but file system does not support large extent counts\n"),
> > +					lino);
> > +			}
> > +			flags2 &= ~XFS_DIFLAG2_NREXT64;
> > +
> > +			if (no_modify) {
> > +				do_warn(_("would zero extent counts.\n"));
> > +			} else {
> > +				do_warn(_("zeroing extent counts.\n"));
> > +				dino->di_nextents = 0;
> > +				dino->di_anextents = 0;
> > +				*dirty = 1;
> 
> Is that necessary? If the existing extent counts are within the
> bounds of the old extent fields, then shouldn't we just rewrite the
> current values into the old format rather than trashing all the
> data/xattrs on the inode?

It's hard to know what to do here -- we haven't actually checked the
forks yet, so we don't know if the dinode flag was set but the !nrext64
extent count fields are ok so all we have to do is clear the dinode
flag; or if the dinode flag was set, the nrext64 extent count fields are
correct and must be moved to the !nrext64 fields; or what?

I guess I could just leave the extent count fields as-is and let the
process_*_fork functions deal with it.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com