Re: SCSI eats error from flush failure during hot plug

Christoph Hellwig
2014-06-27T07:55:54Z

On Thu, Jun 26, 2014 at 02:52:14PM -0400, James Bottomley wrote:
> > + } else if (req->cmd_flags & REQ_FLUSH) {
> > + /*
> > + * Flush request don't transfer data, so we can't try
> > + * to complete the good bytes first before checking
> > + * the error.
> > + */
> > + if (result && !sense_deferred)
> > + error = __scsi_error_from_host_byte(cmd, result);
>
> Well, no, that's wrong. To catch all of the corner cases that slip
> through here, the check is for a zero length command with and error
> otherwise we get the same problem for failed discards.

It's correct for the current code base where FLUSH is the only zero
length !BLOCK_PC case. Your version is fine too, except for the
incorrect comment:

> + } else if (blk_rq_bytes(req) == 0 && result && !sense_deferred) {
> + /*
> + * Certain non BLOCK_PC requests are commands that don't
> + * actually transfer anything (FLUSH, DISCARD), so cannot use
> + * good_bytes == 0 as the signal for an error. This sets the
> + * error explicitly for the problem case
> + */

There aren't BLOCK_PC commands, otherwise they wouldn't end up here.

Note that REQ_DISCARD has been rewritten into either an WRITE_SAME or
UNMAP at this point, which always transfer data.