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@xxxxxxxxxx>
> >
> > 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
>
