Re: [RFC] block: fix barrier error transmission

James Bottomley
2008-04-02T23:11:59Z

On Wed, 2008-04-02 at 21:08 +0200, Jens Axboe wrote:
> > diff --git a/block/blk-barrier.c b/block/blk-barrier.c
> > index 55c5f1f..3a3947c 100644
> > --- a/block/blk-barrier.c
> > +++ b/block/blk-barrier.c
> > @@ -114,18 +114,24 @@ void blk_ordered_complete_seq(struct request_queue *q, unsigned seq, int error)
> >
> > static void pre_flush_end_io(struct request *rq, int error)
> > {
> > + error = rq->errors ? -EIO : error;
> > +
> > elv_completed_request(rq->q, rq);
> > blk_ordered_complete_seq(rq->q, QUEUE_ORDSEQ_PREFLUSH, error);
> > }
>
> It's a bit of a hack, SCSI really should pass the error value back
> instead of fiddling around with possibly perhaps finding it in ->errors.
> And please don't use these ?: constructs, in this case it doesn't even
> make a lot of sense and a
>
> if (rq->errors)
> error = -EIO;
>
> would have been much cleaner ;-)
>
> So my question is why does the model not allow you to return the error
> properly?

I thought it was the sg_io that would be the problem, but apparently on
further research, it simply discards the error as does scsi_execute_req.

I suppose that's a strong enough reason to try returning an error ...
I'm just a bit leery this close to a release.

I think this will work ... it just really needs quite a bit of
testing ...

James

---

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1b199e1..67f412b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -839,7 +839,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
int this_count = scsi_bufflen(cmd);
struct request_queue *q = cmd->device->request_queue;
struct request *req = cmd->request;
- int clear_errors = 1;
+ int error = 0;
struct scsi_sense_hdr sshdr;
int sense_valid = 0;
int sense_deferred = 0;
@@ -853,7 +853,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
if (blk_pc_request(req)) { /* SG_IO ioctl from block level */
req->errors = result;
if (result) {
- clear_errors = 0;
if (sense_valid && req->sense) {
/*
* SG_IO wants current and deferred errors
@@ -865,6 +864,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
memcpy(req->sense, cmd->sense_buffer, len);
req->sense_len = len;
}
+ if (!sense_deferred)
+ error = -EIO;
}
if (scsi_bidi_cmnd(cmd)) {
/* will also release_buffers */
@@ -885,14 +886,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
"%d bytes done.\n",
req->nr_sectors, good_bytes));

- if (clear_errors)
- req->errors = 0;
-
/* A number of bytes were successfully read. If there
* are leftovers and there is some kind of error
* (result != 0), retry the rest.
*/
- if (scsi_end_request(cmd, 0, good_bytes, result == 0) == NULL)
+ if (scsi_end_request(cmd, error, good_bytes, result == 0) == NULL)
return;

/* good_bytes = 0, or (inclusive) there were leftovers and