[libvirt] [PATCH 0/9] iSCSI fixes and improvements

Pavel Hrdina (9): docs: fix iscsi-direct XML example conf: properly translate iscsi-direct storage pool conf: rename and move virStoragePoolSourceInitiatorAttr util: introduce virStorageSourceInitiator functions conf: use virStorageSourceInitiator functions docs: move storage initiator def into storagecommon.rng conf: introduce initiator IQN support for domain disks tests: introduce qemu disk-network-iscsi-modern test cases qemu: add support for domain disk initiator IQN docs/formatdomain.html.in | 20 +++++ docs/schemas/domaincommon.rng | 3 + docs/schemas/storagecommon.rng | 11 +++ docs/schemas/storagepool.rng | 11 --- docs/storage.html.in | 6 +- src/conf/domain_conf.c | 71 +++++++++++++----- src/conf/storage_conf.c | 16 ++-- src/conf/storage_conf.h | 7 +- src/qemu/qemu_block.c | 2 + src/qemu/qemu_domain.c | 9 +++ src/util/virstoragefile.c | 39 ++++++++++ src/util/virstoragefile.h | 23 ++++++ .../disk-network-iscsi-modern.args | 58 ++++++++++++++ .../disk-network-iscsi-modern.xml | 75 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 + 15 files changed, 306 insertions(+), 49 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.xml -- 2.17.1

The <initiator> element is part of <source> element. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/storage.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/storage.html.in b/docs/storage.html.in index 744819d99d..e9e6ec7423 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -447,10 +447,10 @@ <source> <host name="iscsi.example.com"/> <device path="iqn.2013-06.com.example:iscsi-pool"/> + <initiator> + <iqn name="iqn.2013-06.com.example:iscsi-initiator"/> + </initiator> </source> - <initiator> - <iqn name="iqn.2013-06.com.example:iscsi-initiator"/> - </initiator> </pool></pre> <h3>Valid pool format types</h3> -- 2.17.1

On Tue, Aug 07, 2018 at 03:55:20PM +0200, Pavel Hrdina wrote:
The <initiator> element is part of <source> element.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/storage.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

We cannot simply used the same code as for iscsi storage pool because the default mode is 'host' which is not possible with iscsi-direct. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dcbe8a20b..d1504610a1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30143,6 +30143,34 @@ virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, } +static int +virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + + if (virDomainDiskTranslateSourcePoolAuth(def, + &pooldef->source) < 0) + return -1; + + /* Source pool may not fill in the secrettype field, + * so we need to do so here + */ + if (def->src->auth && !def->src->auth->secrettype) { + const char *secrettype = + virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); + if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0) + return -1; + } + + if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) + return -1; + + return 0; +} + + int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) { @@ -30253,6 +30281,20 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) break; case VIR_STORAGE_POOL_ISCSI_DIRECT: + if (def->startupPolicy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT; + + if (virDomainDiskTranslateISCSIDircect(def, pooldef) < 0) + goto cleanup; + + break; + case VIR_STORAGE_POOL_ISCSI: if (def->startupPolicy) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -30273,24 +30315,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) break; case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - - if (virDomainDiskTranslateSourcePoolAuth(def, - &pooldef->source) < 0) - goto cleanup; - - /* Source pool may not fill in the secrettype field, - * so we need to do so here - */ - if (def->src->auth && !def->src->auth->secrettype) { - const char *secrettype = - virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); - if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0) - goto cleanup; - } - - if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) + if (virDomainDiskTranslateISCSIDircect(def, pooldef) < 0) goto cleanup; break; } -- 2.17.1

On Tue, Aug 07, 2018 at 03:55:21PM +0200, Pavel Hrdina wrote:
We cannot simply used the same code as for iscsi storage pool because the default mode is 'host' which is not possible with iscsi-direct.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dcbe8a20b..d1504610a1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30143,6 +30143,34 @@ virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, }
+static int +virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def,
s/Dircect/Direct/
+ virStoragePoolDefPtr pooldef) +{ + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; +
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Tue, Aug 07, 2018 at 04:27:21PM +0200, Ján Tomko wrote:
On Tue, Aug 07, 2018 at 03:55:21PM +0200, Pavel Hrdina wrote:
We cannot simply used the same code as for iscsi storage pool because the default mode is 'host' which is not possible with iscsi-direct.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 61 +++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dcbe8a20b..d1504610a1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30143,6 +30143,34 @@ virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, }
+static int +virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def,
s/Dircect/Direct/
Nice catch, I'll fix it before pushing, thanks. Pavel

This structure will be reused by domain disk images as well. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/storage_conf.h | 7 +------ src/util/virstoragefile.h | 6 ++++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 858623783d..4d7abbbc98 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -140,11 +140,6 @@ struct _virStoragePoolSourceDeviceExtent { int type; /* virStorageFreeType */ }; -typedef struct _virStoragePoolSourceInitiatorAttr virStoragePoolSourceInitiatorAttr; -struct _virStoragePoolSourceInitiatorAttr { - char *iqn; /* Initiator IQN */ -}; - /* * Pools can be backed by one or more devices, and some * allow us to track free space on underlying devices. @@ -189,7 +184,7 @@ struct _virStoragePoolSource { char *name; /* Initiator IQN */ - virStoragePoolSourceInitiatorAttr initiator; + virStorageSourceInitiatorDef initiator; /* Authentication information */ virStorageAuthDefPtr auth; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c2c40edf68..d9e27a4a5f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -226,6 +226,12 @@ struct _virStoragePRDef { char *mgralias; }; +typedef struct _virStorageSourceInitiatorDef virStorageSourceInitiatorDef; +typedef virStorageSourceInitiatorDef *virStorageSourceInitiatorDefPtr; +struct _virStorageSourceInitiatorDef { + char *iqn; /* Initiator IQN */ +}; + typedef struct _virStorageDriverData virStorageDriverData; typedef virStorageDriverData *virStorageDriverDataPtr; -- 2.17.1

On Tue, Aug 07, 2018 at 03:55:22PM +0200, Pavel Hrdina wrote:
This structure will be reused by domain disk images as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/storage_conf.h | 7 +------ src/util/virstoragefile.h | 6 ++++++ 2 files changed, 7 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

The same code would be used for storage pools and domain disks. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virstoragefile.c | 34 ++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 15 +++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 56082f34e9..fb79ceddd0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4314,6 +4314,40 @@ virStorageSourcePrivateDataFormatRelPath(virStorageSourcePtr src, return 0; } +void +virStorageSourceInitiatorParseXML(xmlXPathContextPtr ctxt, + virStorageSourceInitiatorDefPtr initiator) +{ + initiator->iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); +} + +void +virStorageSourceInitiatorFormatXML(virStorageSourceInitiatorDefPtr initiator, + virBufferPtr buf) +{ + if (!initiator->iqn) + return; + + virBufferAddLit(buf, "<initiator>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<iqn name='%s'/>\n", initiator->iqn); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</initiator>\n"); +} + +int +virStorageSourceInitiatorCopy(virStorageSourceInitiatorDefPtr dest, + const virStorageSourceInitiatorDef *src) +{ + return VIR_STRDUP(dest->iqn, src->iqn); +} + +void +virStorageSourceInitiatorClear(virStorageSourceInitiatorDefPtr initiator) +{ + VIR_FREE(initiator->iqn); +} + static bool virStorageFileIsInitialized(const virStorageSource *src) { diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d9e27a4a5f..b6013431cc 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -492,6 +492,21 @@ int virStorageSourcePrivateDataFormatRelPath(virStorageSourcePtr src, virBufferPtr buf); +void +virStorageSourceInitiatorParseXML(xmlXPathContextPtr ctxt, + virStorageSourceInitiatorDefPtr initiator); + +void +virStorageSourceInitiatorFormatXML(virStorageSourceInitiatorDefPtr initiator, + virBufferPtr buf); + +int +virStorageSourceInitiatorCopy(virStorageSourceInitiatorDefPtr dest, + const virStorageSourceInitiatorDef *src); + +void +virStorageSourceInitiatorClear(virStorageSourceInitiatorDefPtr initiator); + int virStorageFileInit(virStorageSourcePtr src); int virStorageFileInitAs(virStorageSourcePtr src, uid_t uid, gid_t gid); -- 2.17.1

On Tue, Aug 07, 2018 at 03:55:23PM +0200, Pavel Hrdina wrote:
The same code would be used for storage pools and domain disks.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virstoragefile.c | 34 ++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 15 +++++++++++++++ 2 files changed, 49 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On 08/07/2018 03:55 PM, Pavel Hrdina wrote:
The same code would be used for storage pools and domain disks.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virstoragefile.c | 34 ++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 15 +++++++++++++++ 2 files changed, 49 insertions(+)
Don't forget to expose these functions in libvirt_private.syms. Michal

On Wed, Aug 08, 2018 at 04:42:13PM +0200, Michal Privoznik wrote:
On 08/07/2018 03:55 PM, Pavel Hrdina wrote:
The same code would be used for storage pools and domain disks.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virstoragefile.c | 34 ++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 15 +++++++++++++++ 2 files changed, 49 insertions(+)
Don't forget to expose these functions in libvirt_private.syms.
Right, I'll fix it before pushing, thanks. Pavel

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/storage_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a0c92af820..50dbc937e7 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -380,7 +380,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source) VIR_FREE(source->dir); VIR_FREE(source->name); virStorageAdapterClear(&source->adapter); - VIR_FREE(source->initiator.iqn); + virStorageSourceInitiatorClear(&source->initiator); virStorageAuthDefFree(source->auth); VIR_FREE(source->vendor); VIR_FREE(source->product); @@ -488,7 +488,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, } VIR_FREE(nodeset); - source->initiator.iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); + + virStorageSourceInitiatorParseXML(ctxt, &source->initiator); nsource = virXPathNodeSet("./device", ctxt, &nodeset); if (nsource < 0) @@ -950,15 +951,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) virBufferEscapeString(buf, "<name>%s</name>\n", src->name); - if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) && - src->initiator.iqn) { - virBufferAddLit(buf, "<initiator>\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<iqn name='%s'/>\n", - src->initiator.iqn); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</initiator>\n"); - } + if (options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) + virStorageSourceInitiatorFormatXML(&src->initiator, buf); if (options->formatToString) { const char *format = (options->formatToString)(src->format); -- 2.17.1

On Tue, Aug 07, 2018 at 03:55:24PM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/storage_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/schemas/storagecommon.rng | 11 +++++++++++ docs/schemas/storagepool.rng | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index cb4f14f52f..9d17d934de 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -39,6 +39,17 @@ </element> </define> + <define name='initiatorinfo'> + <element name='initiator'> + <element name='iqn'> + <attribute name='name'> + <text/> + </attribute> + <empty/> + </element> + </element> + </define> + <define name="reconnect"> <element name="reconnect"> <attribute name="enabled"> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 11ac55d06f..74f4363106 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -336,17 +336,6 @@ </element> </define> - <define name='initiatorinfo'> - <element name='initiator'> - <element name='iqn'> - <attribute name='name'> - <text/> - </attribute> - <empty/> - </element> - </element> - </define> - <define name='devextents'> <oneOrMore> <element name='freeExtent'> -- 2.17.1

On Tue, Aug 07, 2018 at 03:55:25PM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/schemas/storagecommon.rng | 11 +++++++++++ docs/schemas/storagepool.rng | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 20 ++++++++++++++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 10 ++++++++++ src/util/virstoragefile.c | 5 +++++ src/util/virstoragefile.h | 2 ++ 5 files changed, 40 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b63467bd91..a7fe8b5824 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2733,6 +2733,17 @@ </source> <target dev='sdb' bus='scsi'/> </disk> + </disk> + <disk type='network' device='lun'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> + <host name='example.com' port='3260'/> + <initiator> + <iqn name='iqn.2013-07.com.example:client'/> + </initiator> + </source> + <target dev='sdb' bus='scsi'/> + </disk> <disk type='volume' device='disk'> <driver name='qemu' type='raw'/> <source pool='iscsi-pool' volume='unit:0:0:1' mode='host'/> @@ -3090,6 +3101,15 @@ It's recommended to allow libvirt manage the persistent reservations. </dd> + <dt><code>initiator</code></dt> + <dd><span class="since">Since libvirt 4.7.0</span>, the + <code>initiator</code> element is supported for a disk + <code>type</code> "network" that is using a <code>source</code> + element with the <code>protocol</code> attribute "iscsi". + If present, the <code>initiator</code> element provides the + initiator IQN needed to access the source via mandatory + attribute <code>name</code>. + </dd> </dl> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ac04af51a1..1a786968cc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1699,6 +1699,9 @@ <optional> <ref name="encryption"/> </optional> + <optional> + <ref name="initiatorinfo"/> + </optional> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d1504610a1..14112429fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8826,6 +8826,8 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, virStorageSourceNetworkAssignDefaultPorts(src); + virStorageSourceInitiatorParseXML(ctxt, &src->initiator); + ret = 0; cleanup: @@ -23541,6 +23543,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferEscapeString(childBuf, "<snapshot name='%s'/>\n", src->snapshot); virBufferEscapeString(childBuf, "<config file='%s'/>\n", src->configFile); + virStorageSourceInitiatorFormatXML(&src->initiator, childBuf); + return 0; } @@ -30167,6 +30171,12 @@ virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def, if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) return -1; + if (!def->src->initiator.iqn && pooldef->source.initiator.iqn && + virStorageSourceInitiatorCopy(&def->src->initiator, + &pooldef->source.initiator) < 0) { + return -1; + } + return 0; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fb79ceddd0..6939d0d6c9 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2272,6 +2272,9 @@ virStorageSourceCopy(const virStorageSource *src, !(ret->pr = virStoragePRDefCopy(src->pr))) goto error; + if (virStorageSourceInitiatorCopy(&ret->initiator, &src->initiator)) + goto error; + if (backingChain && src->backingStore) { if (!(ret->backingStore = virStorageSourceCopy(src->backingStore, true))) @@ -2503,6 +2506,8 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def->tlsAlias); VIR_FREE(def->tlsCertdir); + virStorageSourceInitiatorClear(&def->initiator); + memset(def, 0, sizeof(*def)); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index b6013431cc..3ff6c4f900 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -261,6 +261,8 @@ struct _virStorageSource { bool encryptionInherited; virStoragePRDefPtr pr; + virStorageSourceInitiatorDef initiator; + virObjectPtr privateData; int format; /* virStorageFileFormat in domain backing chains, but -- 2.17.1

This uses the new -drive options instead of iSCSI URI. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../disk-network-iscsi-modern.args | 52 +++++++++++++++ .../disk-network-iscsi-modern.xml | 65 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ 3 files changed, 121 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.xml diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.args b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args new file mode 100644 index 0000000000..0a3e0bdb8b --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args @@ -0,0 +1,52 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,format=raw,\ +if=none,id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example,file.lun=1,file.transport=tcp,format=raw,\ +if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example:storage,file.lun=1,file.transport=tcp,\ +file.user=myname,file.password-secret=AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A,\ +format=raw,if=none,id=drive-virtio-disk2 \ +-device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ +id=virtio-disk2 \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example:storage,file.lun=2,file.transport=tcp,\ +file.user=myname,file.password-secret=AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A,\ +format=raw,if=none,id=drive-virtio-disk3 \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk3,\ +id=virtio-disk3 \ +-drive file.driver=iscsi,file.portal=example.org:3260,\ +file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,format=raw,\ +if=none,id=drive-scsi0-0-0-0 \ +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml new file mode 100644 index 0000000000..fa2a889b54 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml @@ -0,0 +1,65 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example/1'> + <host name='example.org' port='6000'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example:storage/1'> + <host name='example.org' port='6000'/> + </source> + <target dev='vdc' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example:storage/2'> + <host name='example.org' port='6000'/> + </source> + <target dev='vdd' bus='virtio'/> + </disk> + <disk type='network' device='lun'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='3260'/> + </source> + <target dev='sda' bus='scsi'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <controller type='scsi' index='0' model='virtio-scsi'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index eb53b9afd5..16000ca59d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1057,6 +1057,10 @@ mymain(void) DO_TEST("disk-network-nbd", NONE); DO_TEST_CAPS_LATEST("disk-network-nbd"); DO_TEST("disk-network-iscsi", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_BLOCK); + DO_TEST("disk-network-iscsi-modern", + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_SCSI_BLOCK, + QEMU_CAPS_ISCSI_PASSWORD_SECRET); DO_TEST_CAPS_LATEST("disk-network-iscsi"); DO_TEST_PARSE_ERROR("disk-network-iscsi-auth-secrettype-invalid", NONE); DO_TEST_PARSE_ERROR("disk-network-iscsi-auth-wrong-secrettype", NONE); -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_block.c | 2 ++ src/qemu/qemu_domain.c | 9 +++++++++ tests/qemuxml2argvdata/disk-network-iscsi-modern.args | 8 +++++++- tests/qemuxml2argvdata/disk-network-iscsi-modern.xml | 10 ++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 66e6301210..be120b30f0 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -819,6 +819,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) * lun:1, * user:"username", * password-secret:"secret-alias", + * initiator-name:"iqn.2017-04.com.example:client" * } */ @@ -860,6 +861,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) "s:transport", "tcp", "S:user", username, "S:password-secret", objalias, + "S:initiator-name", src->initiator.iqn, NULL)); goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b50e0c484..e8960105c3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4682,6 +4682,15 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + /* Use QEMU_CAPS_ISCSI_PASSWORD_SECRET as witness that iscsi 'initiator-name' + * option is available, it was introduced at the same time. */ + if (src->initiator.iqn && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iSCSI initiator IQN not supported with this QEMU binary")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.args b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args index 0a3e0bdb8b..a06b63a940 100644 --- a/tests/qemuxml2argvdata/disk-network-iscsi-modern.args +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args @@ -49,4 +49,10 @@ id=virtio-disk3 \ file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,format=raw,\ if=none,id=drive-scsi0-0-0-0 \ -device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ -drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-drive file.driver=iscsi,file.portal=example.org:3260,\ +file.target=iqn.1992-01.com.example:server,file.lun=0,file.transport=tcp,\ +file.initiator-name=iqn.1992-01.com.example:client,format=raw,if=none,\ +id=drive-scsi0-0-0-1 \ +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=1,\ +drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml index fa2a889b54..d1be9c4e7d 100644 --- a/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml @@ -55,6 +55,16 @@ </source> <target dev='sda' bus='scsi'/> </disk> + <disk type='network' device='lun'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example:server/0'> + <host name='example.org' port='3260'/> + <initiator> + <iqn name='iqn.1992-01.com.example:client'/> + </initiator> + </source> + <target dev='sdb' bus='scsi'/> + </disk> <controller type='usb' index='0'/> <controller type='pci' index='0' model='pci-root'/> <controller type='scsi' index='0' model='virtio-scsi'/> -- 2.17.1

On 08/07/2018 03:55 PM, Pavel Hrdina wrote:
Pavel Hrdina (9): docs: fix iscsi-direct XML example conf: properly translate iscsi-direct storage pool conf: rename and move virStoragePoolSourceInitiatorAttr util: introduce virStorageSourceInitiator functions conf: use virStorageSourceInitiator functions docs: move storage initiator def into storagecommon.rng conf: introduce initiator IQN support for domain disks tests: introduce qemu disk-network-iscsi-modern test cases qemu: add support for domain disk initiator IQN
docs/formatdomain.html.in | 20 +++++ docs/schemas/domaincommon.rng | 3 + docs/schemas/storagecommon.rng | 11 +++ docs/schemas/storagepool.rng | 11 --- docs/storage.html.in | 6 +- src/conf/domain_conf.c | 71 +++++++++++++----- src/conf/storage_conf.c | 16 ++-- src/conf/storage_conf.h | 7 +- src/qemu/qemu_block.c | 2 + src/qemu/qemu_domain.c | 9 +++ src/util/virstoragefile.c | 39 ++++++++++ src/util/virstoragefile.h | 23 ++++++ .../disk-network-iscsi-modern.args | 58 ++++++++++++++ .../disk-network-iscsi-modern.xml | 75 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 + 15 files changed, 306 insertions(+), 49 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.xml
ACK series. Michal
participants (3)
-
Ján Tomko
-
Michal Privoznik
-
Pavel Hrdina