From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KlKeT-0004Jp-Vo for qemu-devel@nongnu.org; Thu, 02 Oct 2008 05:39:18 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KlKeS-0004JW-VN for qemu-devel@nongnu.org; Thu, 02 Oct 2008 05:39:17 -0400 Received: from [199.232.76.173] (port=53563 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KlKeS-0004JR-Kv for qemu-devel@nongnu.org; Thu, 02 Oct 2008 05:39:16 -0400 Received: from ecfrec.frec.bull.fr ([129.183.4.8]:48859) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KlKeR-0000HI-Ii for qemu-devel@nongnu.org; Thu, 02 Oct 2008 05:39:16 -0400 From: Laurent Vivier Content-Type: multipart/mixed; boundary="=-+oeuJUqgfDzBt3SSJq5h" Date: Thu, 02 Oct 2008 11:38:26 +0200 Message-Id: <1222940306.4144.16.camel@frecb07144> Mime-Version: 1.0 Subject: [Qemu-devel] [PATCH] scsi-disk: correct SCSI error reporting Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "qemu-devel@nongnu.org" Cc: Paul Brook --=-+oeuJUqgfDzBt3SSJq5h Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi, scsi-disk.c seems to not report to SCSI controller (through completion() routine) the good error value. I've tested this by using a little program trying to write beyond the end of the disk (see attachment read10.c) On real disk: # ./read10 /dev/sg0 READ_10 duration=3D40 millisecs, resid=3D0, msg_status=3D0 status=3D2 sen= se=3D0 driver_status 8 If I use this command with qemu and a scsi disk, the result is: # ./read10 /dev/sg0 sym0: bad DSA (5da40ff) in done queue. sd 0:0:0:0: ABORT operation started sym0: SCSI BUS reset detected. sym0: SCSI BUS has been reset. sd 0:0:0:0: ABORT operation complete. sd 0:0:0:0: ABORT operation started sd 0:0:0:0: ABORT operation failed. sd 0:0:0:0: DEVICE RESET operation started target0:0:0: control msgout: c. lsi_scsi: error: Unimplemented message 0x0c target0:0:0: has been reset sd 0:0:0:0: DEVICE RESET operation complete. sd 0:0:0:0: DEVICE RESET operation started target0:0:0: control msgout: c. lsi_scsi: error: Unimplemented message 0x0c target0:0:0: has been reset sd 0:0:0:0: DEVICE RESET operation complete. sd 0:0:0:0: M_REJECT received (0:0). sd 0:0:0:0: M_REJECT received (0:0). READ_10 duration=3D30032 millisecs, resid=3D0, msg_status=3D0 status=3D0 = sense=3D0 driver_status 0 The following patch corrects this issue by sending to the controller not the sense code but the status code. The result of the command is now: # ./read10 /dev/sg0 READ_10 duration=3D0 millisecs, resid=3D0, msg_status=3D0 status=3D2 sens= e=3D0 driver_status 8 This patch has been tested with linux and windows XP. The VERIFY commands has been implemented (empty) because previously it was simply ignored (and it is needed by windows to format disks). Regards, Laurent --=20 ----------------- Laurent.Vivier@bull.net ------------------ "La perfection est atteinte non quand il ne reste rien =C3=A0 ajouter mais quand il ne reste rien =C3=A0 enlever." Saint Exup=C3=A9ry --=-+oeuJUqgfDzBt3SSJq5h Content-Description: Content-Disposition: inline; filename=read10.c Content-Type: text/x-csrc; charset=UTF-8 Content-Transfer-Encoding: 7bit #include #include #include #include #include #include #include #include #include #include /* modified file from /usr/share/doc/sg3-utils/examples/sg_simple16.c * * Copyright (C) 2001 D. Gilbert * */ #define READ10_REPLY_LEN 512 #define READ10_CMD_LEN 10 #define EBUFF_SZ 256 int main(int argc, char * argv[]) { int sg_fd, k, ok; unsigned char r10CmdBlk [READ10_CMD_LEN] = {0x28, 0, 0x7f, 0xff, 0xff, 0xff, 0, 0, 1, 0}; sg_io_hdr_t io_hdr; char * file_name = 0; char ebuff[EBUFF_SZ]; unsigned char inBuff[READ10_REPLY_LEN]; unsigned char sense_buffer[32]; for (k = 1; k < argc; ++k) { if (*argv[k] == '-') { printf("Unrecognized switch: %s\n", argv[k]); file_name = 0; break; } else if (0 == file_name) file_name = argv[k]; else { printf("too many arguments\n"); file_name = 0; break; } } if (0 == file_name) { printf("Usage: 'sg_simple10 '\n"); return 1; } if ((sg_fd = open(file_name, O_RDWR)) < 0) { snprintf(ebuff, EBUFF_SZ, "sg_simple10: error opening file: %s", file_name); perror(ebuff); return 1; } /* Prepare READ_10 command */ memset(&io_hdr, 0, sizeof(sg_io_hdr_t)); io_hdr.interface_id = 'S'; io_hdr.cmd_len = sizeof(r10CmdBlk); /* io_hdr.iovec_count = 0; */ /* memset takes care of this */ io_hdr.mx_sb_len = sizeof(sense_buffer); io_hdr.dxfer_direction = SG_DXFER_FROM_DEV; io_hdr.dxfer_len = READ10_REPLY_LEN; io_hdr.dxferp = inBuff; io_hdr.cmdp = r10CmdBlk; io_hdr.sbp = sense_buffer; io_hdr.timeout = 20000; /* 20000 millisecs == 20 seconds */ /* io_hdr.flags = 0; */ /* take defaults: indirect IO, etc */ /* io_hdr.pack_id = 0; */ /* io_hdr.usr_ptr = NULL; */ if (ioctl(sg_fd, SG_IO, &io_hdr) < 0) { perror("sg_simple10: Inquiry SG_IO ioctl error"); close(sg_fd); return 1; } printf("READ_10 duration=%u millisecs, resid=%d, msg_status=%d status=%d sense=%d driver_status %x\n", io_hdr.duration, io_hdr.resid, (int)io_hdr.msg_status, io_hdr.status, sense_buffer[2], io_hdr.driver_status); close(sg_fd); return 0; } --=-+oeuJUqgfDzBt3SSJq5h Content-Description: Content-Disposition: inline; filename=scsi-disk-status.patch Content-Type: text/x-patch; charset=UTF-8 Content-Transfer-Encoding: 7bit Signed-off-by: Laurent Vivier --- hw/scsi-disk.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) Index: qemu/hw/scsi-disk.c =================================================================== --- qemu.orig/hw/scsi-disk.c 2008-10-01 10:00:11.000000000 +0200 +++ qemu/hw/scsi-disk.c 2008-10-02 10:51:22.000000000 +0200 @@ -34,6 +34,9 @@ do { fprintf(stderr, "scsi-disk: " fmt , #define SENSE_HARDWARE_ERROR 4 #define SENSE_ILLEGAL_REQUEST 5 +#define STATUS_GOOD 0 +#define STATUS_CHECK_CONDITION 2 + #define SCSI_DMA_BUF_SIZE 131072 typedef struct SCSIRequest { @@ -124,7 +127,7 @@ static SCSIRequest *scsi_find_request(SC } /* Helper function for command completion. */ -static void scsi_command_complete(SCSIRequest *r, int sense) +static void scsi_command_complete(SCSIRequest *r, int status, int sense) { SCSIDeviceState *s = r->dev; uint32_t tag; @@ -132,7 +135,7 @@ static void scsi_command_complete(SCSIRe s->sense = sense; tag = r->tag; scsi_remove_request(r); - s->completion(s->opaque, SCSI_REASON_DONE, tag, sense); + s->completion(s->opaque, SCSI_REASON_DONE, tag, status); } /* Cancel a pending data transfer. */ @@ -157,7 +160,8 @@ static void scsi_read_complete(void * op if (ret) { DPRINTF("IO error\n"); - scsi_command_complete(r, SENSE_HARDWARE_ERROR); + s->completion(s->opaque, SCSI_REASON_DATA, r->tag, 0); + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_NO_SENSE); return; } DPRINTF("Data ready tag=0x%x len=%d\n", r->tag, r->buf_len); @@ -176,7 +180,7 @@ static void scsi_read_data(SCSIDevice *d if (!r) { BADF("Bad read tag 0x%x\n", tag); /* ??? This is the wrong error. */ - scsi_command_complete(r, SENSE_HARDWARE_ERROR); + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR); return; } if (r->sector_count == (uint32_t)-1) { @@ -187,7 +191,7 @@ static void scsi_read_data(SCSIDevice *d } DPRINTF("Read sector_count=%d\n", r->sector_count); if (r->sector_count == 0) { - scsi_command_complete(r, SENSE_NO_SENSE); + scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE); return; } @@ -199,7 +203,7 @@ static void scsi_read_data(SCSIDevice *d r->aiocb = bdrv_aio_read(s->bdrv, r->sector, r->dma_buf, n, scsi_read_complete, r); if (r->aiocb == NULL) - scsi_command_complete(r, SENSE_HARDWARE_ERROR); + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR); r->sector += n; r->sector_count -= n; } @@ -217,7 +221,7 @@ static void scsi_write_complete(void * o r->aiocb = NULL; if (r->sector_count == 0) { - scsi_command_complete(r, SENSE_NO_SENSE); + scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE); } else { len = r->sector_count * 512; if (len > SCSI_DMA_BUF_SIZE) { @@ -241,7 +245,7 @@ static int scsi_write_data(SCSIDevice *d r = scsi_find_request(s, tag); if (!r) { BADF("Bad write tag 0x%x\n", tag); - scsi_command_complete(r, SENSE_HARDWARE_ERROR); + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR); return 1; } if (r->aiocb) @@ -251,7 +255,8 @@ static int scsi_write_data(SCSIDevice *d r->aiocb = bdrv_aio_write(s->bdrv, r->sector, r->dma_buf, n, scsi_write_complete, r); if (r->aiocb == NULL) - scsi_command_complete(r, SENSE_HARDWARE_ERROR); + scsi_command_complete(r, STATUS_CHECK_CONDITION, + SENSE_HARDWARE_ERROR); r->sector += n; r->sector_count -= n; } else { @@ -670,7 +675,7 @@ static int32_t scsi_send_command(SCSIDev outbuf[7] = 0; r->buf_len = 8; } else { - scsi_command_complete(r, SENSE_NOT_READY); + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_NOT_READY); return 0; } break; @@ -754,14 +759,17 @@ static int32_t scsi_send_command(SCSIDev outbuf[3] = 8; r->buf_len = 16; break; + case 0x2f: + DPRINTF("Verify (sector %d, count %d)\n", lba, len); + break; default: DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]); fail: - scsi_command_complete(r, SENSE_ILLEGAL_REQUEST); + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_ILLEGAL_REQUEST); return 0; } if (r->sector_count == 0 && r->buf_len == 0) { - scsi_command_complete(r, SENSE_NO_SENSE); + scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE); } len = r->sector_count * 512 + r->buf_len; if (is_write) { --=-+oeuJUqgfDzBt3SSJq5h--