| Message ID | 20250203085513.79335-2-lukas@herbolt.com (mailing list archive) |
|---|---|
| State | Queued |
| Headers | show |
| Series | RFE: Do not check NEEDSREPAIR if ro,norecovery mount. | expand |
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. --D > xfs_warn(mp, "Filesystem needs repair. Please run xfs_repair."); > error = -EFSCORRUPTED; > goto out_free_sb; > -- > 2.48.1 > >
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. Thanks, -Eric > --D > >> xfs_warn(mp, "Filesystem needs repair. Please run xfs_repair."); >> error = -EFSCORRUPTED; >> goto out_free_sb; >> -- >> 2.48.1 >> >> >
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 > >> > >> > > > >
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)) { > xfs_warn(mp, "Filesystem needs repair. Please run xfs_repair."); > error = -EFSCORRUPTED; > goto out_free_sb; Look fine. I've had to hack around this with xfs_db when testing (broken) xfs_repair mods a couple of times myself... Reviewed-by: Dave Chinner <dchinner@redhat.com>
On 2/3/25 2:55 AM, 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)) { > xfs_warn(mp, "Filesystem needs repair. Please run xfs_repair."); > error = -EFSCORRUPTED; > goto out_free_sb; thanks Lukas, this looks good to me as well. And with ro,norecovery there should (tm) be no writes whatever to the filesystem so risk of introducing further corruption from the mount should be zero. Reviewed-by: Eric Sandeen <sandeen@redhat.com>
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)) { xfs_warn(mp, "Filesystem needs repair. Please run xfs_repair."); error = -EFSCORRUPTED; goto out_free_sb;
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(-)