"Further" clarification (and testing) shows that using a SCSI Fibre Channel NPIV device/lun from a storage pool as a <disk type='volume' device'lun'> will work. So just add that to the allowable options Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1230179 Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d0c1741..cf697f3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1972,8 +1972,10 @@ Using "lun" (<span class="since">since 0.9.10</span>) is only valid when the <code>type</code> is "block" or "network" for <code>protocol='iscsi'</code> or when the <code>type</code> - is "volume" when using an iSCSI source <code>pool</code> - for <code>mode</code> "host". + is "volume" when using an iSCSI storage <code>pool</code> + for <code>mode</code> "host" or as an + <a href="http://wiki.libvirt.org/page/NPIV_in_libvirt">NPIV</a> + virtual Host Bus Adapter (vHBA) using a Fibre Channel storage pool. Configured in this manner, the LUN behaves identically to "disk", except that generic SCSI commands from the guest are accepted and passed through to the physical device. Also note that -- 2.1.0

Rather than provide a somewhat generic error message when the API returns false, allow the caller to supply a "report = true" option in order to cause virReportError's to describe which of the 3 paths that can cause failure. Some callers don't care about what caused the failure, they just want to have a true/false - for those, calling with report = false should be sufficient. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++++++++++--- src/conf/domain_conf.h | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 6 ++---- src/qemu/qemu_command.c | 5 +---- src/qemu/qemu_conf.c | 6 +++--- 6 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5a9a88d..7320d5b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23814,10 +23814,16 @@ virDomainDefFindDevice(virDomainDefPtr def, * Return true if its source is block type, or false otherwise. */ bool -virDomainDiskSourceIsBlockType(virStorageSourcePtr src) +virDomainDiskSourceIsBlockType(virStorageSourcePtr src, + bool report) { - if (!src->path) + if (!src->path) { + if (report) + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("source path not found for device='lun' " + "using type='%d'"), src->type); return false; + } if (src->type == VIR_STORAGE_TYPE_BLOCK) return true; @@ -23833,11 +23839,20 @@ virDomainDiskSourceIsBlockType(virStorageSourcePtr src) * (e.g. set sgio=filtered|unfiltered for it) in libvirt. */ if (src->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && - src->srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT) + src->srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT) { + if (report) + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device='lun' for iSCSI is not " + "supported with mode='direct'.")); return false; + } return true; } + if (report) + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device='lun' is only valid for block " + "type disk source")); return false; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 50750c1..34e1b99 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3126,7 +3126,7 @@ int virDomainDefFindDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev, bool reportError); -bool virDomainDiskSourceIsBlockType(virStorageSourcePtr src) +bool virDomainDiskSourceIsBlockType(virStorageSourcePtr src, bool report) ATTRIBUTE_NONNULL(1); void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def); diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 507d567..e9caa3e 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -382,7 +382,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, VIR_DEBUG("Allowing any disk block devs"); for (i = 0; i < def->ndisks; i++) { - if (!virDomainDiskSourceIsBlockType(def->disks[i]->src)) + if (!virDomainDiskSourceIsBlockType(def->disks[i]->src, false)) continue; if (virCgroupAllowDevicePath(cgroup, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 81bb711..913e007 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4069,11 +4069,9 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - if (!virDomainDiskSourceIsBlockType(def->src)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Can't setup disk for non-block device")); + if (!virDomainDiskSourceIsBlockType(def->src, false)) goto cleanup; - } + src = virDomainDiskGetSource(def); if (src == NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 42906a8..dc2d515 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3472,10 +3472,7 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) virStorageNetProtocolTypeToString(disk->src->protocol)); goto error; } - } else if (!virDomainDiskSourceIsBlockType(disk->src)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device='lun' is only valid for block " - "type disk source")); + } else if (!virDomainDiskSourceIsBlockType(disk->src, true)) { goto error; } if (disk->wwn) { diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 38d4a86..fbe4df3 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1210,7 +1210,7 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, char *key = NULL; int ret = -1; - if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src, false)) return 0; qemuDriverLock(driver); @@ -1355,7 +1355,7 @@ qemuRemoveSharedDisk(virQEMUDriverPtr driver, char *key = NULL; int ret = -1; - if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src, false)) return 0; qemuDriverLock(driver); @@ -1443,7 +1443,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) disk = dev->data.disk; if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || - !virDomainDiskSourceIsBlockType(disk->src)) + !virDomainDiskSourceIsBlockType(disk->src, false)) return 0; path = virDomainDiskGetSource(disk); -- 2.1.0