Re: [PATCH 2/3] xfs_repair: improve checking of existing rmap and refcount btrees

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

On Thu, May 12, 2022 at 02:39:13PM -0500, Eric Sandeen wrote:
> On 5/5/22 11:05 AM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > There are a few deficiencies in the xfs_repair functions that check the
> > existing reverse mapping and reference count btrees.  First of all, we
> > don't report corruption or IO errors if we can't read the ondisk
> > metadata.  Second, we don't consistently warn if we cannot allocate
> > memory to perform the check.
> 
> Well, the caller used to report those, right? i.e.
> 
>         error = check_refcounts(wq->wq_ctx, agno);
>         if (error)
>                 do_error(
> _("%s while checking reference counts"),
>                          strerror(-error));
> 
> So I think this patch is just changing from reporting strerror(-error)
> when these things return, to hand-crafted error messages in the functions
> that get called?
> 
> AFAICT this patch changes
> 
> "Cannot allocate memory while checking reverse-mappings"
> to
> "Not enough memory to check reverse mappings"
> 
> etc.
> 
> Granted, strerror(EFSCORRUPTED) isn't very pretty, and your messages are
> probably more clear. But I just wanted to be sure I'm not missing something;
> I think every error is actually reported today, and this is just changing
> the error messages.

Yep, that's correct.  I guess I should have (re)written the commit
message like this:

"When we're checking the rmap and refcount btrees, push error reporting
down to the exact locations of failures so that the user sees both a
message specific to the failure that occurred and what repair was doing
at the time.  This also removes quite a bit of return code handling,
since all the errors were fatal anyway."

--D

> Thanks,
> -Eric
>