Re: [PATCH 1/1] xfs: do not check NEEDSREPAIR if ro,norecovery mount.

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

On Tue, Feb 04, 2025 at 11:55:00AM -0600, Eric Sandeen wrote:
> On 2/3/25 4:26 PM, Darrick J. Wong wrote:
> > On Mon, Feb 03, 2025 at 09:55:13AM +0100, Lukas Herbolt wrote:
> >> If there is corrutpion on the filesystem andxfs_repair
> >> fails to repair it. The last resort of getting the data
> >> is to use norecovery,ro mount. But if the NEEDSREPAIR is
> >> set the filesystem cannot be mounted. The flag must be
> >> cleared out manually using xfs_db, to get access to what
> >> left over of the corrupted fs.
> >>
> >> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> >> ---
> >>  fs/xfs/xfs_super.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> >> index 394fdf3bb535..c2566dcc4f88 100644
> >> --- a/fs/xfs/xfs_super.c
> >> +++ b/fs/xfs/xfs_super.c
> >> @@ -1635,8 +1635,12 @@ xfs_fs_fill_super(
> >>  #endif
> >>  	}
> >>  
> >> -	/* Filesystem claims it needs repair, so refuse the mount. */
> >> -	if (xfs_has_needsrepair(mp)) {
> >> +	/*
> >> +	 * Filesystem claims it needs repair, so refuse the mount unless
> >> +	 * norecovery is also specified, in which case the filesystem can
> >> +	 * be mounted with no risk of further damage.
> >> +	 */
> >> +	if (xfs_has_needsrepair(mp) && !xfs_has_norecovery(mp)) {
> > 
> > I think a better way to handle badly damaged filesystems is for us to
> > provide a means to extract directory trees in userspace, rather than
> > making the user take the risk of mounting a known-bad filesystem.
> > I've a draft of an xfs_db subcommand for doing exactly that and will
> > share for xfsprogs 6.14.
> 
> I think whether a userspace extractor is better or not depends on the
> usecase. I suppose there's some truth that a NEEDSREPAIR filesystem is
> "known bad" but we already suffer the risk of "unknown bad" filesystems
> today. (Or for that matter, the fact that we allow "norecovery" today,
> which also guarantees a mount of an inconsistent filesystem.)
> 
> "Something is wrong with this filesystem, let's mount it readonly and
> copy off the data" is a pretty time-tested approach, I think, hampered
> only by the fairly recent addition of NEEDSREPAIR.
> 
> a userspace scrape-the-device tool may well be useful for some, but
> I don't think that vs. this kernelspace option needs to be an either/or
> decision.

Fair enough; it's not like we have a tool today that can extract
directory trees from an unmountable filesystem.  Do you want to rvb this
one, then?

--D

> Thanks,
> 
> -Eric
> 
> > --D
> > 
> >>  		xfs_warn(mp, "Filesystem needs repair.  Please run xfs_repair.");
> >>  		error = -EFSCORRUPTED;
> >>  		goto out_free_sb;
> >> -- 
> >> 2.48.1
> >>
> >>
> > 
> 
>