diff mbox

iscsi: add support for iovectors

Message ID 50BCFEF3.4070805@dlhnet.de
State New
Headers show

Commit Message

Peter Lieven Dec. 3, 2012, 7:35 p.m. UTC 
This patch adds support for directly passing the iovec
array from QEMUIOVector if libiscsi supports it.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Paolo Bonzini Dec. 12, 2012, 1:57 p.m. UTC | #1 
Il 03/12/2012 20:35, Peter Lieven ha scritto:
> This patch adds support for directly passing the iovec
> array from QEMUIOVector if libiscsi supports it.

Thanks, one question below.

> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/iscsi.c |   31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index c0b70b3..6f3ee4a 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -55,7 +55,9 @@ typedef struct IscsiAIOCB {
>      QEMUBH *bh;
>      IscsiLun *iscsilun;
>      struct scsi_task *task;
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      uint8_t *buf;
> +#endif
>      int status;
>      int canceled;
>      size_t read_size;
> @@ -192,7 +194,9 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
> 
>      trace_iscsi_aio_write16_cb(iscsi, status, acb, acb->canceled);
> 
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      g_free(acb->buf);
> +#endif
> 
>      if (acb->canceled != 0) {
>          return;
> @@ -225,7 +229,9 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>      size_t size;
>      uint32_t num_sectors;
>      uint64_t lba;
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      struct iscsi_data data;
> +#endif
> 
>      acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
>      trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
> @@ -240,8 +246,11 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>      /* XXX we should pass the iovec to write16 to avoid the extra copy */
>      /* this will allow us to get rid of 'buf' completely */
>      size = nb_sectors * BDRV_SECTOR_SIZE;
> +
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      acb->buf = g_malloc(size);
>      qemu_iovec_to_buf(acb->qiov, 0, acb->buf, size);
> +#endif
> 
>      acb->task = malloc(sizeof(struct scsi_task));
>      if (acb->task == NULL) {
> @@ -262,6 +271,17 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>      *(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors);
>      acb->task->expxferlen = size;
> 
> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
> +    if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task,
> +                                 iscsi_aio_write16_cb,
> +                                 NULL,
> +                                 acb) != 0) {
> +        scsi_free_scsi_task(acb->task);
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +    scsi_task_set_iov_out(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);

Are you sure that scsi_task_set_iov_out must be called _after_
submitting the command?

> +#else
>      data.data = acb->buf;
>      data.size = size;
> 
> @@ -274,6 +294,7 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>          qemu_aio_release(acb);
>          return NULL;
>      }
> +#endif
> 
>      iscsi_set_events(iscsilun);
> 
> @@ -312,7 +333,9 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
>      struct iscsi_context *iscsi = iscsilun->iscsi;
>      IscsiAIOCB *acb;
>      size_t qemu_read_size;
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      int i;
> +#endif
>      uint64_t lba;
>      uint32_t num_sectors;
> 
> @@ -328,7 +351,9 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
>      acb->bh          = NULL;
>      acb->status      = -EINPROGRESS;
>      acb->read_size   = qemu_read_size;
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      acb->buf         = NULL;
> +#endif
> 
>      /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
>       * may be misaligned to the LUN, so we may need to read some extra
> @@ -383,11 +408,15 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
>          return NULL;
>      }
> 
> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
> +    scsi_task_set_iov_in(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);
> +#else
>      for (i = 0; i < acb->qiov->niov; i++) {
>          scsi_task_add_data_in_buffer(acb->task,
>                  acb->qiov->iov[i].iov_len,
>                  acb->qiov->iov[i].iov_base);
>      }
> +#endif
> 
>      iscsi_set_events(iscsilun);
> 
> @@ -557,7 +586,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
>      acb->canceled    = 0;
>      acb->bh          = NULL;
>      acb->status      = -EINPROGRESS;
> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>      acb->buf         = NULL;
> +#endif
>      acb->ioh         = buf;
> 
>      acb->task = malloc(sizeof(struct scsi_task));
>
Peter Lieven Dec. 12, 2012, 3:23 p.m. UTC | #2 
Am 12.12.2012 14:57, schrieb Paolo Bonzini:
> Il 03/12/2012 20:35, Peter Lieven ha scritto:
>> This patch adds support for directly passing the iovec
>> array from QEMUIOVector if libiscsi supports it.
> 
> Thanks, one question below.
> 
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>  block/iscsi.c |   31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index c0b70b3..6f3ee4a 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -55,7 +55,9 @@ typedef struct IscsiAIOCB {
>>      QEMUBH *bh;
>>      IscsiLun *iscsilun;
>>      struct scsi_task *task;
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      uint8_t *buf;
>> +#endif
>>      int status;
>>      int canceled;
>>      size_t read_size;
>> @@ -192,7 +194,9 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
>>
>>      trace_iscsi_aio_write16_cb(iscsi, status, acb, acb->canceled);
>>
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      g_free(acb->buf);
>> +#endif
>>
>>      if (acb->canceled != 0) {
>>          return;
>> @@ -225,7 +229,9 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>      size_t size;
>>      uint32_t num_sectors;
>>      uint64_t lba;
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      struct iscsi_data data;
>> +#endif
>>
>>      acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
>>      trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
>> @@ -240,8 +246,11 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>      /* XXX we should pass the iovec to write16 to avoid the extra copy */
>>      /* this will allow us to get rid of 'buf' completely */
>>      size = nb_sectors * BDRV_SECTOR_SIZE;
>> +
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      acb->buf = g_malloc(size);
>>      qemu_iovec_to_buf(acb->qiov, 0, acb->buf, size);
>> +#endif
>>
>>      acb->task = malloc(sizeof(struct scsi_task));
>>      if (acb->task == NULL) {
>> @@ -262,6 +271,17 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>      *(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors);
>>      acb->task->expxferlen = size;
>>
>> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
>> +    if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task,
>> +                                 iscsi_aio_write16_cb,
>> +                                 NULL,
>> +                                 acb) != 0) {
>> +        scsi_free_scsi_task(acb->task);
>> +        qemu_aio_release(acb);
>> +        return NULL;
>> +    }
>> +    scsi_task_set_iov_out(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);
> 
> Are you sure that scsi_task_set_iov_out must be called _after_
> submitting the command?

This does not matter since iscsi_scsi_command_async() will only queue
the command pdu. The iov has to be set when iscsi_service() is called
which will actually send out the PDUs + payload to the socket.

Peter

> 
>> +#else
>>      data.data = acb->buf;
>>      data.size = size;
>>
>> @@ -274,6 +294,7 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>          qemu_aio_release(acb);
>>          return NULL;
>>      }
>> +#endif
>>
>>      iscsi_set_events(iscsilun);
>>
>> @@ -312,7 +333,9 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
>>      struct iscsi_context *iscsi = iscsilun->iscsi;
>>      IscsiAIOCB *acb;
>>      size_t qemu_read_size;
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      int i;
>> +#endif
>>      uint64_t lba;
>>      uint32_t num_sectors;
>>
>> @@ -328,7 +351,9 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
>>      acb->bh          = NULL;
>>      acb->status      = -EINPROGRESS;
>>      acb->read_size   = qemu_read_size;
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      acb->buf         = NULL;
>> +#endif
>>
>>      /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
>>       * may be misaligned to the LUN, so we may need to read some extra
>> @@ -383,11 +408,15 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
>>          return NULL;
>>      }
>>
>> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
>> +    scsi_task_set_iov_in(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);
>> +#else
>>      for (i = 0; i < acb->qiov->niov; i++) {
>>          scsi_task_add_data_in_buffer(acb->task,
>>                  acb->qiov->iov[i].iov_len,
>>                  acb->qiov->iov[i].iov_base);
>>      }
>> +#endif
>>
>>      iscsi_set_events(iscsilun);
>>
>> @@ -557,7 +586,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
>>      acb->canceled    = 0;
>>      acb->bh          = NULL;
>>      acb->status      = -EINPROGRESS;
>> +#if !defined(LIBISCSI_FEATURE_IOVECTOR)
>>      acb->buf         = NULL;
>> +#endif
>>      acb->ioh         = buf;
>>
>>      acb->task = malloc(sizeof(struct scsi_task));
>>
>
Paolo Bonzini Dec. 12, 2012, 3:25 p.m. UTC | #3 
Il 12/12/2012 16:23, Peter Lieven ha scritto:
>>> >> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
>>> >> +    if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task,
>>> >> +                                 iscsi_aio_write16_cb,
>>> >> +                                 NULL,
>>> >> +                                 acb) != 0) {
>>> >> +        scsi_free_scsi_task(acb->task);
>>> >> +        qemu_aio_release(acb);
>>> >> +        return NULL;
>>> >> +    }
>>> >> +    scsi_task_set_iov_out(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);
>> > 
>> > Are you sure that scsi_task_set_iov_out must be called _after_
>> > submitting the command?
> This does not matter since iscsi_scsi_command_async() will only queue
> the command pdu. The iov has to be set when iscsi_service() is called
> which will actually send out the PDUs + payload to the socket.

Ok, I queued the patch to scsi-next branch.

Paolo
diff mbox

Patch

diff --git a/block/iscsi.c b/block/iscsi.c
index c0b70b3..6f3ee4a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -55,7 +55,9 @@  typedef struct IscsiAIOCB {
     QEMUBH *bh;
     IscsiLun *iscsilun;
     struct scsi_task *task;
+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     uint8_t *buf;
+#endif
     int status;
     int canceled;
     size_t read_size;
@@ -192,7 +194,9 @@  iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,

     trace_iscsi_aio_write16_cb(iscsi, status, acb, acb->canceled);

+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     g_free(acb->buf);
+#endif

     if (acb->canceled != 0) {
         return;
@@ -225,7 +229,9 @@  iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
     size_t size;
     uint32_t num_sectors;
     uint64_t lba;
+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     struct iscsi_data data;
+#endif

     acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
     trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
@@ -240,8 +246,11 @@  iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
     /* XXX we should pass the iovec to write16 to avoid the extra copy */
     /* this will allow us to get rid of 'buf' completely */
     size = nb_sectors * BDRV_SECTOR_SIZE;
+
+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     acb->buf = g_malloc(size);
     qemu_iovec_to_buf(acb->qiov, 0, acb->buf, size);
+#endif

     acb->task = malloc(sizeof(struct scsi_task));
     if (acb->task == NULL) {
@@ -262,6 +271,17 @@  iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
     *(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors);
     acb->task->expxferlen = size;

+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+    if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task,
+                                 iscsi_aio_write16_cb,
+                                 NULL,
+                                 acb) != 0) {
+        scsi_free_scsi_task(acb->task);
+        qemu_aio_release(acb);
+        return NULL;
+    }
+    scsi_task_set_iov_out(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);
+#else
     data.data = acb->buf;
     data.size = size;

@@ -274,6 +294,7 @@  iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
         qemu_aio_release(acb);
         return NULL;
     }
+#endif

     iscsi_set_events(iscsilun);

@@ -312,7 +333,9 @@  iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
     struct iscsi_context *iscsi = iscsilun->iscsi;
     IscsiAIOCB *acb;
     size_t qemu_read_size;
+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     int i;
+#endif
     uint64_t lba;
     uint32_t num_sectors;

@@ -328,7 +351,9 @@  iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
     acb->bh          = NULL;
     acb->status      = -EINPROGRESS;
     acb->read_size   = qemu_read_size;
+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     acb->buf         = NULL;
+#endif

     /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
      * may be misaligned to the LUN, so we may need to read some extra
@@ -383,11 +408,15 @@  iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
         return NULL;
     }

+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+    scsi_task_set_iov_in(acb->task, (struct scsi_iovec*) acb->qiov->iov, acb->qiov->niov);
+#else
     for (i = 0; i < acb->qiov->niov; i++) {
         scsi_task_add_data_in_buffer(acb->task,
                 acb->qiov->iov[i].iov_len,
                 acb->qiov->iov[i].iov_base);
     }
+#endif

     iscsi_set_events(iscsilun);

@@ -557,7 +586,9 @@  static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     acb->canceled    = 0;
     acb->bh          = NULL;
     acb->status      = -EINPROGRESS;
+#if !defined(LIBISCSI_FEATURE_IOVECTOR)
     acb->buf         = NULL;
+#endif
     acb->ioh         = buf;

     acb->task = malloc(sizeof(struct scsi_task));