* [Qemu-devel] [PATCH][v2] scsi-generic: correct error management
@ 2008-10-13 15:09 Laurent Vivier
2008-10-17 8:08 ` Aurelien Jarno
0 siblings, 1 reply; 2+ messages in thread
From: Laurent Vivier @ 2008-10-13 15:09 UTC (permalink / raw)
To: qemu-devel@nongnu.org
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]
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.
[-- Attachment #2: Type: text/x-patch, Size: 7617 bytes --]
Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
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 */
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH][v2] scsi-generic: correct error management
2008-10-13 15:09 [Qemu-devel] [PATCH][v2] scsi-generic: correct error management Laurent Vivier
@ 2008-10-17 8:08 ` Aurelien Jarno
0 siblings, 0 replies; 2+ messages in thread
From: Aurelien Jarno @ 2008-10-17 8:08 UTC (permalink / raw)
To: qemu-devel
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 <Laurent.Vivier@bull.net>
> ---
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2008-10-17 8:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-13 15:09 [Qemu-devel] [PATCH][v2] scsi-generic: correct error management Laurent Vivier
2008-10-17 8:08 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).