The iscsi iser transport is introdcued since QEMU 2.9. For iscsi blockdev
json, it will be shown at 'transport' field:
'json:{...,{"driver": "iscsi","transport":"iser",...}}'
For legacy drive filename as iscsi uri, it will start with 'iser'
scheme:
iser://[[username][%<password>]@]<host>[:<port>]/<target-iqn-name>/<lun>
By default, the iscsi transport is still tcp.
Signed-off-by: Han Han <hhan@redhat.com>
---
src/qemu/qemu_backup.c | 1 +
src/qemu/qemu_block.c | 16 +++++++++++++---
src/qemu/qemu_command.c | 17 +++++++++++++++++
src/qemu/qemu_monitor_json.c | 1 +
src/storage/storage_file_gluster.c | 7 +++++++
src/util/virstoragefile.c | 26 ++++++++++++++++++--------
src/util/virstoragefile.h | 1 +
7 files changed, 58 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 80fc5d77..caf0b4ce 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -88,6 +88,7 @@ qemuBackupPrepare(virDomainBackupDefPtr def)
/* TODO: Do we need to mess with selinux? */
break;
+ case VIR_STORAGE_NET_HOST_TRANS_ISER:
case VIR_STORAGE_NET_HOST_TRANS_RDMA:
case VIR_STORAGE_NET_HOST_TRANS_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index d32277d7..003c18f1 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -418,10 +418,16 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src)
if (VIR_ALLOC(uri) < 0)
return NULL;
- if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
+ if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
+ src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_RDMA ||
+ src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER)
uri->port = src->hosts->port;
+ if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
uri->scheme = g_strdup(virStorageNetProtocolTypeToString(src->protocol));
+ } else if (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
+ src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER) {
+ uri->scheme = g_strdup("iser");
} else {
uri->scheme = g_strdup_printf("%s+%s",
virStorageNetProtocolTypeToString(src->protocol),
@@ -497,6 +503,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host,
return NULL;
break;
+ case VIR_STORAGE_NET_HOST_TRANS_ISER:
case VIR_STORAGE_NET_HOST_TRANS_RDMA:
case VIR_STORAGE_NET_HOST_TRANS_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -743,6 +750,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
{
qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
g_autofree char *target = NULL;
+ const char *transport = NULL;
char *lunStr = NULL;
char *username = NULL;
char *objalias = NULL;
@@ -762,6 +770,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
*/
target = g_strdup(src->path);
+ transport = virStorageNetHostTransportTypeToString(src->hosts->transport);
/* Separate the target and lun */
if ((lunStr = strchr(target, '/'))) {
@@ -791,7 +800,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
"s:portal", portal,
"s:target", target,
"u:lun", lun,
- "s:transport", "tcp",
+ "s:transport", transport,
"S:user", username,
"S:password-secret", objalias,
"S:initiator-name", src->initiator.iqn,
@@ -2063,7 +2072,8 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr src,
/* generate simplified URIs for the easy cases */
if (actualType == VIR_STORAGE_TYPE_NETWORK &&
src->nhosts == 1 &&
- src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
+ (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
+ src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER) &&
src->timeout == 0 &&
src->ncookies == 0 &&
src->sslverify == VIR_TRISTATE_BOOL_ABSENT &&
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45dd8307..c5cf9401 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1443,6 +1443,16 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk,
return -1;
}
}
+
+ if (disk->src &&
+ disk->src->hosts &&
+ disk->src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_TRANSPORT_ISER)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("iSCSI iser transport is not supported by this "
+ "QEMU binary"));
+ return -1;
+ }
}
if (disk->serial &&
@@ -4888,6 +4898,13 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
qemuDomainStorageSourcePrivatePtr srcPriv =
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
+ if (iscsisrc->src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_TRANSPORT_ISER)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support iSCSI iser transport"));
+ return NULL;
+ }
+
if (qemuDiskSourceNeedsProps(iscsisrc->src, qemuCaps)) {
if (!(srcprops = qemuDiskSourceGetProps(iscsisrc->src)))
return NULL;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 619717ea..338f66ef 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7199,6 +7199,7 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
case VIR_STORAGE_NET_HOST_TRANS_UNIX:
addr = qemuMonitorJSONBuildUnixSocketAddress(server->socket);
break;
+ case VIR_STORAGE_NET_HOST_TRANS_ISER:
case VIR_STORAGE_NET_HOST_TRANS_RDMA:
case VIR_STORAGE_NET_HOST_TRANS_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c
index f389a944..73dfef0c 100644
--- a/src/storage/storage_file_gluster.c
+++ b/src/storage/storage_file_gluster.c
@@ -78,6 +78,13 @@ virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPrivPtr priv,
hoststr = host->socket;
break;
+ case VIR_STORAGE_NET_HOST_TRANS_ISER:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid transport '%s' for gluster host"),
+ transport);
+ return -1;
+ break;
+
case VIR_STORAGE_NET_HOST_TRANS_LAST:
break;
}
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ffc8bdb3..4f162f10 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -97,6 +97,7 @@ VIR_ENUM_IMPL(virStorageNetHostTransport,
"tcp",
"unix",
"rdma",
+ "iser",
);
VIR_ENUM_IMPL(virStorageSourcePoolMode,
@@ -2839,10 +2840,15 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
if (!scheme[0] ||
(src->protocol = virStorageNetProtocolTypeFromString(scheme[0])) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("invalid backing protocol '%s'"),
- NULLSTR(scheme[0]));
- return -1;
+ if (STRNEQ(scheme[0], "iser")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid backing protocol '%s'"),
+ NULLSTR(scheme[0]));
+ return -1;
+ }
+
+ src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
+ src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_ISER;
}
if (scheme[1] &&
@@ -3523,14 +3529,17 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
return -1;
src->nhosts = 1;
+ src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
- if (STRNEQ_NULLABLE(transport, "tcp")) {
+ if (STRNEQ(transport, "tcp") && STRNEQ(transport, "iser") && transport) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("only TCP transport is supported for iSCSI volumes"));
+ _("only TCP or iSER transport is supported for iSCSI "
+ "volumes"));
return -1;
}
- src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
+ if (transport)
+ src->hosts->transport = virStorageNetHostTransportTypeFromString(transport);
if (!portal) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -4710,7 +4719,8 @@ virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src)
size_t i;
for (i = 0; i < src->nhosts; i++) {
- if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
+ if ((src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
+ src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_ISER) &&
src->hosts[i].port == 0)
src->hosts[i].port = virStorageSourceNetworkDefaultPort(src->protocol);
}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 7939c09c..120d6190 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -146,6 +146,7 @@ typedef enum {
VIR_STORAGE_NET_HOST_TRANS_TCP,
VIR_STORAGE_NET_HOST_TRANS_UNIX,
VIR_STORAGE_NET_HOST_TRANS_RDMA,
+ VIR_STORAGE_NET_HOST_TRANS_ISER,
VIR_STORAGE_NET_HOST_TRANS_LAST
} virStorageNetHostTransport;
--
2.25.0
On Mon, Apr 27, 2020 at 10:01:07 +0800, Han Han wrote:
> The iscsi iser transport is introdcued since QEMU 2.9. For iscsi blockdev
> json, it will be shown at 'transport' field:
> 'json:{...,{"driver": "iscsi","transport":"iser",...}}'
>
> For legacy drive filename as iscsi uri, it will start with 'iser'
> scheme:
> iser://[[username][%<password>]@]<host>[:<port>]/<target-iqn-name>/<lun>
>
> By default, the iscsi transport is still tcp.
>
> Signed-off-by: Han Han <hhan@redhat.com>
> ---
> src/qemu/qemu_backup.c | 1 +
> src/qemu/qemu_block.c | 16 +++++++++++++---
> src/qemu/qemu_command.c | 17 +++++++++++++++++
> src/qemu/qemu_monitor_json.c | 1 +
> src/storage/storage_file_gluster.c | 7 +++++++
> src/util/virstoragefile.c | 26 ++++++++++++++++++--------
> src/util/virstoragefile.h | 1 +
> 7 files changed, 58 insertions(+), 11 deletions(-)
[...]
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index d32277d7..003c18f1 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -418,10 +418,16 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src)
> if (VIR_ALLOC(uri) < 0)
> return NULL;
>
> - if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
> + if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
> + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_RDMA ||
> + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER)
> uri->port = src->hosts->port;
RDMA is added here along with ISER. Please add it separately with proper
justification and a test.
Additionally I'm unsure that the design proposed in this patchset is
entirely correct. According to the iSER protocol RFC 5046 iSER is meant
as an extension to the iSCSI protocol. Specifically it's layered on top
of any transport (called RCaP - "RDMA-Capable Protocol" in the RFC).
https://tools.ietf.org/html/rfc5046#section-1.7
Trying to handle it as a transport is thus wrong ...
> + if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
> uri->scheme = g_strdup(virStorageNetProtocolTypeToString(src->protocol));
> + } else if (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
> + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER) {
... and weird. E.g. this code could end-up formatting nbd+iser if the
protocol is not iSCSI in the else branch below.
I'm not even sure whether our use of 'rdma' makes sense, but iSER
appropriated as a transport seems to be even worse.
Unfortunately I don't have any other idea besides changing this to
another network storage protocol rather than transport.
> + uri->scheme = g_strdup("iser");
> } else {
> uri->scheme = g_strdup_printf("%s+%s",
> virStorageNetProtocolTypeToString(src->protocol),
> @@ -497,6 +503,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host,
> return NULL;
> break;
>
> + case VIR_STORAGE_NET_HOST_TRANS_ISER:
> case VIR_STORAGE_NET_HOST_TRANS_RDMA:
> case VIR_STORAGE_NET_HOST_TRANS_LAST:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -743,6 +750,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
> {
> qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> g_autofree char *target = NULL;
> + const char *transport = NULL;
> char *lunStr = NULL;
> char *username = NULL;
> char *objalias = NULL;
> @@ -762,6 +770,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
> */
>
> target = g_strdup(src->path);
> + transport = virStorageNetHostTransportTypeToString(src->hosts->transport);
>
> /* Separate the target and lun */
> if ((lunStr = strchr(target, '/'))) {
> @@ -791,7 +800,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
> "s:portal", portal,
> "s:target", target,
> "u:lun", lun,
> - "s:transport", "tcp",
> + "s:transport", transport,
> "S:user", username,
> "S:password-secret", objalias,
> "S:initiator-name", src->initiator.iqn,
> @@ -2063,7 +2072,8 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr src,
> /* generate simplified URIs for the easy cases */
> if (actualType == VIR_STORAGE_TYPE_NETWORK &&
> src->nhosts == 1 &&
> - src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
> + (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
> + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER) &&
> src->timeout == 0 &&
> src->ncookies == 0 &&
> src->sslverify == VIR_TRISTATE_BOOL_ABSENT &&
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 45dd8307..c5cf9401 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1443,6 +1443,16 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk,
> return -1;
> }
> }
> +
> + if (disk->src &&
> + disk->src->hosts &&
> + disk->src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER &&
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_TRANSPORT_ISER)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("iSCSI iser transport is not supported by this "
> + "QEMU binary"));
> + return -1;
> + }
> }
If the design were okay, this would still require interlocking with
other protocols.
>
> if (disk->serial &&
> @@ -4888,6 +4898,13 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
> qemuDomainStorageSourcePrivatePtr srcPriv =
> QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
>
> + if (iscsisrc->src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER &&
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_TRANSPORT_ISER)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("This QEMU doesn't support iSCSI iser transport"));
> + return NULL;
> + }
> +
> if (qemuDiskSourceNeedsProps(iscsisrc->src, qemuCaps)) {
> if (!(srcprops = qemuDiskSourceGetProps(iscsisrc->src)))
> return NULL;
[...]
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index ffc8bdb3..4f162f10 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -97,6 +97,7 @@ VIR_ENUM_IMPL(virStorageNetHostTransport,
> "tcp",
> "unix",
> "rdma",
> + "iser",
> );
>
> VIR_ENUM_IMPL(virStorageSourcePoolMode,
> @@ -2839,10 +2840,15 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
>
> if (!scheme[0] ||
> (src->protocol = virStorageNetProtocolTypeFromString(scheme[0])) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("invalid backing protocol '%s'"),
> - NULLSTR(scheme[0]));
> - return -1;
> + if (STRNEQ(scheme[0], "iser")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("invalid backing protocol '%s'"),
> + NULLSTR(scheme[0]));
> + return -1;
> + }
> +
> + src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
> + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_ISER;
> }
>
> if (scheme[1] &&
> @@ -3523,14 +3529,17 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
> return -1;
>
> src->nhosts = 1;
> + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
>
> - if (STRNEQ_NULLABLE(transport, "tcp")) {
> + if (STRNEQ(transport, "tcp") && STRNEQ(transport, "iser") && transport) {
If 'transport' is NULL, this will crash on the first strneq. You'd have
to check it first.
> virReportError(VIR_ERR_INVALID_ARG, "%s",
> - _("only TCP transport is supported for iSCSI volumes"));
> + _("only TCP or iSER transport is supported for iSCSI "
> + "volumes"));
> return -1;
> }
>
> - src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
> + if (transport)
> + src->hosts->transport = virStorageNetHostTransportTypeFromString(transport);
>
> if (!portal) {
> virReportError(VIR_ERR_INVALID_ARG, "%s",
> @@ -4710,7 +4719,8 @@ virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src)
> size_t i;
>
> for (i = 0; i < src->nhosts; i++) {
> - if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
> + if ((src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
> + src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_ISER) &&
This would assume that iSER has addressing equal to TCP, but if layered
on top of something else that might not be the case.
> src->hosts[i].port == 0)
> src->hosts[i].port = virStorageSourceNetworkDefaultPort(src->protocol);
> }
On Mon, May 11, 2020 at 8:02 PM Peter Krempa <pkrempa@redhat.com> wrote:
> On Mon, Apr 27, 2020 at 10:01:07 +0800, Han Han wrote:
> > The iscsi iser transport is introdcued since QEMU 2.9. For iscsi blockdev
> > json, it will be shown at 'transport' field:
> > 'json:{...,{"driver": "iscsi","transport":"iser",...}}'
> >
> > For legacy drive filename as iscsi uri, it will start with 'iser'
> > scheme:
> > iser://[[username][%<password>]@]<host>[:<port>]/<target-iqn-name>/<lun>
> >
> > By default, the iscsi transport is still tcp.
> >
> > Signed-off-by: Han Han <hhan@redhat.com>
> > ---
> > src/qemu/qemu_backup.c | 1 +
> > src/qemu/qemu_block.c | 16 +++++++++++++---
> > src/qemu/qemu_command.c | 17 +++++++++++++++++
> > src/qemu/qemu_monitor_json.c | 1 +
> > src/storage/storage_file_gluster.c | 7 +++++++
> > src/util/virstoragefile.c | 26 ++++++++++++++++++--------
> > src/util/virstoragefile.h | 1 +
> > 7 files changed, 58 insertions(+), 11 deletions(-)
>
> [...]
>
> > diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> > index d32277d7..003c18f1 100644
> > --- a/src/qemu/qemu_block.c
> > +++ b/src/qemu/qemu_block.c
> > @@ -418,10 +418,16 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr
> src)
> > if (VIR_ALLOC(uri) < 0)
> > return NULL;
> >
> > - if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
> > + if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
> > + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_RDMA ||
> > + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER)
> > uri->port = src->hosts->port;
>
> RDMA is added here along with ISER. Please add it separately with proper
> justification and a test.
>
> Additionally I'm unsure that the design proposed in this patchset is
> entirely correct. According to the iSER protocol RFC 5046 iSER is meant
>
I just simply adapted it to the qemu iser transport of iscsi:
https://github.com/qemu/qemu/blob/d5c75ec500d96f1d93447f990cd5a4ef5ba27fae/qapi/block-core.json#L3510
Well, iSER is iSCSI specific, it will not be used in multiple protocols
like tcp or unix transport. It is better to design that as
an iscsi only attrib:
<source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2'
iser='yes'>
...
> as an extension to the iSCSI protocol. Specifically it's layered on top
> of any transport (called RCaP - "RDMA-Capable Protocol" in the RFC).
>
> https://tools.ietf.org/html/rfc5046#section-1.7
>
> Trying to handle it as a transport is thus wrong ...
>
>
> > + if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
> > uri->scheme =
> g_strdup(virStorageNetProtocolTypeToString(src->protocol));
> > + } else if (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
> > + src->hosts->transport ==
> VIR_STORAGE_NET_HOST_TRANS_ISER) {
>
> ... and weird. E.g. this code could end-up formatting nbd+iser if the
> protocol is not iSCSI in the else branch below.
>
> I'm not even sure whether our use of 'rdma' makes sense, but iSER
> appropriated as a transport seems to be even worse.
>
'rdma' network storage transport doesn't make sense for the qemu driver. It
is only used in gluster.
However, the struct BlockdevOptionsGluster has no attribs associated with
rdma. For the legacy gluster uri
scheme gluster+rdma, in fact it is the same as tcp transport:
https://github.com/qemu/qemu/blob/762fa6d79aa30e1a713444da0399739423f8d00e/block/gluster.c#L375
For the storage driver, I am not sure if gluster+rdma works.
>
> Unfortunately I don't have any other idea besides changing this to
> another network storage protocol rather than transport.
>
> > + uri->scheme = g_strdup("iser");
> > } else {
> > uri->scheme = g_strdup_printf("%s+%s",
> >
> virStorageNetProtocolTypeToString(src->protocol),
> > @@ -497,6 +503,7 @@
> qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host,
> > return NULL;
> > break;
> >
> > + case VIR_STORAGE_NET_HOST_TRANS_ISER:
> > case VIR_STORAGE_NET_HOST_TRANS_RDMA:
> > case VIR_STORAGE_NET_HOST_TRANS_LAST:
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > @@ -743,6 +750,7 @@
> qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
> > {
> > qemuDomainStorageSourcePrivatePtr srcPriv =
> QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> > g_autofree char *target = NULL;
> > + const char *transport = NULL;
> > char *lunStr = NULL;
> > char *username = NULL;
> > char *objalias = NULL;
> > @@ -762,6 +770,7 @@
> qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
> > */
> >
> > target = g_strdup(src->path);
> > + transport =
> virStorageNetHostTransportTypeToString(src->hosts->transport);
> >
> > /* Separate the target and lun */
> > if ((lunStr = strchr(target, '/'))) {
> > @@ -791,7 +800,7 @@
> qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src,
> > "s:portal", portal,
> > "s:target", target,
> > "u:lun", lun,
> > - "s:transport", "tcp",
> > + "s:transport", transport,
> > "S:user", username,
> > "S:password-secret", objalias,
> > "S:initiator-name",
> src->initiator.iqn,
> > @@ -2063,7 +2072,8 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr
> src,
> > /* generate simplified URIs for the easy cases */
> > if (actualType == VIR_STORAGE_TYPE_NETWORK &&
> > src->nhosts == 1 &&
> > - src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
> > + (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP ||
> > + src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_ISER)
> &&
> > src->timeout == 0 &&
> > src->ncookies == 0 &&
> > src->sslverify == VIR_TRISTATE_BOOL_ABSENT &&
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 45dd8307..c5cf9401 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -1443,6 +1443,16 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk,
> > return -1;
> > }
> > }
> > +
> > + if (disk->src &&
> > + disk->src->hosts &&
> > + disk->src->hosts->transport ==
> VIR_STORAGE_NET_HOST_TRANS_ISER &&
> > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_TRANSPORT_ISER)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > + _("iSCSI iser transport is not supported by this "
> > + "QEMU binary"));
> > + return -1;
> > + }
> > }
>
> If the design were okay, this would still require interlocking with
> other protocols.
>
> >
> > if (disk->serial &&
> > @@ -4888,6 +4898,13 @@
> qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
> > qemuDomainStorageSourcePrivatePtr srcPriv =
> > QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
> >
> > + if (iscsisrc->src->hosts->transport ==
> VIR_STORAGE_NET_HOST_TRANS_ISER &&
> > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_TRANSPORT_ISER)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > + _("This QEMU doesn't support iSCSI iser
> transport"));
> > + return NULL;
> > + }
> > +
> > if (qemuDiskSourceNeedsProps(iscsisrc->src, qemuCaps)) {
> > if (!(srcprops = qemuDiskSourceGetProps(iscsisrc->src)))
> > return NULL;
>
> [...]
>
> > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> > index ffc8bdb3..4f162f10 100644
> > --- a/src/util/virstoragefile.c
> > +++ b/src/util/virstoragefile.c
> > @@ -97,6 +97,7 @@ VIR_ENUM_IMPL(virStorageNetHostTransport,
> > "tcp",
> > "unix",
> > "rdma",
> > + "iser",
> > );
> >
> > VIR_ENUM_IMPL(virStorageSourcePoolMode,
> > @@ -2839,10 +2840,15 @@
> virStorageSourceParseBackingURI(virStorageSourcePtr src,
> >
> > if (!scheme[0] ||
> > (src->protocol =
> virStorageNetProtocolTypeFromString(scheme[0])) < 0) {
> > - virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("invalid backing protocol '%s'"),
> > - NULLSTR(scheme[0]));
> > - return -1;
> > + if (STRNEQ(scheme[0], "iser")) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("invalid backing protocol '%s'"),
> > + NULLSTR(scheme[0]));
> > + return -1;
> > + }
> > +
> > + src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
> > + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_ISER;
> > }
> >
> > if (scheme[1] &&
> > @@ -3523,14 +3529,17 @@
> virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
> > return -1;
> >
> > src->nhosts = 1;
> > + src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
> >
> > - if (STRNEQ_NULLABLE(transport, "tcp")) {
> > + if (STRNEQ(transport, "tcp") && STRNEQ(transport, "iser") &&
> transport) {
>
> If 'transport' is NULL, this will crash on the first strneq. You'd have
> to check it first.
>
>
> > virReportError(VIR_ERR_INVALID_ARG, "%s",
> > - _("only TCP transport is supported for iSCSI
> volumes"));
> > + _("only TCP or iSER transport is supported for
> iSCSI "
> > + "volumes"));
> > return -1;
> > }
> >
> > - src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
> > + if (transport)
> > + src->hosts->transport =
> virStorageNetHostTransportTypeFromString(transport);
> >
> > if (!portal) {
> > virReportError(VIR_ERR_INVALID_ARG, "%s",
> > @@ -4710,7 +4719,8 @@
> virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src)
> > size_t i;
> >
> > for (i = 0; i < src->nhosts; i++) {
> > - if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
> > + if ((src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP
> ||
> > + src->hosts[i].transport ==
> VIR_STORAGE_NET_HOST_TRANS_ISER) &&
>
> This would assume that iSER has addressing equal to TCP, but if layered
> on top of something else that might not be the case.
>
> > src->hosts[i].port == 0)
> > src->hosts[i].port =
> virStorageSourceNetworkDefaultPort(src->protocol);
> > }
>
>
--
Best regards,
-----------------------------------
Han Han
Quality Engineer
Redhat.
Email: hhan@redhat.com
Phone: +861065339333
© 2016 - 2026 Red Hat, Inc.