[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QE
|
From: |
Roy Shterman |
|
Subject: |
Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU |
|
Date: |
Sat, 6 Aug 2016 08:48:51 +0000 |
Hi,
I checked my patch and found only warnings about long lines (over 80
characters),
Splitting those lines will make the code much less verbose in my opinion. If
you still think
I should split those lines I will do it gladly.
Other than that I couldn't find any coding style issue in my patch.
Thanks,
Roy
-----Original Message-----
From: Peter Lieven [mailto:address@hidden
Sent: Thursday, July 28, 2016 11:46 AM
To: Roy Shterman <address@hidden>; address@hidden
Cc: address@hidden; address@hidden
Subject: Re: [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
Am 27.07.2016 um 12:02 schrieb Roy Shterman:
> iSER is a new transport layer supported in Libiscsi, iSER provides a
> zero-copy RDMA capable interface that can improve performance.
>
> New API is introduced in abstracion of the Libiscsi transport layer.
> In order to use the new iSER transport, one need to add the ?iser
> option at the end of Libiscsi URI.
>
> For now iSER memory buffers are pre-allocated and pre-registered,
> hence in order to work with iSER from QEMU, one need to enable MEMLOCK
> attribute in the VM to be large enough for all iSER buffers and RDMA
> resources.
>
> A new functionallity is also introduced in this commit, a new API to
> deploy zero-copy command submission. iSER is differing from TCP in
> data-path, hence IO vectors must be transferred already when queueing
> the PDU.
>
> Signed-off-by: Roy Shterman <address@hidden>
> ---
> block/iscsi.c | 45 +++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c index 7e78ade..6b95636
> 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -41,6 +41,7 @@
> #include "qapi/qmp/qstring.h"
> #include "crypto/secret.h"
>
> +#include "qemu/uri.h"
> #include <iscsi/iscsi.h>
> #include <iscsi/scsi-lowlevel.h>
>
> @@ -484,6 +485,18 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t
> sector_num, int nb_sectors,
> iscsi_co_init_iscsitask(iscsilun, &iTask);
> retry:
> if (iscsilun->use_16_for_rw) {
> +#if LIBISCSI_API_VERSION >= (20160603)
> + iTask.task = iscsi_write16_iov_task(iscsilun->iscsi, iscsilun->lun,
> lba,
> + NULL, num_sectors *
> iscsilun->block_size,
> + iscsilun->block_size, 0, 0, fua,
> 0, 0,
> + iscsi_co_generic_cb, &iTask,
> (struct scsi_iovec *)iov->iov, iov->niov);
> + } else {
> + iTask.task = iscsi_write10_iov_task(iscsilun->iscsi, iscsilun->lun,
> lba,
> + NULL, num_sectors *
> iscsilun->block_size,
> + iscsilun->block_size, 0, 0, fua,
> 0, 0,
> + iscsi_co_generic_cb, &iTask,
> (struct scsi_iovec *)iov->iov, iov->niov);
> + }
> +#else
> iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
> NULL, num_sectors *
> iscsilun->block_size,
> iscsilun->block_size, 0, 0,
> fua, 0, 0, @@ -494,11 +507,14 @@ retry:
> iscsilun->block_size, 0, 0, fua, 0,
> 0,
> iscsi_co_generic_cb, &iTask);
> }
> +#endif
> if (iTask.task == NULL) {
> return -ENOMEM;
> }
> +#if LIBISCSI_API_VERSION < (20160603)
> scsi_task_set_iov_out(iTask.task, (struct scsi_iovec *) iov->iov,
> iov->niov);
> +#endif
> while (!iTask.complete) {
> iscsi_set_events(iscsilun);
> qemu_coroutine_yield();
> @@ -677,6 +693,19 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState
> *bs,
> iscsi_co_init_iscsitask(iscsilun, &iTask);
> retry:
> if (iscsilun->use_16_for_rw) {
> +#if LIBISCSI_API_VERSION >= (20160603)
> + iTask.task = iscsi_read16_iov_task(iscsilun->iscsi, iscsilun->lun,
> lba,
> + num_sectors *
> iscsilun->block_size,
> + iscsilun->block_size, 0, 0, 0, 0,
> 0,
> + iscsi_co_generic_cb, &iTask,
> (struct scsi_iovec *)iov->iov, iov->niov);
> + } else {
> + iTask.task = iscsi_read10_iov_task(iscsilun->iscsi, iscsilun->lun,
> lba,
> + num_sectors *
> iscsilun->block_size,
> + iscsilun->block_size,
> + 0, 0, 0, 0, 0,
> + iscsi_co_generic_cb, &iTask,
> (struct scsi_iovec *)iov->iov, iov->niov);
> + }
> +#else
> iTask.task = iscsi_read16_task(iscsilun->iscsi, iscsilun->lun, lba,
> num_sectors * iscsilun->block_size,
> iscsilun->block_size, 0, 0,
> 0, 0, 0, @@ -688,11 +717,13 @@ retry:
> 0, 0, 0, 0, 0,
> iscsi_co_generic_cb, &iTask);
> }
> +#endif
> if (iTask.task == NULL) {
> return -ENOMEM;
> }
> +#if LIBISCSI_API_VERSION < (20160603)
> scsi_task_set_iov_in(iTask.task, (struct scsi_iovec *) iov->iov,
> iov->niov);
> -
> +#endif
> while (!iTask.complete) {
> iscsi_set_events(iscsilun);
> qemu_coroutine_yield();
> @@ -1477,9 +1508,9 @@ static int iscsi_open(BlockDriverState *bs,
> QDict *options, int flags,
>
> filename = qemu_opt_get(opts, "filename");
>
> - iscsi_url = iscsi_parse_full_url(iscsi, filename);
> + iscsi_url = iscsi_parse_full_url(iscsi,
> + uri_string_unescape(filename, -1, NULL));
> if (iscsi_url == NULL) {
> - error_setg(errp, "Failed to parse URL : %s", filename);
> + error_setg(errp, "Failed to parse URL : %s",
> + uri_string_unescape(filename, -1, NULL));
> ret = -EINVAL;
> goto out;
> }
> @@ -1494,7 +1525,13 @@ static int iscsi_open(BlockDriverState *bs, QDict
> *options, int flags,
> ret = -ENOMEM;
> goto out;
> }
> -
> +#if LIBISCSI_API_VERSION >= (20160603)
> + if (iscsi_init_transport(iscsi, iscsi_url->transport)) {
> + error_setg(errp, ("Error initializing transport."));
> + ret = -EINVAL;
> + goto out;
> + }
> +#endif
> if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
> error_setg(errp, "iSCSI: Failed to set target name.");
> ret = -EINVAL;
Hi Roy,
your patch has style problems. Please use scripts/checkpatch.pl to check for
errors.
Furthermore I would suggest using LIBISCS_FEATURE_ISER and not the API version
in the preprocessor commands.
Peter