On Tue, Oct 15, 2019 at 12:07:11PM -0500, Eric Sandeen wrote:
> On 9/25/19 4:34 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Fix some bogosity with how we handle runtime errors in the read verify
> > pool functions. First of all, memory allocation failures shouldn't be
> > recorded as disk IO errors, they should just complain and abort the
> > phase. Second, we need to collect any other runtime errors in the IO
> > thread and abort the phase instead of silently ignoring them.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > scrub/read_verify.c | 23 +++++++++++++++++++----
> > 1 file changed, 19 insertions(+), 4 deletions(-)
> >
> >
> > diff --git a/scrub/read_verify.c b/scrub/read_verify.c
> > index b890c92f..00627307 100644
> > --- a/scrub/read_verify.c
> > +++ b/scrub/read_verify.c
> > @@ -53,6 +53,7 @@ struct read_verify_pool {
> > struct disk *disk; /* which disk? */
> > read_verify_ioerr_fn_t ioerr_fn; /* io error callback */
> > size_t miniosz; /* minimum io size, bytes */
> > + int errors_seen;
> > };
>
> I'd like to see a comment that says /* runtime/operational errors */
> to differentiate between verification errors.
>
> Or maybe even rename it to runtime_errors or something.
>
> I'm also confused; it's an int, but this patch assigns it with true/false,
> the next patch assigns errnos i.e. ECANCELED, .... what's it supposed to be?
It's a mess. :(
It ought to be an int since that's where we're going anyway.
Will add a comment, change the name, and make the usage consistent
throughout the series.
--D
