Re: [PATCH] dm-verity: Add error handling modes for corrupted blocks

[Sami Tolvanen <samitolvanen@google.com>]

On Mon, Mar 16, 2015 at 05:13:10PM -0500, Will Drewry wrote:
> > +       if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS)
> > +               goto out;
> > +
> > +       ++v->corrupted_errs;
> > +
> 
> The conditional and increment should be moved below the DMERR_LIMIT().
> Otherwise, no logging will occur in non-logging modes.

This only limits the maximum number of logged errors, but until it's reached,
it does log them in all modes.

> This would be a change from how the default "eio" mode behaves today.

The only difference is that it will stop logging after reaching the maximum.

> > +       DMERR_LIMIT("%s: %s block %llu is corrupted", v->data_dev->name,
> > +               type_str, block);
> 
> Perhaps it'd make sense to consider whether to use DMERR_LIMIT or not
> depending on if the mode is logging.  Otherwise you may get weird
> interactions from having two different limits.

My intention was initially to use DMERR_LIMIT to handle error bursts, but you
are correct, it's not needed. I'll change this to DMERR in v2.

> >                         v->hash_failed = 1;
> 
> Should the dm status reflect the failure even if logging mode isn't
> returning EIOs? I think it makes sense still, but it might be good to
> note that it is intentionally kept this way.

Yes, I think it makes sense. We should be able to check the device status and
see if there have been corrupted blocks even in logging mode. I will move this
to the error handling function and add a note about it.

Sami