[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] iscsi: add support for iovectors
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH] iscsi: add support for iovectors |
Date: |
Wed, 12 Dec 2012 16:23:36 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 |
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 <address@hidden>
>> ---
>> 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));
>>
>