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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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