On Wed, Jan 13, 2021 at 01:17:49PM -0500, Brian Foster wrote:
> On Fri, Jan 08, 2021 at 10:28:16PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Clear the needsrepair flag, since it's used to prevent mounting of an
> > inconsistent filesystem.
> >
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
> > repair/agheader.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> >
> > diff --git a/repair/agheader.c b/repair/agheader.c
> > index 8bb99489..f6174dbf 100644
> > --- a/repair/agheader.c
> > +++ b/repair/agheader.c
> > @@ -452,6 +452,17 @@ secondary_sb_whack(
> > rval |= XR_AG_SB_SEC;
> > }
> >
> > + if (xfs_sb_version_needsrepair(sb)) {
> > + if (!no_modify)
> > + sb->sb_features_incompat &=
> > + ~XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR;
> > + if (!do_bzero) {
> > + rval |= XR_AG_SB;
> > + do_warn(_("needsrepair flag set in sb %d\n"), i);
> > + } else
> > + rval |= XR_AG_SB_SEC;
> > + }
> > +
>
> Looks reasonable modulo the questions on the previous patch. When I give
> this a test, one thing that stands out is that the needsrepair state
> itself sort of presents as corruption. I.e.,
>
> # ./db/xfs_db -x -c "version needsrepair" <dev>
> Upgrading V5 filesystem
> Upgraded V5 filesystem. Please run xfs_repair.
> versionnum [0xb4a5+0x18a] =
> V5,NLINK,DIRV2,ALIGN,LOGV2,EXTFLG,MOREBITS,ATTR2,LAZYSBCOUNT,PROJID32BIT,CRC,FTYPE,FINOBT,SPARSE_INODES,REFLINK,NEEDSREPAIR
> # ./repair/xfs_repair <dev>
> Phase 1 - find and verify superblock...
> Phase 2 - using internal log
> - zero log...
> - scan filesystem freespace and inode maps...
> needsrepair flag set in sb 1
> reset bad sb for ag 1
> needsrepair flag set in sb 2
> reset bad sb for ag 2
> needsrepair flag set in sb 0
> reset bad sb for ag 0
> needsrepair flag set in sb 3
> reset bad sb for ag 3
> - found root inode chunk
> Phase 3 - for each AG...
> ...
>
> So nothing was ever done to this fs besides set and clear the bit. Not a
> huge deal, but I wonder if we should print something more user friendly
> to indicate that repair found and cleared the needsrepair state, or at
> least just avoid the "reset bad sb ..." message for the needsrepair
> case.
Hm. For the backup supers I guess there's really not much point in
saying anything about the bit being set, because the only time they get
used is when repair tries to use one to fix a filesystem.
As for AG 0, I guess I could change that to say:
dbprintf(_("Thank you for running xfs_repair!"));
:D Or maybe there's no need to say anything at all.
--D
> Brian
>
> > return(rval);
> > }
> >
> >
>
