On Tue, Jan 19, 2021 at 09:37:54AM -0500, Brian Foster wrote:
> On Fri, Jan 15, 2021 at 05:24:53PM -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>
> > ---
>
> Code/errors look much cleaner. Though looking at the repair code again,
> I wonder... if we clear the needsrepair bit and dirty/write the sb in
> phase 2 and then xfs_repair happens to crash, do we risk clearing the
> bit and thus allowing a potential mount before whatever requisite
> metadata updates have been made?
[Oh good, now mail.kernel.org is having problems...]
Yes, though I think that falls into the realm of "sysadmins should be
sufficiently self-aware not to expect mount to work after repair
fails/system crashes during an upgrade".
I've thought about how to solve the general problem of preventing people
from mounting filesystems if repair doesn't run to completion. I think
xfs_repair could be modified so that once it finds the primary super, it
writes it back out with NEEDSREPAIR set (V5) or inprogress set (V4).
Once we've finished the buffer cache flush at the end of repair, we
clear needsrepair/inprogress and write the primary super again.
An optimization on that would be to find a way to avoid that first super
write until we flush the first dirty buffer.
Another way to make repair more "transactional" would be to do it would
be to fiddle with the buffer manager so that writes are sent to a
metadump file which could be mdrestore'd if repair completes
successfully. But that's a short-circuit around the even bigger project
of porting the kernel logging code to userspace and use that in repair.
--D
> Brian
>
> > repair/agheader.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> >
> > diff --git a/repair/agheader.c b/repair/agheader.c
> > index 8bb99489..d9b72d3a 100644
> > --- a/repair/agheader.c
> > +++ b/repair/agheader.c
> > @@ -452,6 +452,21 @@ 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 (i == 0) {
> > + if (!no_modify)
> > + do_warn(
> > + _("clearing needsrepair flag and regenerating metadata\n"));
> > + else
> > + do_warn(
> > + _("would clear needsrepair flag and regenerate metadata\n"));
> > + }
> > + rval |= XR_AG_SB_SEC;
> > + }
> > +
> > return(rval);
> > }
> >
> >
>
