From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KnyU8-0004qa-CG for qemu-devel@nongnu.org; Thu, 09 Oct 2008 12:35:32 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KnyU6-0004pc-Fr for qemu-devel@nongnu.org; Thu, 09 Oct 2008 12:35:31 -0400 Received: from [199.232.76.173] (port=39541 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KnyU6-0004pZ-9K for qemu-devel@nongnu.org; Thu, 09 Oct 2008 12:35:30 -0400 Received: from ecfrec.frec.bull.fr ([129.183.4.8]:52732) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KnyU5-0007rz-J9 for qemu-devel@nongnu.org; Thu, 09 Oct 2008 12:35:30 -0400 Received: from localhost (localhost [127.0.0.1]) by ecfrec.frec.bull.fr (Postfix) with ESMTP id D74441A1978 for ; Thu, 9 Oct 2008 18:34:47 +0200 (CEST) Received: from ecfrec.frec.bull.fr ([127.0.0.1]) by localhost (ecfrec.frec.bull.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 18791-01 for ; Thu, 9 Oct 2008 18:34:44 +0200 (CEST) Received: from cyclope.frec.bull.fr (cyclope.frec.bull.fr [129.183.4.9]) by ecfrec.frec.bull.fr (Postfix) with ESMTP id BE7001A18F1 for ; Thu, 9 Oct 2008 18:34:44 +0200 (CEST) Received: from [129.183.101.63] (frecb007144.frec.bull.fr [129.183.101.63]) by cyclope.frec.bull.fr (Postfix) with ESMTP id B92432728D for ; Thu, 9 Oct 2008 18:34:41 +0200 (CEST) From: Laurent Vivier Content-Type: multipart/mixed; boundary="=-RoyKg2J3NTmUM4/fczBn" Date: Thu, 09 Oct 2008 18:34:50 +0200 Message-Id: <1223570090.4143.22.camel@frecb07144> Mime-Version: 1.0 Subject: [Qemu-devel] [PATCH] [v2] 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" --=-RoyKg2J3NTmUM4/fczBn 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 read 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 key 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 (x86_64 and sparc) 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). Changelog: [v2] The v1 was very close to the (reversed) patch of Marcelo Tosatti (r4260) which was breaking sparc scsi support. In fact, management of invalid LUN was wrong: on an invalid LUN, disk sends a SCSI status of "CHECK_CONDITION" and the driver sends "REQUEST_SENSE" to retrieve the sense key. The disk must accept and answer to this request (to send the sense key ILLEGAL_REQUEST). Moreover, INQUIRY with an invalid LUN must answer with PERIPHERAL QUALIFIER field set to 011b and PERIPHERAL DEVICE TYPE field set to 1Fh. 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 --=-RoyKg2J3NTmUM4/fczBn Content-Description: Content-Disposition: attachment; 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; } --=-RoyKg2J3NTmUM4/fczBn 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 | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) Index: qemu/hw/scsi-disk.c =================================================================== --- qemu.orig/hw/scsi-disk.c 2008-10-09 16:32:38.000000000 +0200 +++ qemu/hw/scsi-disk.c 2008-10-09 18:08:12.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,15 +127,15 @@ 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; - DPRINTF("Command complete tag=0x%x sense=%d\n", r->tag, sense); + DPRINTF("Command complete tag=0x%x status=%d sense=%d\n", r->tag, status, sense); 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 { @@ -344,7 +349,8 @@ static int32_t scsi_send_command(SCSIDev if (lun || buf[1] >> 5) { /* Only LUN 0 supported. */ DPRINTF("Unimplemented LUN %d\n", lun ? lun : buf[1] >> 5); - goto fail; + if (command != 0x03 && command != 0x12) /* REQUEST SENSE and INQUIRY */ + goto fail; } switch (command) { case 0x0: @@ -487,7 +493,10 @@ static int32_t scsi_send_command(SCSIDev } } memset(outbuf, 0, 36); - if (bdrv_get_type_hint(s->bdrv) == BDRV_TYPE_CDROM) { + + if (lun || buf[1] >> 5) { + outbuf[0] = 0x7f; /* LUN not supported */ + } else if (bdrv_get_type_hint(s->bdrv) == BDRV_TYPE_CDROM) { outbuf[0] = 5; outbuf[1] = 0x80; memcpy(&outbuf[16], "QEMU CD-ROM ", 16); @@ -670,7 +679,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 +763,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) { --=-RoyKg2J3NTmUM4/fczBn--