[libvirt] [PATCH v2 0/4] Allow creation of vHBA by parent_wwnn/wwpn or fabric_name
v1: http://www.redhat.com/archives/libvir-list/2016-November/msg00866.html Based on ACKs some of the v1 changes were pushed to make this series appear shorter. Changes since v1 - Patch 1 (formerly patch 4) Rather than separate XML elements, use the existing <parent> and just make attributes for wwnn, wwpn, and fabric_wwn. The parent element can now be string text or empty. Added checked for if either wwnn/wwpn provided, then the other is too. - Patch 2 (formerly patch 5) No changes, but no formal ACK either - Patch 3 (formerly patch 10) Rather than READ_WWN macro across two functions, I created a helper function to read and compare the various wwn values (wwnn, wwpn, and fabric_wwn). John Ferlan (4): nodedev: Add the ability to create vHBA by parent wwnn/wwpn or fabric_wwn conf: Add more fchost search fields for storage pool vHBA creation util: Introduce virGetFCHostNameByFabricWWN iscsi: Add parent wwnn/wwpn or fabric capability for createVport docs/schemas/basictypes.rng | 15 ++++ docs/schemas/nodedev.rng | 24 +++++- src/conf/node_device_conf.c | 126 +++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 14 ++++ src/conf/storage_conf.c | 21 +++++- src/conf/storage_conf.h | 3 + src/libvirt_private.syms | 3 + src/node_device/node_device_driver.c | 13 ++++ src/storage/storage_backend_scsi.c | 21 +++++- src/util/virutil.c | 141 ++++++++++++++++++++++++++--------- src/util/virutil.h | 4 + 11 files changed, 346 insertions(+), 39 deletions(-) -- 2.7.4
https://bugzilla.redhat.com/show_bug.cgi?id=1349696 When creating a vHBA, the process is to feed XML to nodeDeviceCreateXML that lists the <parent> scsi_hostX to use to create the vHBA. However, between reboots, it's possible that the <parent> changes its scsi_hostX to scsi_hostY and saved XML to perform the creation will either fail or create a vHBA using the wrong parent. So add the ability to provide "wwnn" and "wwpn" or "fabric_wwn" to the <parent> instead of a name of the scsi_hostN that is the parent. The allowed XML will thus be: <parent>scsi_host3</parent> (current) or <parent wwnn='$WWNN' wwpn='$WWPN'/> or <parent fabric_wwn='$WWNN'/> Using the wwnn/wwpn or fabric_wwn ensures the same 'scsi_hostN' is selected between hardware reconfigs or host reboots. The fabric_wwn Using the wwnn/wwpn pair will provide the most specific search option, while fabric_wwn will at least ensure usage of the same SAN, but maybe not the same scsi_hostN. This patch will add the new fields to the nodedev.rng for input purposes only since the input XML is essentially thrown away, no need to Format the values since they'd already be printed as part of the scsi_host data block. New API virNodeDeviceGetParentHostByWWNs will take the parent "wwnn" and "wwpn" in order to search the list of devices for matching capability data fields wwnn and wwpn. New API virNodeDeviceGetParentHostByFabricWWN will take the parent "fabric_wwn" in order to search the list of devices for matching capability data field fabric_wwn. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/schemas/nodedev.rng | 24 ++++++- src/conf/node_device_conf.c | 126 +++++++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 14 ++++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 13 ++++ 5 files changed, 178 insertions(+), 1 deletion(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 93a88d8..4666c9c 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -16,7 +16,7 @@ <element name="path"><text/></element> </optional> <optional> - <element name="parent"><text/></element> + <ref name="parent"/> </optional> <optional> @@ -31,6 +31,28 @@ </element> </define> + <define name='parent'> + <element name='parent'> + <optional> + <attribute name='wwnn'> + <ref name='wwn'/> + </attribute> + <attribute name='wwpn'> + <ref name='wwn'/> + </attribute> + </optional> + <optional> + <attribute name='fabric_wwn'> + <ref name='wwn'/> + </attribute> + </optional> + <choice> + <text/> + <empty/> + </choice> + </element> + </define> + <define name='capability'> <element name="capability"> <choice> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index b13cb6b..4d3268f 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -97,6 +97,30 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap) } +/* virNodeDeviceFindFCCapDef: + * @dev: Pointer to current device + * + * Search the device object 'caps' array for fc_host capability. + * + * Returns: + * Pointer to the caps or NULL if not found + */ +static virNodeDevCapsDefPtr +virNodeDeviceFindFCCapDef(const virNodeDeviceObj *dev) +{ + virNodeDevCapsDefPtr caps = dev->def->caps; + + while (caps) { + if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST && + (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) + break; + + caps = caps->next; + } + return caps; +} + + /* virNodeDeviceFindVPORTCapDef: * @dev: Pointer to current device * @@ -157,6 +181,46 @@ virNodeDeviceObjPtr virNodeDeviceFindByName(virNodeDeviceObjListPtr devs, static virNodeDeviceObjPtr +virNodeDeviceFindByWWNs(virNodeDeviceObjListPtr devs, + const char *parent_wwnn, + const char *parent_wwpn) +{ + size_t i; + + for (i = 0; i < devs->count; i++) { + virNodeDevCapsDefPtr cap; + virNodeDeviceObjLock(devs->objs[i]); + if ((cap = virNodeDeviceFindFCCapDef(devs->objs[i])) && + STREQ_NULLABLE(cap->data.scsi_host.wwnn, parent_wwnn) && + STREQ_NULLABLE(cap->data.scsi_host.wwpn, parent_wwpn)) + return devs->objs[i]; + virNodeDeviceObjUnlock(devs->objs[i]); + } + + return NULL; +} + + +static virNodeDeviceObjPtr +virNodeDeviceFindByFabricWWN(virNodeDeviceObjListPtr devs, + const char *parent_fabric_wwn) +{ + size_t i; + + for (i = 0; i < devs->count; i++) { + virNodeDevCapsDefPtr cap; + virNodeDeviceObjLock(devs->objs[i]); + if ((cap = virNodeDeviceFindFCCapDef(devs->objs[i])) && + STREQ_NULLABLE(cap->data.scsi_host.fabric_wwn, parent_fabric_wwn)) + return devs->objs[i]; + virNodeDeviceObjUnlock(devs->objs[i]); + } + + return NULL; +} + + +static virNodeDeviceObjPtr virNodeDeviceFindByCap(virNodeDeviceObjListPtr devs, const char *cap) { @@ -182,6 +246,9 @@ void virNodeDeviceDefFree(virNodeDeviceDefPtr def) VIR_FREE(def->name); VIR_FREE(def->parent); + VIR_FREE(def->parent_wwnn); + VIR_FREE(def->parent_wwpn); + VIR_FREE(def->parent_fabric_wwn); VIR_FREE(def->driver); VIR_FREE(def->sysfs_path); VIR_FREE(def->parent_sysfs_path); @@ -1657,6 +1724,16 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, /* Extract device parent, if any */ def->parent = virXPathString("string(./parent[1])", ctxt); + def->parent_wwnn = virXPathString("string(./parent[1]/@wwnn)", ctxt); + def->parent_wwpn = virXPathString("string(./parent[1]/@wwpn)", ctxt); + if ((def->parent_wwnn && !def->parent_wwpn) || + (!def->parent_wwnn && def->parent_wwpn)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("must supply both wwnn and wwpn for parent")); + goto error; + } + def->parent_fabric_wwn = virXPathString("string(./parent[1]/@fabric_wwn)", + ctxt); /* Parse device capabilities */ nodes = NULL; @@ -1852,6 +1929,55 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, int +virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_wwnn, + const char *parent_wwpn, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + int ret; + + if (!(parent = virNodeDeviceFindByWWNs(devs, parent_wwnn, parent_wwpn))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find parent device for '%s'"), + dev_name); + return -1; + } + + ret = virNodeDeviceFindFCParentHost(parent, parent_host); + + virNodeDeviceObjUnlock(parent); + + return ret; +} + + +int +virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_fabric_wwn, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + int ret; + + if (!(parent = virNodeDeviceFindByFabricWWN(devs, parent_fabric_wwn))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find parent device for '%s'"), + dev_name); + return -1; + } + + ret = virNodeDeviceFindFCParentHost(parent, parent_host); + + virNodeDeviceObjUnlock(parent); + + return ret; +} + + +int virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, int *parent_host) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 2b2aed7..1634483 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -200,6 +200,9 @@ struct _virNodeDeviceDef { char *sysfs_path; /* udev name/sysfs path */ char *parent; /* optional parent device name */ char *parent_sysfs_path; /* udev parent name/sysfs path */ + char *parent_wwnn; /* optional parent wwnn */ + char *parent_wwpn; /* optional parent wwpn */ + char *parent_fabric_wwn; /* optional parent fabric_wwn */ char *driver; /* optional driver name */ virNodeDevCapsDefPtr caps; /* optional device capabilities */ }; @@ -273,6 +276,17 @@ int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, const char *parent_name, int *parent_host); +int virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_wwnn, + const char *parent_wwpn, + int *parent_host); + +int virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_fabric_wwn, + int *parent_host); + int virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, int *parent_host); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a928e90..8e4e52d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -702,6 +702,8 @@ virNodeDeviceFindByName; virNodeDeviceFindBySysfsPath; virNodeDeviceFindVportParentHost; virNodeDeviceGetParentHost; +virNodeDeviceGetParentHostByFabricWWN; +virNodeDeviceGetParentHostByWWNs; virNodeDeviceGetWWNs; virNodeDeviceHasCap; virNodeDeviceObjListExport; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 474b713..5238e23 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -590,6 +590,19 @@ nodeDeviceCreateXML(virConnectPtr conn, def->parent, &parent_host) < 0) goto cleanup; + } else if (def->parent_wwnn && def->parent_wwpn) { + if (virNodeDeviceGetParentHostByWWNs(&driver->devs, + def->name, + def->parent_wwnn, + def->parent_wwpn, + &parent_host) < 0) + goto cleanup; + } else if (def->parent_fabric_wwn) { + if (virNodeDeviceGetParentHostByFabricWWN(&driver->devs, + def->name, + def->parent_fabric_wwn, + &parent_host) < 0) + goto cleanup; } else { /* Try to find a vport capable scsi_host when no parent supplied */ if (virNodeDeviceFindVportParentHost(&driver->devs, &parent_host) < 0) -- 2.7.4
On Wed, Jan 04, 2017 at 05:48:35PM -0500, John Ferlan wrote:
@@ -31,6 +31,28 @@ </element> </define>
+ <define name='parent'> + <element name='parent'> + <optional> + <attribute name='wwnn'> + <ref name='wwn'/> + </attribute> + <attribute name='wwpn'> + <ref name='wwn'/> + </attribute> + </optional> + <optional> + <attribute name='fabric_wwn'> + <ref name='wwn'/> + </attribute> + </optional> + <choice> + <text/> + <empty/> + </choice> + </element> + </define> +
Could this be tightened as: <choice> <group> wwnn wwpn <empty/> </group> <group> fabric_wwn <empty/> </group> <text/> </choice> ? Jan
Add new fields to the fchost structure to allow creation of a vHBA via the storage pool when a parent_wwnn/parent_wwpn or parent_fabric_wwn is supplied in the storage pool XML. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/schemas/basictypes.rng | 15 +++++++++++++++ src/conf/storage_conf.c | 21 +++++++++++++++++++-- src/conf/storage_conf.h | 3 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1b4f980..cc560e6 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -427,6 +427,21 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name='parent_wwnn'> + <ref name='wwn'/> + </attribute> + </optional> + <optional> + <attribute name='parent_wwpn'> + <ref name='wwn'/> + </attribute> + </optional> + <optional> + <attribute name='parent_fabric_wwn'> + <ref name='wwn'/> + </attribute> + </optional> <attribute name='wwnn'> <ref name='wwn'/> </attribute> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 71ea0c9..c53f080 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -335,6 +335,9 @@ virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter) VIR_FREE(adapter->data.fchost.wwnn); VIR_FREE(adapter->data.fchost.wwpn); VIR_FREE(adapter->data.fchost.parent); + VIR_FREE(adapter->data.fchost.parent_wwnn); + VIR_FREE(adapter->data.fchost.parent_wwpn); + VIR_FREE(adapter->data.fchost.parent_fabric_wwn); } else if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { VIR_FREE(adapter->data.scsi_host.name); @@ -591,10 +594,17 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, } } - source->adapter.data.fchost.wwnn = - virXPathString("string(./adapter/@wwnn)", ctxt); + source->adapter.data.fchost.parent_wwnn = + virXPathString("string(./adapter/@parent_wwnn)", ctxt); + source->adapter.data.fchost.parent_wwpn = + virXPathString("string(./adapter/@parent_wwpn)", ctxt); + source->adapter.data.fchost.parent_fabric_wwn = + virXPathString("string(./adapter/@parent_fabric_wwn)", ctxt); + source->adapter.data.fchost.wwpn = virXPathString("string(./adapter/@wwpn)", ctxt); + source->adapter.data.fchost.wwnn = + virXPathString("string(./adapter/@wwnn)", ctxt); } else if (source->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { @@ -1100,6 +1110,13 @@ virStoragePoolSourceFormat(virBufferPtr buf, if (src->adapter.data.fchost.managed) virBufferAsprintf(buf, " managed='%s'", virTristateBoolTypeToString(src->adapter.data.fchost.managed)); + virBufferEscapeString(buf, " parent_wwnn='%s'", + src->adapter.data.fchost.parent_wwnn); + virBufferEscapeString(buf, " parent_wwpn='%s'", + src->adapter.data.fchost.parent_wwpn); + virBufferEscapeString(buf, " parent_fabric_wwn='%s'", + src->adapter.data.fchost.parent_fabric_wwn); + virBufferAsprintf(buf, " wwnn='%s' wwpn='%s'/>\n", src->adapter.data.fchost.wwnn, src->adapter.data.fchost.wwpn); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 185ae5e..b35471d 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -193,6 +193,9 @@ struct _virStoragePoolSourceAdapter { } scsi_host; struct { char *parent; + char *parent_wwnn; + char *parent_wwpn; + char *parent_fabric_wwn; char *wwnn; char *wwpn; int managed; /* enum virTristateSwitch */ -- 2.7.4
Create a utility routine in order to read the scsi_host fabric_name files looking for a match to a passed fabric_name Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virutil.c | 141 +++++++++++++++++++++++++++++++++++------------ src/util/virutil.h | 4 ++ 3 files changed, 111 insertions(+), 35 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8e4e52d..c5b9cee 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2667,6 +2667,7 @@ virGetDeviceID; virGetDeviceUnprivSGIO; virGetEnvAllowSUID; virGetEnvBlockSUID; +virGetFCHostNameByFabricWWN; virGetFCHostNameByWWN; virGetGroupID; virGetGroupList; diff --git a/src/util/virutil.c b/src/util/virutil.c index 701c382..da836b4 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2166,6 +2166,60 @@ virManageVport(const int parent_host, return ret; } + +/* virReadCompareWWN + * @prefix: path to the wwn file + * @d_name: name of the current directory + * @f_name: file name to read + * + * Read/compare the on-disk file with the passed wwn value. + * + * Returns: + * -1 : Error + * 0 : No match + * 1 : Match + */ +static int +virReadCompareWWN(const char *prefix, + const char *d_name, + const char *f_name, + const char *wwn) +{ + char *path; + char *buf = NULL; + char *p; + int ret = -1; + + if (virAsprintf(&path, "%s/%s/%s", prefix, d_name, f_name) < 0) + return -1; + + if (!virFileExists(path)) { + ret = 0; + goto cleanup; + } + + if (virFileReadAll(path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + if (STRPREFIX(buf, "0x")) + p = buf + strlen("0x"); + else + p = buf; + + if (STRNEQ(wwn, p)) + ret = 0; + else + ret = 1; + + cleanup: + VIR_FREE(path); + VIR_FREE(buf); + + return ret; +} + /* virGetFCHostNameByWWN: * * Iterate over the sysfs tree to get FC host name (e.g. host5) @@ -2182,56 +2236,77 @@ virGetFCHostNameByWWN(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; struct dirent *entry = NULL; DIR *dir = NULL; - char *wwnn_path = NULL; - char *wwpn_path = NULL; - char *wwnn_buf = NULL; - char *wwpn_buf = NULL; - char *p; char *ret = NULL; if (virDirOpen(&dir, prefix) < 0) return NULL; -# define READ_WWN(wwn_path, buf) \ - do { \ - if (virFileReadAll(wwn_path, 1024, &buf) < 0) \ - goto cleanup; \ - if ((p = strchr(buf, '\n'))) \ - *p = '\0'; \ - if (STRPREFIX(buf, "0x")) \ - p = buf + strlen("0x"); \ - else \ - p = buf; \ - } while (0) - while (virDirRead(dir, &entry, prefix) > 0) { - VIR_FREE(wwnn_buf); - VIR_FREE(wwnn_path); - VIR_FREE(wwpn_buf); - VIR_FREE(wwpn_path); + int rc; - if (virAsprintf(&wwnn_path, "%s/%s/node_name", prefix, - entry->d_name) < 0) + if ((rc = virReadCompareWWN(prefix, entry->d_name, + "node_name", wwnn)) < 0) goto cleanup; - if (!virFileExists(wwnn_path)) + if (rc == 0) continue; - READ_WWN(wwnn_path, wwnn_buf); + if ((rc = virReadCompareWWN(prefix, entry->d_name, + "port_name", wwpn)) < 0) + goto cleanup; - if (STRNEQ(wwnn, p)) + if (rc == 0) continue; - if (virAsprintf(&wwpn_path, "%s/%s/port_name", prefix, + ignore_value(VIR_STRDUP(ret, entry->d_name)); + break; + } + + cleanup: + VIR_DIR_CLOSE(dir); + return ret; +} + +/* virGetFCHostNameByFabricWWN: + * + * Iterate over the sysfs tree to get FC host name (e.g. host5) + * by the provided "fabric_wwn". This would find a host on a SAN. + * + * Returns the FC host name which must be freed by the caller, + * or NULL on failure. + */ +char * +virGetFCHostNameByFabricWWN(const char *sysfs_prefix, + const char *fabric_wwn) +{ + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; + struct dirent *entry = NULL; + DIR *dir = NULL; + char *vport_create_path = NULL; + char *ret = NULL; + + if (virDirOpen(&dir, prefix) < 0) + return NULL; + + while (virDirRead(dir, &entry, prefix) > 0) { + int rc; + + VIR_FREE(vport_create_path); + + /* Existing vHBA's will have the same fabric_name, but won't + * have the vport_create file - so we check for both */ + if (virAsprintf(&vport_create_path, "%s/%s/vport_create", prefix, entry->d_name) < 0) goto cleanup; - if (!virFileExists(wwpn_path)) + if (!virFileExists(vport_create_path)) continue; - READ_WWN(wwpn_path, wwpn_buf); + if ((rc = virReadCompareWWN(prefix, entry->d_name, + "fabric_name", fabric_wwn)) < 0) + goto cleanup; - if (STRNEQ(wwpn, p)) + if (rc == 0) continue; ignore_value(VIR_STRDUP(ret, entry->d_name)); @@ -2239,12 +2314,8 @@ virGetFCHostNameByWWN(const char *sysfs_prefix, } cleanup: -# undef READ_WWN VIR_DIR_CLOSE(dir); - VIR_FREE(wwnn_path); - VIR_FREE(wwpn_path); - VIR_FREE(wwnn_buf); - VIR_FREE(wwpn_buf); + VIR_FREE(vport_create_path); return ret; } diff --git a/src/util/virutil.h b/src/util/virutil.h index 8c0d83c..3fbd7b0 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -206,6 +206,10 @@ char *virGetFCHostNameByWWN(const char *sysfs_prefix, const char *wwpn) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +char *virGetFCHostNameByFabricWWN(const char *sysfs_prefix, + const char *fabric_wwn) + ATTRIBUTE_NONNULL(2); + char *virFindFCHostCapableVport(const char *sysfs_prefix); int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); -- 2.7.4
https://bugzilla.redhat.com/show_bug.cgi?id=1349696 As it turns out using only the 'parent' to achieve the goal of a consistent vHBA parent has issues with reboots where the scsi_hostX parent could change to scsi_hostY causing either failure to create the vHBA or usage of the wrong HBA for our vHBA. Thus add the ability to search for the "parent" by the parent wwnn/ wwpn values or just a fabric_name if someone only cares to ensure usage of the same SAN for the vHBA. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 25a8dea..be02f6e 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -695,6 +695,7 @@ createVport(virConnectPtr conn, unsigned int parent_host; char *name = NULL; char *parent_hoststr = NULL; + bool skip_capable_check = false; virStoragePoolFCRefreshInfoPtr cbdata = NULL; virThread thread; int ret = -1; @@ -725,6 +726,23 @@ createVport(virConnectPtr conn, if (adapter->data.fchost.parent) { if (VIR_STRDUP(parent_hoststr, adapter->data.fchost.parent) < 0) goto cleanup; + } else if (adapter->data.fchost.parent_wwnn && + adapter->data.fchost.parent_wwpn) { + if (!(parent_hoststr = + virGetFCHostNameByWWN(NULL, adapter->data.fchost.parent_wwnn, + adapter->data.fchost.parent_wwpn))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("cannot find parent using provided wwnn/wwpn")); + goto cleanup; + } + } else if (adapter->data.fchost.parent_fabric_wwn) { + if (!(parent_hoststr = + virGetFCHostNameByFabricWWN(NULL, + adapter->data.fchost.parent_fabric_wwn))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("cannot find parent using provided fabric_wwn")); + goto cleanup; + } } else { if (!(parent_hoststr = virFindFCHostCapableVport(NULL))) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -732,6 +750,7 @@ createVport(virConnectPtr conn, "cannot find one on this host")); goto cleanup; } + skip_capable_check = true; } if (virGetSCSIHostNumber(parent_hoststr, &parent_host) < 0) @@ -745,7 +764,7 @@ createVport(virConnectPtr conn, * parent. Besides we have a way to determine the parent based on * the 'name' field. */ - if (adapter->data.fchost.parent && !virIsCapableFCHost(NULL, parent_host)) { + if (!skip_capable_check && !virIsCapableFCHost(NULL, parent_host)) { virReportError(VIR_ERR_XML_ERROR, _("parent '%s' specified for vHBA is not vport capable"), parent_hoststr); -- 2.7.4
On Wed, Jan 04, 2017 at 05:48:34PM -0500, John Ferlan wrote:
v1: http://www.redhat.com/archives/libvir-list/2016-November/msg00866.html
Based on ACKs some of the v1 changes were pushed to make this series appear shorter.
Changes since v1
- Patch 1 (formerly patch 4)
Rather than separate XML elements, use the existing <parent> and just make attributes for wwnn, wwpn, and fabric_wwn. The parent element can now be string text or empty.
Added checked for if either wwnn/wwpn provided, then the other is too.
- Patch 2 (formerly patch 5)
No changes, but no formal ACK either
- Patch 3 (formerly patch 10)
Rather than READ_WWN macro across two functions, I created a helper function to read and compare the various wwn values (wwnn, wwpn, and fabric_wwn).
John Ferlan (4): nodedev: Add the ability to create vHBA by parent wwnn/wwpn or fabric_wwn conf: Add more fchost search fields for storage pool vHBA creation util: Introduce virGetFCHostNameByFabricWWN iscsi: Add parent wwnn/wwpn or fabric capability for createVport
docs/schemas/basictypes.rng | 15 ++++ docs/schemas/nodedev.rng | 24 +++++- src/conf/node_device_conf.c | 126 +++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 14 ++++ src/conf/storage_conf.c | 21 +++++- src/conf/storage_conf.h | 3 + src/libvirt_private.syms | 3 + src/node_device/node_device_driver.c | 13 ++++ src/storage/storage_backend_scsi.c | 21 +++++- src/util/virutil.c | 141 ++++++++++++++++++++++++++--------- src/util/virutil.h | 4 + 11 files changed, 346 insertions(+), 39 deletions(-)
ACK series Jan
On 01/06/2017 08:56 AM, Ján Tomko wrote:
On Wed, Jan 04, 2017 at 05:48:34PM -0500, John Ferlan wrote:
v1: http://www.redhat.com/archives/libvir-list/2016-November/msg00866.html
Based on ACKs some of the v1 changes were pushed to make this series appear shorter.
Changes since v1
- Patch 1 (formerly patch 4)
Rather than separate XML elements, use the existing <parent> and just make attributes for wwnn, wwpn, and fabric_wwn. The parent element can now be string text or empty.
Added checked for if either wwnn/wwpn provided, then the other is too.
- Patch 2 (formerly patch 5)
No changes, but no formal ACK either
- Patch 3 (formerly patch 10)
Rather than READ_WWN macro across two functions, I created a helper function to read and compare the various wwn values (wwnn, wwpn, and fabric_wwn).
John Ferlan (4): nodedev: Add the ability to create vHBA by parent wwnn/wwpn or fabric_wwn conf: Add more fchost search fields for storage pool vHBA creation util: Introduce virGetFCHostNameByFabricWWN iscsi: Add parent wwnn/wwpn or fabric capability for createVport
docs/schemas/basictypes.rng | 15 ++++ docs/schemas/nodedev.rng | 24 +++++- src/conf/node_device_conf.c | 126 +++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 14 ++++ src/conf/storage_conf.c | 21 +++++- src/conf/storage_conf.h | 3 + src/libvirt_private.syms | 3 + src/node_device/node_device_driver.c | 13 ++++ src/storage/storage_backend_scsi.c | 21 +++++- src/util/virutil.c | 141 ++++++++++++++++++++++++++--------- src/util/virutil.h | 4 + 11 files changed, 346 insertions(+), 39 deletions(-)
ACK series
Adjusted patch 1 to use the suggested RNG and pushed - Thanks John
participants (2)
-
John Ferlan -
Ján Tomko