[Qemu-devel] [PATCH 15/16] scsi: Implement get_req_iov callback

[Qemu-devel] [PATCH 15/16] scsi: Implement get_req_iov callback

[Hannes Reinecke]

Add callback to create a request with a predefined iovec.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 hw/scsi-disk.c    |   25 +++++++++++++++++++++----
 hw/scsi-generic.c |   44 ++++++++++++++++++++++++++++++++++----------
 hw/scsi.h         |    2 ++
 3 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f9b66ac..0f144f3 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -95,14 +95,30 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
     return req;
 }
 
+static SCSIRequest *scsi_new_request_iovec(SCSIDevice *d, uint32_t tag,
+        uint32_t lun, struct iovec *iov, int iov_num)
+{
+    SCSIRequest *req;
+    SCSIDiskReq *r;
+
+    req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun);
+    r = DO_UPCAST(SCSIDiskReq, req, req);
+    r->iov = iov;
+    r->iov_num = iov_num;
+    r->iov_buf = NULL;
+    return req;
+}
+
 static void scsi_remove_request(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
-    qemu_vfree(r->iov);
-    r->iov = NULL;
-    qemu_vfree(r->iov_buf);
-    r->iov_buf = NULL;
+    if (r->iov_buf) {
+        qemu_vfree(r->iov);
+        r->iov = NULL;
+        qemu_vfree(r->iov_buf);
+        r->iov_buf = NULL;
+    }
     scsi_req_free(&r->req);
 }
 
@@ -1217,6 +1233,7 @@ static SCSIDeviceInfo scsi_disk_info = {
     .init         = scsi_disk_initfn,
     .destroy      = scsi_destroy,
     .get_req      = scsi_new_request,
+    .get_req_iov  = scsi_new_request_iovec,
     .put_req      = scsi_remove_request,
     .send_command = scsi_send_command,
     .read_data    = scsi_read_data,
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index fa03337..13fa6f4 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -107,6 +107,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
     return req;
 }
 
+static SCSIRequest *scsi_new_request_iovec(SCSIDevice *d, uint32_t tag,
+                                           uint32_t lun, struct iovec *iov, int iov_num)
+{
+    SCSIRequest *req;
+    SCSIGenericReq *r;
+    int i;
+
+    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
+    r = DO_UPCAST(SCSIGenericReq, req, req);
+    r->io_header.dxferp = iov;
+    r->io_header.iovec_count = iov_num;
+    r->io_header.dxfer_len = 0;
+    for (i = 0; i < iov_num; i++)
+        r->io_header.dxfer_len += iov[i].iov_len;
+    r->buf = NULL;
+    r->buflen = 0;
+    return req;
+}
+
 static void scsi_remove_request(SCSIRequest *req)
 {
     SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -176,8 +195,10 @@ static int execute_command(BlockDriverState *bdrv,
 
     r->io_header.interface_id = 'S';
     r->io_header.dxfer_direction = direction;
-    r->io_header.dxferp = r->buf;
-    r->io_header.dxfer_len = r->buflen;
+    if (r->buf) {
+        r->io_header.dxferp = r->buf;
+        r->io_header.dxfer_len = r->buflen;
+    }
     r->io_header.cmdp = r->req.cmd.buf;
     r->io_header.cmd_len = r->req.cmd.len;
     r->io_header.mx_sb_len = sizeof(s->sensebuf);
@@ -282,7 +303,7 @@ static int scsi_write_data(SCSIRequest *req)
 
     DPRINTF("scsi_write_data 0x%x\n", req->tag);
 
-    if (r->len == 0) {
+    if (r->len == 0 && r->io_header.dxfer_len == 0) {
         r->len = r->buflen;
         r->req.bus->complete(&r->req, SCSI_REASON_DATA, r->len);
         return 0;
@@ -376,14 +397,16 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
         return 0;
     }
 
-    if (r->buflen != r->req.cmd.xfer) {
-        if (r->buf != NULL)
-            qemu_free(r->buf);
-        r->buf = qemu_malloc(r->req.cmd.xfer);
-        r->buflen = r->req.cmd.xfer;
-    }
+    if (!r->io_header.iovec_count) {
+        if (r->buflen != r->req.cmd.xfer) {
+            if (r->buf != NULL)
+                qemu_free(r->buf);
+            r->buf = qemu_malloc(r->req.cmd.xfer);
+            r->buflen = r->req.cmd.xfer;
+        }
 
-    memset(r->buf, 0, r->buflen);
+        memset(r->buf, 0, r->buflen);
+    }
     r->len = r->req.cmd.xfer;
     if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
         r->len = 0;
@@ -553,6 +576,7 @@ static SCSIDeviceInfo scsi_generic_info = {
     .init         = scsi_generic_initfn,
     .destroy      = scsi_destroy,
     .get_req      = scsi_new_request,
+    .get_req_iov  = scsi_new_request_iovec,
     .put_req      = scsi_remove_request,
     .send_command = scsi_send_command,
     .read_data    = scsi_read_data,
diff --git a/hw/scsi.h b/hw/scsi.h
index 6c2540e..10594de 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -72,6 +72,8 @@ struct SCSIDeviceInfo {
     scsi_qdev_initfn init;
     void (*destroy)(SCSIDevice *s);
     SCSIRequest *(*get_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
+    SCSIRequest *(*get_req_iov)(SCSIDevice *s, uint32_t tag, uint32_t lun,
+                                struct iovec *iov, int iov_num);
     void (*put_req)(SCSIRequest *req);
     int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
     void (*read_data)(SCSIRequest *req);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 15/16] scsi: Implement get_req_iov callback

[Christoph Hellwig]

> @@ -95,14 +95,30 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
>      return req;
>  }
>  
> +static SCSIRequest *scsi_new_request_iovec(SCSIDevice *d, uint32_t tag,
> +        uint32_t lun, struct iovec *iov, int iov_num)
> +{
> +    SCSIRequest *req;
> +    SCSIDiskReq *r;
> +
> +    req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun);
> +    r = DO_UPCAST(SCSIDiskReq, req, req);
> +    r->iov = iov;
> +    r->iov_num = iov_num;
> +    r->iov_buf = NULL;
> +    return req;
> +}

While the amount of duplicated code here is rather small I still hate
the duplication.

The simplest step is to implement scsi_new_request on top of
scsi_new_request_iovec by just allocation the iovec in scsi_new_request
and passing it to scsi_new_request_iovec.  The next patch on top would
be to move the iovec allocation to the HBA driver and only stick to
one interface.