[Qemu-devel] [PATCH] scsi: Implement 'get_sense' callback

[Qemu-devel] [PATCH] scsi: Implement 'get_sense' callback

[Hannes Reinecke]

The get_sense callback copies existing sense information into
the provided buffer. This is required if sense information
should be transferred together with the command response.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 hw/scsi-disk.c    |    9 +++++++++
 hw/scsi-generic.c |   17 +++++++++++++++++
 hw/scsi.h         |    1 +
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d43c7ae..d1b7f74 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -360,6 +360,14 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
     return r->iov_buf;
 }
 
+/* Copy sense information into the provided buffer */
+static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+
+    return scsi_build_sense(s->sense, outbuf, len, len > 14);
+}
+
 static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
@@ -1205,6 +1213,7 @@ static SCSIDeviceInfo scsi_disk_info = {
     .write_data   = scsi_write_data,
     .cancel_io    = scsi_cancel_io,
     .get_buf      = scsi_get_buf,
+    .get_sense    = scsi_get_sense,
     .qdev.props   = (Property[]) {
         DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
         DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 40b1255..5c0f6ab 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -83,6 +83,22 @@ static void scsi_clear_sense(SCSIGenericState *s)
     s->driver_status = 0;
 }
 
+static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
+{
+    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
+    int size = SCSI_SENSE_BUF_SIZE;
+
+    if (s->driver_status & SG_ERR_DRIVER_SENSE) {
+        if (len < SCSI_SENSE_BUF_SIZE)
+            size = len;
+        else
+            size = SCSI_SENSE_BUF_SIZE;
+        memcpy(outbuf, s->sensebuf, size);
+    }
+
+    return size;
+}
+
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
 {
     SCSIRequest *req;
@@ -550,6 +566,7 @@ static SCSIDeviceInfo scsi_generic_info = {
     .write_data   = scsi_write_data,
     .cancel_io    = scsi_cancel_io,
     .get_buf      = scsi_get_buf,
+    .get_sense    = scsi_get_sense,
     .qdev.props   = (Property[]) {
         DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
         DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/scsi.h b/hw/scsi.h
index 25fda2f..0c467d1 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -78,6 +78,7 @@ struct SCSIDeviceInfo {
     int (*write_data)(SCSIRequest *req);
     void (*cancel_io)(SCSIRequest *req);
     uint8_t *(*get_buf)(SCSIRequest *req);
+    int (*get_sense)(SCSIRequest *req, uint8_t *buf, int len);
 };
 
 typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH] scsi: Implement 'get_sense' callback

[Stefan Hajnoczi]

On Mon, Nov 22, 2010 at 10:15 AM, Hannes Reinecke <hare@suse.de> wrote:
> +static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
> +{
> +    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
> +    int size = SCSI_SENSE_BUF_SIZE;

If there is no error we return SCSI_SENSE_BUF_SIZE without touching
outbuf?  I was expecting a memset(outbuf, 0, ...) or something that
initializes outbuf.

> +
> +    if (s->driver_status & SG_ERR_DRIVER_SENSE) {
> +        if (len < SCSI_SENSE_BUF_SIZE)

{}

> +            size = len;
> +        else

{}

Stefan

[Qemu-devel] Re: [PATCH] scsi: Implement 'get_sense' callback

[Hannes Reinecke]

On 11/22/2010 10:56 PM, Stefan Hajnoczi wrote:
> On Mon, Nov 22, 2010 at 10:15 AM, Hannes Reinecke <hare@suse.de> wrote:
>> +static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
>> +{
>> +    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
>> +    int size = SCSI_SENSE_BUF_SIZE;
> 
> If there is no error we return SCSI_SENSE_BUF_SIZE without touching
> outbuf?  I was expecting a memset(outbuf, 0, ...) or something that
> initializes outbuf.
> 
If there is no error SG_ERR_DRIVER_SENSE is not set, hence there is
no sense data to fill out.
But yes, you are correct; we should be doing something sensible here.
I'll be setting it to 'NO SENSE' for initialisation.

>> +
>> +    if (s->driver_status & SG_ERR_DRIVER_SENSE) {
>> +        if (len < SCSI_SENSE_BUF_SIZE)
> 
> {}
> 
>> +            size = len;
>> +        else
> 
> {}
> 
OK, will be fixing it with the next patchset.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)