good catch!
On Wed, 2006-02-22 at 18:26 +0100, Arne Redlich wrote:
> This patch fixes a rather nasty behaviour (bug?): in case of I/O errors,
> IET doesn't notify the initiator and instead lets it believe the I/O
> operation succeeded.
>
> Please note that the patch below is not extensively tested. It also
> introduces a new struct iscsi_sense_hdr which is apparently somewhat
> close to struct scsi_sense_hdr from <scsi/scsi_eh.h>. I'd happily
> replace the former with the latter if you think it would be beneficial.
>
> Arne
>
> Index: kernel/target_disk.c
> ===================================================================
> --- kernel/target_disk.c (revision 29)
> +++ kernel/target_disk.c (working copy)
> @@ -73,7 +73,8 @@ static int insert_geo_m_pg(u8 *ptr, u64
> return sizeof(geo_m_pg);
> }
>
> -static int build_mode_sense_response(struct iscsi_cmnd *cmnd)
> +static int build_mode_sense_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> struct iscsi_scsi_cmd_hdr *req = cmnd_hdr(cmnd);
> struct tio *tio = cmnd->tio;
> @@ -139,7 +140,8 @@ static int build_mode_sense_response(str
> return err;
> }
>
> -static int build_inquiry_response(struct iscsi_cmnd *cmnd)
> +static int build_inquiry_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> struct iscsi_scsi_cmd_hdr *req = cmnd_hdr(cmnd);
> struct tio *tio = cmnd->tio;
> @@ -221,7 +223,8 @@ static int build_inquiry_response(struct
> return err;
> }
>
> -static int build_report_luns_response(struct iscsi_cmnd *cmnd)
> +static int build_report_luns_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> struct iscsi_scsi_cmd_hdr *req = cmnd_hdr(cmnd);
> struct tio *tio = cmnd->tio;
> @@ -265,7 +268,8 @@ static int build_report_luns_response(st
> return 0;
> }
>
> -static int build_read_capacity_response(struct iscsi_cmnd *cmnd)
> +static int build_read_capacity_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> struct tio *tio = cmnd->tio;
> u32 *data;
> @@ -284,7 +288,8 @@ static int build_read_capacity_response(
> return 0;
> }
>
> -static int build_request_sense_response(struct iscsi_cmnd *cmnd)
> +static int build_request_sense_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> struct tio *tio = cmnd->tio;
> u8 *data;
> @@ -303,7 +308,8 @@ static int build_request_sense_response(
> return 0;
> }
>
> -static int build_sevice_action_response(struct iscsi_cmnd *cmnd)
> +static int build_sevice_action_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> struct tio *tio = cmnd->tio;
> u32 *data;
> @@ -323,19 +329,29 @@ static int build_sevice_action_response(
> return 0;
> }
>
> -static int build_read_response(struct iscsi_cmnd *cmnd)
> +static int build_read_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> + int err;
> struct tio *tio = cmnd->tio;
>
> assert(tio);
> assert(cmnd->lun);
> - tio_read(cmnd->lun, tio);
> -
> - return 0;
> + err = tio_read(cmnd->lun, tio);
> + if (err) {
> + sshdr->valid = 1;
> + /* Medium Error/Unrecovered Read Error */
> + sshdr->sense_key = MEDIUM_ERROR;
> + sshdr->asc = 0x11;
> + sshdr->ascq = 0x0;
> + }
> + return err;
> }
>
> -static int build_write_response(struct iscsi_cmnd *cmnd)
> +static int build_write_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> + int err;
> struct tio *tio = cmnd->tio;
>
> assert(cmnd);
> @@ -343,13 +359,22 @@ static int build_write_response(struct i
> assert(cmnd->lun);
>
> list_del_init(&cmnd->list);
> - tio_write(cmnd->lun, tio);
> - tio_sync(cmnd->lun, tio);
> -
> - return 0;
> + err = tio_write(cmnd->lun, tio);
> + if (!err)
> + err = tio_sync(cmnd->lun, tio);
> +
> + if (err) {
> + sshdr->valid = 1;
> + /* Medium Error/Write Fault */
> + sshdr->sense_key = MEDIUM_ERROR;
> + sshdr->asc = 0x03;
> + sshdr->ascq = 0x0;
> + }
> + return err;
> }
>
> -static int build_generic_response(struct iscsi_cmnd *cmnd)
> +static int build_generic_response(struct iscsi_cmnd *cmnd,
> + struct iscsi_sense_hdr *sshdr)
> {
> return 0;
> }
> Index: kernel/iscsi.c
> ===================================================================
> --- kernel/iscsi.c (revision 29)
> +++ kernel/iscsi.c (working copy)
> @@ -315,25 +315,6 @@ static struct iscsi_cmnd *create_scsi_rs
> return rsp;
> }
>
> -void send_scsi_rsp(struct iscsi_cmnd *req, int (*func)(struct iscsi_cmnd *))
> -{
> - struct iscsi_cmnd *rsp;
> - struct iscsi_scsi_rsp_hdr *rsp_hdr;
> - u32 size;
> -
> - rsp = create_scsi_rsp(req);
> - rsp_hdr = (struct iscsi_scsi_rsp_hdr *) &rsp->pdu.bhs;
> - if ((size = cmnd_read_size(req)) != 0) {
> - rsp_hdr->flags |= ISCSI_FLG_RESIDUAL_UNDERFLOW;
> - rsp_hdr->residual_count = cpu_to_be32(size);
> - }
> -
> - if (func(req) < 0)
> - eprintk("%x\n", cmnd_opcode(req));
> -
> - iscsi_cmnd_init_write(rsp);
> -}
> -
> static struct iscsi_cmnd *create_sense_rsp(struct iscsi_cmnd *req,
> u8 sense_key, u8 asc, u8 ascq)
> {
> @@ -369,12 +350,43 @@ static struct iscsi_cmnd *create_sense_r
> return rsp;
> }
>
> -void send_data_rsp(struct iscsi_cmnd *req, int (*func)(struct iscsi_cmnd *))
> +void send_scsi_rsp(struct iscsi_cmnd *req,
> + int (*func)(struct iscsi_cmnd *, struct iscsi_sense_hdr *))
> +{
> + struct iscsi_cmnd *rsp;
> + struct iscsi_scsi_rsp_hdr *rsp_hdr;
> + struct iscsi_sense_hdr sshdr;
> + u32 size;
> +
> + sshdr.valid = 0;
> + if (func(req, &sshdr) < 0)
> + rsp = sshdr.valid ?
> + create_sense_rsp(req, sshdr.sense_key, sshdr.asc,
> + sshdr.ascq) :
> + create_sense_rsp(req, ILLEGAL_REQUEST, 0x24, 0x0);
> + else {
> + rsp = create_scsi_rsp(req);
> + rsp_hdr = (struct iscsi_scsi_rsp_hdr *) &rsp->pdu.bhs;
> + if ((size = cmnd_read_size(req)) != 0) {
> + rsp_hdr->flags |= ISCSI_FLG_RESIDUAL_UNDERFLOW;
> + rsp_hdr->residual_count = cpu_to_be32(size);
> + }
> + }
> + iscsi_cmnd_init_write(rsp);
> +}
> +
> +void send_data_rsp(struct iscsi_cmnd *req, int (*func)(struct iscsi_cmnd *,
> + struct iscsi_sense_hdr
> *))
> {
> struct iscsi_cmnd *rsp;
> + struct iscsi_sense_hdr sshdr;
>
> - if (func(req) < 0) {
> - rsp = create_sense_rsp(req, ILLEGAL_REQUEST, 0x24, 0x0);
> + sshdr.valid = 0;
> + if (func(req, &sshdr) < 0) {
> + rsp = sshdr.valid ?
> + create_sense_rsp(req, sshdr.sense_key, sshdr.asc,
> + sshdr.ascq) :
> + create_sense_rsp(req, ILLEGAL_REQUEST, 0x24, 0x0);
> iscsi_cmnd_init_write(rsp);
> } else
> do_send_data_rsp(req);
> Index: kernel/iscsi.h
> ===================================================================
> --- kernel/iscsi.h (revision 29)
> +++ kernel/iscsi.h (working copy)
> @@ -254,6 +254,13 @@ struct iscsi_cmnd {
> struct iscsi_cmnd *req;
> };
>
> +struct iscsi_sense_hdr {
> + u8 valid;
> + u8 sense_key;
> + u8 asc;
> + u8 ascq;
> +};
> +
> #define ISCSI_OP_SCSI_REJECT ISCSI_OP_VENDOR1_CMD
> #define ISCSI_OP_PDU_REJECT ISCSI_OP_VENDOR2_CMD
> #define ISCSI_OP_DATA_REJECT ISCSI_OP_VENDOR3_CMD
> @@ -266,8 +273,13 @@ extern void cmnd_rx_end(struct iscsi_cmn
> extern void cmnd_tx_start(struct iscsi_cmnd *);
> extern void cmnd_tx_end(struct iscsi_cmnd *);
> extern void cmnd_release(struct iscsi_cmnd *, int);
> -extern void send_data_rsp(struct iscsi_cmnd *, int (*)(struct iscsi_cmnd *));
> -extern void send_scsi_rsp(struct iscsi_cmnd *, int (*)(struct iscsi_cmnd *));
> +extern void send_data_rsp(struct iscsi_cmnd *,
> + int (*)(struct iscsi_cmnd *,
> + struct iscsi_sense_hdr *));
> +
> +extern void send_scsi_rsp(struct iscsi_cmnd *,
> + int (*)(struct iscsi_cmnd *,
> + struct iscsi_sense_hdr *));
>
> /* conn.c */
> extern struct iscsi_conn *conn_lookup(struct iscsi_session *, u16);
> Index: kernel/file-io.c
> ===================================================================
> --- kernel/file-io.c (revision 29)
> +++ kernel/file-io.c (working copy)
> @@ -26,7 +26,7 @@ static int fileio_make_request(struct ie
> u32 offset, size;
> loff_t ppos, count;
> char *buf;
> - int i;
> + int i, err = 0;
> ssize_t ret;
>
> assert(p);
> @@ -58,15 +58,17 @@ static int fileio_make_request(struct ie
>
> set_fs(oldfs);
>
> - if (ret != count)
> + if (ret != count) {
> eprintk("I/O error %lld, %ld\n", count, (long) ret);
> + err = ret;
> + }
>
> size -= count;
> offset = 0;
> }
> assert(!size);
>
> - return 0;
> + return err;
> }
>
> static int fileio_sync(struct iet_volume *lu, struct tio *tio)
>
>
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems? Stop! Download the new AJAX search engine that makes
> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
> _______________________________________________
> Iscsitarget-devel mailing list
> Iscsitarget-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
|
