From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KqkOT-0003Wf-Vj for qemu-devel@nongnu.org; Fri, 17 Oct 2008 04:09:10 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KqkOT-0003W7-0J for qemu-devel@nongnu.org; Fri, 17 Oct 2008 04:09:09 -0400 Received: from [199.232.76.173] (port=47598 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KqkOS-0003W1-QC for qemu-devel@nongnu.org; Fri, 17 Oct 2008 04:09:08 -0400 Received: from hall.aurel32.net ([88.191.82.174]:55760) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KqkOS-0005Wz-C2 for qemu-devel@nongnu.org; Fri, 17 Oct 2008 04:09:08 -0400 Received: from volta-wlan.aurel32.net ([2002:52e8:2fb:ffff:21d:e0ff:fe49:1047] helo=volta.aurel32.net) by hall.aurel32.net with esmtpsa (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1KqkOM-0003vy-RT for qemu-devel@nongnu.org; Fri, 17 Oct 2008 10:09:02 +0200 Received: from aurel32 by volta.aurel32.net with local (Exim 4.69) (envelope-from ) id 1KqkOH-0002bU-15 for qemu-devel@nongnu.org; Fri, 17 Oct 2008 10:08:57 +0200 Date: Fri, 17 Oct 2008 10:08:57 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH][v2] scsi-generic: correct error management Message-ID: <20081017080857.GA9870@volta.aurel32.net> References: <1223910597.4153.9.camel@frecb07144> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <1223910597.4153.9.camel@frecb07144> 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 On Mon, Oct 13, 2008 at 05:09:57PM +0200, Laurent Vivier wrote: > Hi, > > this patch allows to fully use a tape device connected to qemu through > the scsi-generic interface. > > Previous patch introduced tape SCSI commands management, this one > improve error case management: > > - the SCSI controller command completion must be called with the status > value, not the sense value. In the case of scsi-generic, the SCSI status > is given by the field status of sg_io_hdr_t (the value is left shifted > by one regarding status codes defined in /usr/include/scsi/scsi.h) > > - when a read is aborted due to a mark/EOF/EOD/EOM, the len reported to > controller can be 0. LSI controller emulation doesn't know how to manage > this. A workaround found is to call the completion routine with > SCSI_REASON_DONE just after calling it with SCSI_REASON_DATA with len=0. > > This patch also manages correctly the block size of the tape device. > > This patch has been tested with a real tape device "HP C5683A", linux > guest (debian etch) and tools like "mt", "tar" and "btape". > > Windows guest is not better supported than before... > > Changelog: > > v2: Improve management of sense buffer. Store sense buffer length and > generate a fake sense buffer when an invalid LUN is given. Allow > "REQUEST SENSE" on an invalid LUN. > Applied, thanks. > Signed-off-by: Laurent Vivier > --- > hw/scsi-generic.c | 132 ++++++++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 103 insertions(+), 29 deletions(-) > > Index: qemu/hw/scsi-generic.c > =================================================================== > --- qemu.orig/hw/scsi-generic.c 2008-10-13 13:37:09.000000000 +0200 > +++ qemu/hw/scsi-generic.c 2008-10-13 16:35:34.000000000 +0200 > @@ -84,6 +84,7 @@ struct SCSIDeviceState > void *opaque; > int driver_status; > uint8_t sensebuf[SCSI_SENSE_BUF_SIZE]; > + uint8_t senselen; > }; > > /* Global pool of SCSIRequest structures. */ > @@ -154,25 +155,30 @@ static void scsi_command_complete(void * > SCSIRequest *r = (SCSIRequest *)opaque; > SCSIDeviceState *s = r->dev; > uint32_t tag; > - int sense; > + int status; > > s->driver_status = r->io_header.driver_status; > + if (s->driver_status & SG_ERR_DRIVER_SENSE) > + s->senselen = r->io_header.sb_len_wr; > + > if (ret != 0) > - sense = HARDWARE_ERROR; > + status = BUSY << 1; > else { > if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) { > - sense = HARDWARE_ERROR; > + status = BUSY << 1; > BADF("Driver Timeout\n"); > - } else if ((s->driver_status & SG_ERR_DRIVER_SENSE) == 0) > - sense = NO_SENSE; > + } else if (r->io_header.status) > + status = r->io_header.status; > + else if (s->driver_status & SG_ERR_DRIVER_SENSE) > + status = CHECK_CONDITION << 1; > else > - sense = s->sensebuf[2]; > + status = GOOD << 1; > } > - > - DPRINTF("Command complete 0x%p tag=0x%x sense=%d\n", r, r->tag, sense); > + DPRINTF("Command complete 0x%p tag=0x%x status=%d\n", > + r, r->tag, status); > 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. */ > @@ -251,6 +257,8 @@ static void scsi_read_complete(void * op > > r->len = -1; > s->completion(s->opaque, SCSI_REASON_DATA, r->tag, len); > + if (len == 0) > + scsi_command_complete(r, 0); > } > > /* Read more data from scsi device into buffer. */ > @@ -276,14 +284,17 @@ static void scsi_read_data(SCSIDevice *d > > if (r->cmd[0] == REQUEST_SENSE && s->driver_status & SG_ERR_DRIVER_SENSE) > { > - int len = MIN(r->len, SCSI_SENSE_BUF_SIZE); > - memcpy(r->buf, s->sensebuf, len); > + s->senselen = MIN(r->len, s->senselen); > + memcpy(r->buf, s->sensebuf, s->senselen); > r->io_header.driver_status = 0; > + r->io_header.status = 0; > + r->io_header.dxfer_len = s->senselen; > r->len = -1; > + DPRINTF("Data ready tag=0x%x len=%d\n", r->tag, s->senselen); > DPRINTF("Sense: %d %d %d %d %d %d %d %d\n", > r->buf[0], r->buf[1], r->buf[2], r->buf[3], > r->buf[4], r->buf[5], r->buf[6], r->buf[7]); > - s->completion(s->opaque, SCSI_REASON_DATA, r->tag, len); > + s->completion(s->opaque, SCSI_REASON_DATA, r->tag, s->senselen); > return; > } > > @@ -305,6 +316,12 @@ static void scsi_write_complete(void * o > return; > } > > + if (r->cmd[0] == MODE_SELECT && r->cmd[4] == 12 && > + r->dev->type == TYPE_TAPE) { > + r->dev->blocksize = (r->buf[9] << 16) | (r->buf[10] << 8) | r->buf[11]; > + DPRINTF("block size %d\n", r->dev->blocksize); > + } > + > scsi_command_complete(r, ret); > } > > @@ -437,6 +454,9 @@ static int scsi_length(uint8_t *cmd, int > case READ_12: > *len *= blocksize; > break; > + case INQUIRY: > + *len = cmd[4] | (cmd[3] << 8); > + break; > } > return 0; > } > @@ -519,15 +539,6 @@ static int32_t scsi_send_command(SCSIDev > SCSIRequest *r; > int ret; > > - /* ??? Tags are not unique for different luns. We only implement a > - single lun, so this should not matter. */ > - > - if (lun != s->lun || (cmd[1] >> 5) != s->lun) { > - DPRINTF("Unimplemented LUN %d\n", lun ? lun : cmd[1] >> 5); > - s->completion(s->opaque, SCSI_REASON_DONE, tag, ILLEGAL_REQUEST); > - return 0; > - } > - > if (s->type == TYPE_TAPE) { > if (scsi_stream_length(cmd, s->blocksize, &cmdlen, &len) == -1) { > BADF("Unsupported command length, command %x\n", cmd[0]); > @@ -543,6 +554,23 @@ static int32_t scsi_send_command(SCSIDev > DPRINTF("Command: lun=%d tag=0x%x data=0x%02x len %d\n", lun, tag, > cmd[0], len); > > + if (cmd[0] != REQUEST_SENSE && > + (lun != s->lun || (cmd[1] >> 5) != s->lun)) { > + DPRINTF("Unimplemented LUN %d\n", lun ? lun : cmd[1] >> 5); > + > + s->sensebuf[0] = 0x70; > + s->sensebuf[1] = 0x00; > + s->sensebuf[2] = ILLEGAL_REQUEST; > + s->sensebuf[3] = 0x00; > + s->sensebuf[4] = 0x00; > + s->sensebuf[5] = 0x00; > + s->sensebuf[6] = 0x00; > + s->senselen = 7; > + s->driver_status = SG_ERR_DRIVER_SENSE; > + s->completion(s->opaque, SCSI_REASON_DONE, tag, CHECK_CONDITION << 1); > + return 0; > + } > + > r = scsi_find_request(s, tag); > if (r) { > BADF("Tag 0x%x already in use %p\n", tag, r); > @@ -619,6 +647,43 @@ static int get_blocksize(BlockDriverStat > return (buf[4] << 24) | (buf[5] << 16) | (buf[6] << 8) | buf[7]; > } > > +static int get_stream_blocksize(BlockDriverState *bdrv) > +{ > + uint8_t cmd[6]; > + uint8_t buf[12]; > + uint8_t sensebuf[8]; > + sg_io_hdr_t io_header; > + int ret; > + > + memset(cmd, 0, sizeof(cmd)); > + memset(buf, 0, sizeof(buf)); > + cmd[0] = MODE_SENSE; > + cmd[4] = sizeof(buf); > + > + memset(&io_header, 0, sizeof(io_header)); > + io_header.interface_id = 'S'; > + io_header.dxfer_direction = SG_DXFER_FROM_DEV; > + io_header.dxfer_len = sizeof(buf); > + io_header.dxferp = buf; > + io_header.cmdp = cmd; > + io_header.cmd_len = sizeof(cmd); > + io_header.mx_sb_len = sizeof(sensebuf); > + io_header.sbp = sensebuf; > + io_header.timeout = 6000; /* XXX */ > + > + ret = bdrv_pwrite(bdrv, -1, &io_header, sizeof(io_header)); > + if (ret == -1) > + return -1; > + > + while ((ret = bdrv_pread(bdrv, -1, &io_header, sizeof(io_header))) == -1 && > + errno == EINTR); > + > + if (ret == -1) > + return -1; > + > + return (buf[9] << 16) | (buf[10] << 8) | buf[11]; > +} > + > static void scsi_destroy(SCSIDevice *d) > { > SCSIRequest *r, *n; > @@ -673,17 +738,26 @@ SCSIDevice *scsi_generic_init(BlockDrive > s->completion = completion; > s->opaque = opaque; > s->lun = scsiid.lun; > + DPRINTF("LUN %d\n", s->lun); > s->type = scsiid.scsi_type; > - s->blocksize = get_blocksize(s->bdrv); > + DPRINTF("device type %d\n", s->type); > + if (s->type == TYPE_TAPE) { > + s->blocksize = get_stream_blocksize(s->bdrv); > + if (s->blocksize == -1) > + s->blocksize = 0; > + } else { > + s->blocksize = get_blocksize(s->bdrv); > + /* removable media returns 0 if not present */ > + if (s->blocksize <= 0) { > + if (s->type == TYPE_ROM || s->type == TYPE_WORM) > + s->blocksize = 2048; > + else > + s->blocksize = 512; > + } > + } > + DPRINTF("block size %d\n", s->blocksize); > s->driver_status = 0; > memset(s->sensebuf, 0, sizeof(s->sensebuf)); > - /* removable media returns 0 if not present */ > - if (s->blocksize <= 0) { > - if (s->type == TYPE_ROM || s->type == TYPE_WORM) > - s->blocksize = 2048; > - else > - s->blocksize = 512; > - } > > /* define function to manage device */ > -- .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73 : :' : Debian developer | Electrical Engineer `. `' aurel32@debian.org | aurelien@aurel32.net `- people.debian.org/~aurel32 | www.aurel32.net