* [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
@ 2016-07-27 10:02 Roy Shterman
2016-07-28 8:45 ` Peter Lieven
2016-08-01 13:50 ` Paolo Bonzini
0 siblings, 2 replies; 8+ messages in thread
From: Roy Shterman @ 2016-07-27 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: roysh, ronniesahlberg, pbonzini, pl
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 <roysh@mellanox.com>
---
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;
--
1.7.8.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-07-27 10:02 [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU Roy Shterman
@ 2016-07-28 8:45 ` Peter Lieven
2016-08-01 13:52 ` Paolo Bonzini
2016-08-06 8:48 ` Roy Shterman
2016-08-01 13:50 ` Paolo Bonzini
1 sibling, 2 replies; 8+ messages in thread
From: Peter Lieven @ 2016-07-28 8:45 UTC (permalink / raw)
To: Roy Shterman, qemu-devel; +Cc: ronniesahlberg, pbonzini
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 <roysh@mellanox.com>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-07-27 10:02 [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU Roy Shterman
2016-07-28 8:45 ` Peter Lieven
@ 2016-08-01 13:50 ` Paolo Bonzini
2016-08-22 20:35 ` ronnie sahlberg
1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2016-08-01 13:50 UTC (permalink / raw)
To: Roy Shterman, qemu-devel; +Cc: ronniesahlberg, pl
On 27/07/2016 12:02, Roy Shterman wrote:
> 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.
Hi, is it too late to use the URI scheme instead---for example
iscsi+iser://.../... ? In any case this should not affect the QEMU bits.
Paolo
> 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 <roysh@mellanox.com>
> ---
> 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;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-07-28 8:45 ` Peter Lieven
@ 2016-08-01 13:52 ` Paolo Bonzini
2016-08-01 14:27 ` Roy Shterman
2016-08-06 8:48 ` Roy Shterman
1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2016-08-01 13:52 UTC (permalink / raw)
To: Peter Lieven, Roy Shterman, qemu-devel; +Cc: ronniesahlberg
On 28/07/2016 10:45, Peter Lieven wrote:
>
> Furthermore I would suggest using LIBISCS_FEATURE_ISER and not the API
> version in the
> preprocessor commands.
Actually I disagree with this suggestion. The new API could be used
also if iSER is not used, can it?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-08-01 13:52 ` Paolo Bonzini
@ 2016-08-01 14:27 ` Roy Shterman
0 siblings, 0 replies; 8+ messages in thread
From: Roy Shterman @ 2016-08-01 14:27 UTC (permalink / raw)
To: Paolo Bonzini, Peter Lieven, qemu-devel; +Cc: ronniesahlberg
On 8/1/2016 4:52 PM, Paolo Bonzini wrote:
>
> On 28/07/2016 10:45, Peter Lieven wrote:
>> Furthermore I would suggest using LIBISCS_FEATURE_ISER and not the API
>> version in the
>> preprocessor commands.
> Actually I disagree with this suggestion. The new API could be used
> also if iSER is not used, can it?
Yes, this is a new API to do zero-copy in TCP and iSER.
>
> Thanks,
>
> Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-07-28 8:45 ` Peter Lieven
2016-08-01 13:52 ` Paolo Bonzini
@ 2016-08-06 8:48 ` Roy Shterman
1 sibling, 0 replies; 8+ messages in thread
From: Roy Shterman @ 2016-08-06 8:48 UTC (permalink / raw)
To: Peter Lieven, qemu-devel; +Cc: ronniesahlberg, pbonzini
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:pl@kamp.de]
Sent: Thursday, July 28, 2016 11:46 AM
To: Roy Shterman <roysh@mellanox.com>; qemu-devel@nongnu.org
Cc: ronniesahlberg@gmail.com; pbonzini@redhat.com
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 <roysh@mellanox.com>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-08-01 13:50 ` Paolo Bonzini
@ 2016-08-22 20:35 ` ronnie sahlberg
2016-08-24 11:32 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: ronnie sahlberg @ 2016-08-22 20:35 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Roy Shterman, qemu-devel, Peter Lieven
It is never too late.
I can start working on a patch to add "iser://" URL support to
libiscsi right now. It should be a trivial change.
I think I would prefer iser:// instead of iscsi+iser:// but it is not
religiusly. Let me know if you rather want iscsi+iser.
But you would still need some changes to Roy's patch to QEMU, right?
I.e. I think you would need an additional, new block driver :
static BlockDriver bdrv_iser = {
.format_name = "iser",
.protocol_name = "iser",
...
No ?
On Mon, Aug 1, 2016 at 6:50 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 27/07/2016 12:02, Roy Shterman wrote:
>> 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.
>
> Hi, is it too late to use the URI scheme instead---for example
> iscsi+iser://.../... ? In any case this should not affect the QEMU bits.
>
> Paolo
>
>> 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 <roysh@mellanox.com>
>> ---
>> 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;
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
2016-08-22 20:35 ` ronnie sahlberg
@ 2016-08-24 11:32 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2016-08-24 11:32 UTC (permalink / raw)
To: ronnie sahlberg; +Cc: Roy Shterman, qemu-devel, Peter Lieven
----- Original Message -----
> From: "ronnie sahlberg" <ronniesahlberg@gmail.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Roy Shterman" <roysh@mellanox.com>, "qemu-devel" <qemu-devel@nongnu.org>, "Peter Lieven" <pl@kamp.de>
> Sent: Monday, August 22, 2016 10:35:03 PM
> Subject: Re: [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU
>
> It is never too late.
>
> I can start working on a patch to add "iser://" URL support to
> libiscsi right now. It should be a trivial change.
> I think I would prefer iser:// instead of iscsi+iser:// but it is not
> religiusly. Let me know if you rather want iscsi+iser.
It's up to Roy I guess. I'm fine with both.
> But you would still need some changes to Roy's patch to QEMU, right?
> I.e. I think you would need an additional, new block driver :
>
> static BlockDriver bdrv_iser = {
> .format_name = "iser",
> .protocol_name = "iser",
> ...
>
> No ?
Yes, it would.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-24 11:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-27 10:02 [Qemu-devel] [PATCH] block/iscsi: Adding iser support in Libiscsi-QEMU Roy Shterman
2016-07-28 8:45 ` Peter Lieven
2016-08-01 13:52 ` Paolo Bonzini
2016-08-01 14:27 ` Roy Shterman
2016-08-06 8:48 ` Roy Shterman
2016-08-01 13:50 ` Paolo Bonzini
2016-08-22 20:35 ` ronnie sahlberg
2016-08-24 11:32 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.