On Fri, Nov 01, 2019 at 11:40:10AM -0700, Darrick J. Wong wrote:
> On Fri, Nov 01, 2019 at 01:17:11PM -0500, Eric Sandeen wrote:
> > On 10/22/19 1:46 PM, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > > If the kernel thinks a piece of metadata is bad, we must always report
> > > it. This will happen with an upcoming series to mark things sick
> > > whenever we return EFSCORRUPTED at runtime.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > I gotta say, I find this all really hard to read - something I should
> > have commented on earlier. Masks, maps, and functions oh my. bad and
> > sick and checked ... reported++ with no actual reporting ....
>
> Hm, yeah, the "reported" variable here counts the number of things the
> kernel reported to us as having been checked or marked sick at some point.
> The whole point of that is that if the kernel hasn't marked anything
> checked or sick then we really can't say much about the state of the
> filesystem and maybe the user should run xfs_scrub.
>
> > I'll try to think about what would make my poor brain happier later.
> > Comments, for one I think. Maybe some bikeshedding over variable names.
> >
> > I guess the upshot here is that if it's marked sick due to the kernel
> > sumbling over corruption, report it whether or not we ever explicitly
> > /asked/ for a check via the scrub interfaces?
>
> Correct.
Sigh. Change that to:
Correct, and soon I will send a kernel series that amends all of our
EFSCORRUPTED returns in libxfs to set the appropriate sick bit. Then
spaceman will be able to pull reports about past corruption that were
found in the course of normal operations.
--D
>
> --D
>
> > Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> >
> > > ---
> > > spaceman/health.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > >
> > > diff --git a/spaceman/health.c b/spaceman/health.c
> > > index 8fd985a2..0d3aa243 100644
> > > --- a/spaceman/health.c
> > > +++ b/spaceman/health.c
> > > @@ -171,10 +171,10 @@ report_sick(
> > > for (f = maps; f->mask != 0; f++) {
> > > if (f->has_fn && !f->has_fn(&file->xfd.fsgeom))
> > > continue;
> > > - if (!(checked & f->mask))
> > > + bad = sick & f->mask;
> > > + if (!bad && !(checked & f->mask))
> > > continue;
> > > reported++;
> > > - bad = sick & f->mask;
> > > if (!bad && quiet)
> > > continue;
> > > printf("%s %s: %s\n", descr, _(f->descr),
> > >
