Patchwork iSCSI: Add support for SG_IO in bdrv_ioctl()

login
register
mail settings
Submitter ronniesahlberg@gmail.com
Date Aug. 20, 2012, 11:59 p.m.
Message ID <1345507175-7248-2-git-send-email-ronniesahlberg@gmail.com>
Download mbox | patch
Permalink /patch/178943/
State New
Headers show

Comments

ronniesahlberg@gmail.com - Aug. 20, 2012, 11:59 p.m.
We need to support SG_IO in the synchronous bdrv_ioctl() since this
is used by scsi-block

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 108 insertions(+), 1 deletions(-)
Paolo Bonzini - Aug. 21, 2012, 7 a.m.
Il 21/08/2012 01:59, Ronnie Sahlberg ha scritto:
> We need to support SG_IO in the synchronous bdrv_ioctl() since this
> is used by scsi-block
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  block/iscsi.c |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 108 insertions(+), 1 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 993a86d..9e98bfe 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -548,7 +548,8 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
>  
>  #define SG_ERR_DRIVER_SENSE    0x08
>  
> -    if (status == SCSI_STATUS_CHECK_CONDITION && acb->task->datain.size >= 2) {
> +    if (status == SCSI_STATUS_CHECK_CONDITION
> +    && acb->task->datain.size >= 2) {
>          int ss;
>  
>          acb->ioh->driver_status |= SG_ERR_DRIVER_SENSE;
> @@ -633,9 +634,53 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
>      return &acb->common;
>  }
>  
> +struct IoctlTask {
> +    int status;
> +    int complete;
> +    sg_io_hdr_t *ioh;
> +    struct scsi_task *task;
> +};
> +
> +static void
> +iscsi_ioctl_cb(struct iscsi_context *iscsi, int status,
> +                     void *command_data, void *opaque)
> +{
> +    struct IoctlTask *itask = opaque;
> +
> +    if (status < 0) {
> +        error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        itask->status = -EIO;
> +    }
> +
> +    itask->ioh->driver_status = 0;
> +    itask->ioh->host_status   = 0;
> +    itask->ioh->resid         = 0;
> +
> +#define SG_ERR_DRIVER_SENSE    0x08
> +
> +    if (status == SCSI_STATUS_CHECK_CONDITION
> +    && itask->task->datain.size >= 2) {
> +        int ss;
> +
> +        itask->ioh->driver_status |= SG_ERR_DRIVER_SENSE;
> +
> +        itask->ioh->sb_len_wr = itask->task->datain.size - 2;
> +        ss = (itask->ioh->mx_sb_len >= itask->ioh->sb_len_wr) ?
> +             itask->ioh->mx_sb_len : itask->ioh->sb_len_wr;
> +        memcpy(itask->ioh->sbp, &itask->task->datain.data[2], ss);
> +    }
> +
> +    itask->complete = 1;
> +}
> +
>  static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
>  {
>      IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    struct IoctlTask itask;
> +    struct scsi_task *task;
> +    struct iscsi_data data;
>  
>      switch (req) {
>      case SG_GET_VERSION_NUM:
> @@ -644,6 +689,68 @@ static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
>      case SG_GET_SCSI_ID:
>          ((struct sg_scsi_id *)buf)->scsi_type = iscsilun->type;
>          break;
> +    case SG_IO:
> +        itask.ioh = buf;
> +        task = malloc(sizeof(struct scsi_task));
> +        if (task == NULL) {
> +            error_report("iSCSI: Failed to allocate task for scsi command. %s",
> +                         iscsi_get_error(iscsi));
> +            return -1;
> +        }
> +        memset(task, 0, sizeof(struct scsi_task));
> +
> +        switch (itask.ioh->dxfer_direction) {
> +        case SG_DXFER_TO_DEV:
> +            task->xfer_dir = SCSI_XFER_WRITE;
> +            break;
> +        case SG_DXFER_FROM_DEV:
> +            task->xfer_dir = SCSI_XFER_READ;
> +            break;
> +        default:
> +            task->xfer_dir = SCSI_XFER_NONE;
> +            break;
> +        }
> +        task->cdb_size = itask.ioh->cmd_len;
> +        memcpy(&task->cdb[0], itask.ioh->cmdp, itask.ioh->cmd_len);
> +        task->expxferlen = itask.ioh->dxfer_len;
> +
> +        if (task->xfer_dir == SCSI_XFER_WRITE) {
> +            data.data = itask.ioh->dxferp;
> +            data.size = itask.ioh->dxfer_len;
> +        }
> +
> +        if (iscsi_scsi_command_async(iscsi, iscsilun->lun, task,
> +                                     iscsi_ioctl_cb,
> +                                     (task->xfer_dir == SCSI_XFER_WRITE) ?
> +                                         &data : NULL,
> +                                     &itask) != 0) {
> +            scsi_free_scsi_task(task);
> +            return -1;
> +        }
> +
> +        /* tell libiscsi to read straight into the buffer we got from ioctl */
> +        if (task->xfer_dir == SCSI_XFER_READ) {
> +            scsi_task_add_data_in_buffer(task,
> +                                         itask.ioh->dxfer_len,
> +                                         itask.ioh->dxferp);
> +        }
> +
> +        itask.complete = 0;
> +        itask.status   = 0;
> +        itask.task     = task;
> +        while (!itask.complete) {
> +            iscsi_set_events(iscsilun);
> +            qemu_aio_wait();
> +        }
> +        scsi_free_scsi_task(task);
> +
> +        if (itask.status != 0) {
> +            error_report("iSCSI: Failed to send async command to target : %s",
> +                         iscsi_get_error(iscsi));
> +            return -1;
> +        }
> +
> +        return 0;
>      default:
>          return -1;
>      }
> 

Lots of duplicate code, can you just call bdrv_aio_ioctl with a callback
that is as simple as

    int *p_status = opaque;
    *p_status = status;

and then

    status = -EINPROGRESS;
    bdrv_aio_ioctl(..., ioctl_cb, &status);
    while (status == -EINPROGRESS) {
        qemu_aio_wait();
    }

?

The iscsi_set_events should not be needed because bdrv_aio_ioctl calls it.

Paolo

Patch

diff --git a/block/iscsi.c b/block/iscsi.c
index 993a86d..9e98bfe 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -548,7 +548,8 @@  iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
 
 #define SG_ERR_DRIVER_SENSE    0x08
 
-    if (status == SCSI_STATUS_CHECK_CONDITION && acb->task->datain.size >= 2) {
+    if (status == SCSI_STATUS_CHECK_CONDITION
+    && acb->task->datain.size >= 2) {
         int ss;
 
         acb->ioh->driver_status |= SG_ERR_DRIVER_SENSE;
@@ -633,9 +634,53 @@  static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     return &acb->common;
 }
 
+struct IoctlTask {
+    int status;
+    int complete;
+    sg_io_hdr_t *ioh;
+    struct scsi_task *task;
+};
+
+static void
+iscsi_ioctl_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    struct IoctlTask *itask = opaque;
+
+    if (status < 0) {
+        error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        itask->status = -EIO;
+    }
+
+    itask->ioh->driver_status = 0;
+    itask->ioh->host_status   = 0;
+    itask->ioh->resid         = 0;
+
+#define SG_ERR_DRIVER_SENSE    0x08
+
+    if (status == SCSI_STATUS_CHECK_CONDITION
+    && itask->task->datain.size >= 2) {
+        int ss;
+
+        itask->ioh->driver_status |= SG_ERR_DRIVER_SENSE;
+
+        itask->ioh->sb_len_wr = itask->task->datain.size - 2;
+        ss = (itask->ioh->mx_sb_len >= itask->ioh->sb_len_wr) ?
+             itask->ioh->mx_sb_len : itask->ioh->sb_len_wr;
+        memcpy(itask->ioh->sbp, &itask->task->datain.data[2], ss);
+    }
+
+    itask->complete = 1;
+}
+
 static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 {
     IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    struct IoctlTask itask;
+    struct scsi_task *task;
+    struct iscsi_data data;
 
     switch (req) {
     case SG_GET_VERSION_NUM:
@@ -644,6 +689,68 @@  static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
     case SG_GET_SCSI_ID:
         ((struct sg_scsi_id *)buf)->scsi_type = iscsilun->type;
         break;
+    case SG_IO:
+        itask.ioh = buf;
+        task = malloc(sizeof(struct scsi_task));
+        if (task == NULL) {
+            error_report("iSCSI: Failed to allocate task for scsi command. %s",
+                         iscsi_get_error(iscsi));
+            return -1;
+        }
+        memset(task, 0, sizeof(struct scsi_task));
+
+        switch (itask.ioh->dxfer_direction) {
+        case SG_DXFER_TO_DEV:
+            task->xfer_dir = SCSI_XFER_WRITE;
+            break;
+        case SG_DXFER_FROM_DEV:
+            task->xfer_dir = SCSI_XFER_READ;
+            break;
+        default:
+            task->xfer_dir = SCSI_XFER_NONE;
+            break;
+        }
+        task->cdb_size = itask.ioh->cmd_len;
+        memcpy(&task->cdb[0], itask.ioh->cmdp, itask.ioh->cmd_len);
+        task->expxferlen = itask.ioh->dxfer_len;
+
+        if (task->xfer_dir == SCSI_XFER_WRITE) {
+            data.data = itask.ioh->dxferp;
+            data.size = itask.ioh->dxfer_len;
+        }
+
+        if (iscsi_scsi_command_async(iscsi, iscsilun->lun, task,
+                                     iscsi_ioctl_cb,
+                                     (task->xfer_dir == SCSI_XFER_WRITE) ?
+                                         &data : NULL,
+                                     &itask) != 0) {
+            scsi_free_scsi_task(task);
+            return -1;
+        }
+
+        /* tell libiscsi to read straight into the buffer we got from ioctl */
+        if (task->xfer_dir == SCSI_XFER_READ) {
+            scsi_task_add_data_in_buffer(task,
+                                         itask.ioh->dxfer_len,
+                                         itask.ioh->dxferp);
+        }
+
+        itask.complete = 0;
+        itask.status   = 0;
+        itask.task     = task;
+        while (!itask.complete) {
+            iscsi_set_events(iscsilun);
+            qemu_aio_wait();
+        }
+        scsi_free_scsi_task(task);
+
+        if (itask.status != 0) {
+            error_report("iSCSI: Failed to send async command to target : %s",
+                         iscsi_get_error(iscsi));
+            return -1;
+        }
+
+        return 0;
     default:
         return -1;
     }