[libvirt] [PATCH v2] qemu: Provide default LUN=0 for iSCSI if not provided

John Ferlan posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170911232618.4433-1-jferlan@redhat.com
src/conf/domain_conf.c                             | 43 ++++++++++++++++++++++
.../qemuargv2xml-disk-drive-network-iscsi.xml      |  2 +-
.../qemuxml2argv-disk-drive-network-iscsi-lun.args |  2 +-
.../qemuxml2argv-disk-drive-network-iscsi.args     |  2 +-
.../qemuxml2argv-hostdev-scsi-lsi-iscsi.args       |  2 +-
.../qemuxml2argv-hostdev-scsi-virtio-iscsi.args    |  2 +-
.../qemuxml2xmlout-disk-drive-network-iscsi.xml    |  2 +-
.../qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml      |  2 +-
.../qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml   |  2 +-
9 files changed, 51 insertions(+), 8 deletions(-)
[libvirt] [PATCH v2] qemu: Provide default LUN=0 for iSCSI if not provided
Posted by John Ferlan 6 years, 7 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1477880

If the "/#" is missing from the provided iSCSI path, then we need
to provide the default LUN of /0; otherwise, QEMU will fail to parse
the URL causing a failure to either create the guest or hotplug
attach the storage.

During post parse, for any iSCSI disk or hostdev, scan the source
path looking for the presence of '/', if found, then we can assume
the LUN is provided.  If not found, alter the input XML to add the
"/0".  This will cause the generated XML to have the generated
value when the domain config is saved after post parse.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---

 v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00122.html

 Alter the patch to make the adjustment during device post parsing rather
 than command line creation as requested by code review. This will alter
 the output XML as well as the command line arguments for those entries
 that don't supply a LUN.  We had a few, so that works out testing wise!

 src/conf/domain_conf.c                             | 43 ++++++++++++++++++++++
 .../qemuargv2xml-disk-drive-network-iscsi.xml      |  2 +-
 .../qemuxml2argv-disk-drive-network-iscsi-lun.args |  2 +-
 .../qemuxml2argv-disk-drive-network-iscsi.args     |  2 +-
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args       |  2 +-
 .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args    |  2 +-
 .../qemuxml2xmlout-disk-drive-network-iscsi.xml    |  2 +-
 .../qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml      |  2 +-
 .../qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml   |  2 +-
 9 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 676fc0f34..a43b25c31 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4308,16 +4308,54 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
 }
 
 
+/**
+ * virDomainPostParseCheckISCSIPath
+ * @srcpath: Source path read (a/k/a, IQN) either disk or hostdev
+ *
+ * The details of an IQN is defined by RFC 3720 and 3721, but
+ * we just need to make sure there's a lun provided. If not
+ * provided, then default to zero. For an ISCSI LUN that is
+ * is provided by /dev/disk/by-path/... , then that path will
+ * have the specific lun requested.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+static int
+virDomainPostParseCheckISCSIPath(char **srcpath)
+{
+    char *path = NULL;
+
+    if (strchr(*srcpath, '/'))
+        return 0;
+
+    if (virAsprintf(&path, "%s/0", *srcpath) < 0)
+        return -1;
+    VIR_FREE(*srcpath);
+    VIR_STEAL_PTR(*srcpath, path);
+    return 0;
+}
+
+
 static int
 virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev,
                              const virDomainDef *def,
                              virDomainXMLOptionPtr xmlopt)
 {
+    virDomainHostdevSubsysSCSIPtr scsisrc;
+
     if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
         return 0;
 
     switch (dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+        scsisrc = &dev->source.subsys.u.scsi;
+        if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
+            virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
+
+            if (virDomainPostParseCheckISCSIPath(&iscsisrc->path) < 0)
+                return -1;
+        }
+
         if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
             virDomainHostdevAssignAddress(xmlopt, def, dev) < 0) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -4455,6 +4493,11 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
             }
         }
 
+        if (disk->src->type == VIR_STORAGE_TYPE_NETWORK &&
+            disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
+            virDomainPostParseCheckISCSIPath(&disk->src->path) < 0)
+            return -1;
+
         if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO &&
             virDomainCheckVirtioOptions(disk->virtio) < 0)
             return -1;
diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi.xml b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi.xml
index 23542fa1d..694412b5c 100644
--- a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi.xml
+++ b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi.xml
@@ -16,7 +16,7 @@
     <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'>
+      <source protocol='iscsi' name='iqn.1992-01.com.example/0'>
         <host name='example.org' port='6000'/>
       </source>
       <target dev='vda' bus='virtio'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
index b25f3a0d6..0fbfd9a6d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
@@ -21,7 +21,7 @@ server,nowait \
 -boot c \
 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
 -usb \
--drive file=iscsi://example.org:3260/iqn.1992-01.com.example,format=raw,\
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/0,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/qemuxml2argv-disk-drive-network-iscsi.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args
index a1d93af10..ed15fda21 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args
@@ -19,7 +19,7 @@ server,nowait \
 -no-acpi \
 -boot c \
 -usb \
--drive file=iscsi://example.org:6000/iqn.1992-01.com.example,format=raw,\
+-drive file=iscsi://example.org:6000/iqn.1992-01.com.example/0,format=raw,\
 if=none,id=drive-virtio-disk0 \
 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
 id=virtio-disk0 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
index 43c555a50..07ae9f592 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
@@ -22,7 +22,7 @@ server,nowait \
 -usb \
 -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--drive file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,\
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/0,if=none,\
 format=raw,id=drive-hostdev0 \
 -device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \
 -drive file=iscsi://example.org:3260/iqn.1992-01.com.example/1,if=none,\
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
index a78e3092c..d80c85918 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
@@ -22,7 +22,7 @@ server,nowait \
 -usb \
 -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--drive file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,\
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/0,if=none,\
 format=raw,id=drive-hostdev0 \
 -device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=4,\
 drive=drive-hostdev0,id=hostdev0 \
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-iscsi.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-iscsi.xml
index 23542fa1d..694412b5c 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-iscsi.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-iscsi.xml
@@ -16,7 +16,7 @@
     <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'>
+      <source protocol='iscsi' name='iqn.1992-01.com.example/0'>
         <host name='example.org' port='6000'/>
       </source>
       <target dev='vda' bus='virtio'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml
index e19bce6ea..b7312ca4f 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml
@@ -32,7 +32,7 @@
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <hostdev mode='subsystem' type='scsi' managed='yes'>
-      <source protocol='iscsi' name='iqn.1992-01.com.example'>
+      <source protocol='iscsi' name='iqn.1992-01.com.example/0'>
         <host name='example.org' port='3260'/>
       </source>
       <address type='drive' controller='0' bus='0' target='0' unit='4'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml
index 37635b4f9..ddf780160 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml
@@ -32,7 +32,7 @@
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <hostdev mode='subsystem' type='scsi' managed='yes'>
-      <source protocol='iscsi' name='iqn.1992-01.com.example'>
+      <source protocol='iscsi' name='iqn.1992-01.com.example/0'>
         <host name='example.org' port='3260'/>
       </source>
       <address type='drive' controller='0' bus='0' target='2' unit='4'/>
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Provide default LUN=0 for iSCSI if not provided
Posted by Peter Krempa 6 years, 7 months ago
On Mon, Sep 11, 2017 at 19:26:18 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1477880
> 
> If the "/#" is missing from the provided iSCSI path, then we need
> to provide the default LUN of /0; otherwise, QEMU will fail to parse
> the URL causing a failure to either create the guest or hotplug
> attach the storage.
> 
> During post parse, for any iSCSI disk or hostdev, scan the source
> path looking for the presence of '/', if found, then we can assume
> the LUN is provided.  If not found, alter the input XML to add the
> "/0".  This will cause the generated XML to have the generated
> value when the domain config is saved after post parse.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> 
>  v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00122.html
> 
>  Alter the patch to make the adjustment during device post parsing rather
>  than command line creation as requested by code review. This will alter
>  the output XML as well as the command line arguments for those entries
>  that don't supply a LUN.  We had a few, so that works out testing wise!
> 
>  src/conf/domain_conf.c                             | 43 ++++++++++++++++++++++
>  .../qemuargv2xml-disk-drive-network-iscsi.xml      |  2 +-
>  .../qemuxml2argv-disk-drive-network-iscsi-lun.args |  2 +-
>  .../qemuxml2argv-disk-drive-network-iscsi.args     |  2 +-
>  .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args       |  2 +-
>  .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args    |  2 +-
>  .../qemuxml2xmlout-disk-drive-network-iscsi.xml    |  2 +-
>  .../qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml      |  2 +-
>  .../qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml   |  2 +-
>  9 files changed, 51 insertions(+), 8 deletions(-)
> 

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list