Add a new test to fchosttest in order to test creation of our vHBA via the Storage Pool logic. Unlike the real code, we cannot yet use the virVHBA* API's because they (currently) traverse the file system in order to get the parent vport capable scsi_host. Besides there's no "real" NPIV device here - so we have to take some liberties, at least for now. Instead, we'll follow the node device tests partially in order to create and destroy the vHBA with the test node devices. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/test/test_driver.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-- tests/fchosttest.c | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 61c82b9..a9bb5a6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4366,6 +4366,34 @@ testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, } +static virNodeDeviceObjPtr +testNodeDeviceMockCreateVport(testDriverPtr driver, + const char *wwnn, + const char *wwpn); +static int +testCreateVport(testDriverPtr driver, + const char *wwnn, + const char *wwpn) +{ + virNodeDeviceObjPtr obj = NULL; + /* The storage_backend_scsi createVport() will use the input adapter + * fields parent name, parent_wwnn/parent_wwpn, or parent_fabric_wwn + * in order to determine whether the provided parent can be used to + * create a vHBA or will find "an available vport capable" to create + * a vHBA. In order to do this, it uses the virVHBA* API's which traverse + * the sysfs looking at various fields (rather than going via nodedev). + * + * Since the test environ doesn't have the sysfs for the storage pool + * test, at least for now use the node device test infrastructure to + * create the vHBA. In the long run the result is the same. */ + if (!(obj = testNodeDeviceMockCreateVport(driver, wwnn, wwpn))) + return -1; + virNodeDeviceObjUnlock(obj); + + return 0; +} + + static virStoragePoolPtr testStoragePoolCreateXML(virConnectPtr conn, const char *xml, @@ -4396,6 +4424,21 @@ testStoragePoolCreateXML(virConnectPtr conn, goto cleanup; def = NULL; + if (pool->def->source.adapter.type == + VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + /* In the real code, we'd call virVHBAManageVport followed by + * find_new_device, but we cannot do that here since we're not + * mocking udev. The mock routine will copy an existing vHBA and + * rename a few fields to mock that. */ + if (testCreateVport(privconn, + pool->def->source.adapter.data.fchost.wwnn, + pool->def->source.adapter.data.fchost.wwpn) < 0) { + virStoragePoolObjRemove(&privconn->pools, pool); + pool = NULL; + goto cleanup; + } + } + if (testStoragePoolObjSetDefaults(pool) == -1) { virStoragePoolObjRemove(&privconn->pools, pool); pool = NULL; @@ -4406,7 +4449,6 @@ testStoragePoolCreateXML(virConnectPtr conn, event = virStoragePoolEventLifecycleNew(pool->def->name, pool->def->uuid, VIR_STORAGE_POOL_EVENT_STARTED, 0); - ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, NULL, NULL); @@ -4540,6 +4582,44 @@ testStoragePoolBuild(virStoragePoolPtr pool, static int +testDestroyVport(testDriverPtr privconn, + const char *wwnn ATTRIBUTE_UNUSED, + const char *wwpn ATTRIBUTE_UNUSED) +{ + int ret = -1; + virNodeDeviceObjPtr obj = NULL; + virObjectEventPtr event = NULL; + + /* NB: Cannot use virVHBAGetHostByWWN (yet) like the storage_backend_scsi + * deleteVport() helper since that traverses the file system looking for + * the wwnn/wwpn. So our choice short term is to cheat and use the name + * (scsi_host12) we know was created. + * + * Reaching across the boundaries of space and time into the + * Node Device in order to remove */ + if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) { + virReportError(VIR_ERR_NO_NODE_DEVICE, "%s", + _("no node device with matching name 'scsi_host12'")); + goto cleanup; + } + + event = virNodeDeviceEventLifecycleNew("scsi_host12", + VIR_NODE_DEVICE_EVENT_DELETED, + 0); + + virNodeDeviceObjRemove(&privconn->devs, &obj); + + ret = 0; + + cleanup: + if (obj) + virNodeDeviceObjUnlock(obj); + testObjectEventQueue(privconn, event); + return ret; +} + + +static int testStoragePoolDestroy(virStoragePoolPtr pool) { testDriverPtr privconn = pool->conn->privateData; @@ -4563,7 +4643,17 @@ testStoragePoolDestroy(virStoragePoolPtr pool) } privpool->active = 0; - event = virStoragePoolEventLifecycleNew(privpool->def->name, privpool->def->uuid, + + if (privpool->def->source.adapter.type == + VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (testDestroyVport(privconn, + privpool->def->source.adapter.data.fchost.wwnn, + privpool->def->source.adapter.data.fchost.wwpn) < 0) + goto cleanup; + } + + event = virStoragePoolEventLifecycleNew(privpool->def->name, + privpool->def->uuid, VIR_STORAGE_POOL_EVENT_STOPPED, 0); diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 51fdcbd..dc6b9af 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -75,6 +75,19 @@ static const char test10_xml[] = " </capability>" "</device>"; +/* virStoragePoolCreateXML using parent='%s' to find the vport capable HBA */ +static const char test11_xml[] = +"<pool type='scsi'>" +" <name>vhba_pool</name>" +" <source>" +" <adapter type='fc_host' parent='scsi_host1' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>" +" </source>" +" <target>" +" <path>/dev/disk/by-path</path>" +" </target>" +"</pool>"; + + /* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) @@ -275,6 +288,54 @@ manageVHBAByNodeDevice(const void *data) } +/* Test manageVHBAByStoragePool + * - Test both virStoragePoolCreateXML and virStoragePoolDestroy + * - Create a storage pool vHBA allowing usage of various different + * methods based on the input data/xml argument. + * - Be sure that it's possible to destroy the storage pool as well. + */ +static int +manageVHBAByStoragePool(const void *data) +{ + const char *expect_hostname = "scsi_host12"; + virConnectPtr conn = NULL; + virStoragePoolPtr pool = NULL; + virNodeDevicePtr dev = NULL; + int ret = -1; + const char *vhba = data; + + if (!(conn = virConnectOpen("test:///default"))) + return -1; + + if (!(pool = virStoragePoolCreateXML(conn, vhba, 0))) + goto cleanup; + + if (!(dev = virNodeDeviceLookupByName(conn, expect_hostname))) { + VIR_DEBUG("Failed to find expected_hostname '%s'", expect_hostname); + ignore_value(virStoragePoolDestroy(pool)); + goto cleanup; + } + + if (virStoragePoolDestroy(pool) < 0) + goto cleanup; + + if ((dev = virNodeDeviceLookupByName(conn, expect_hostname))) { + VIR_DEBUG("Found expected_hostname '%s' after destroy", + expect_hostname); + goto cleanup; + } + + ret = 0; + + cleanup: + if (pool) + virStoragePoolFree(pool); + if (conn) + virConnectClose(conn); + return ret; +} + + static int mymain(void) { @@ -310,6 +371,9 @@ mymain(void) if (virTestRun("manageVHBAByNodeDevice-parent-fabric-wwn", manageVHBAByNodeDevice, test10_xml) < 0) ret = -1; + if (virTestRun("manageVHBAByStoragePool-by-parent", manageVHBAByStoragePool, + test11_xml) < 0) + ret = -1; cleanup: VIR_FREE(fchost_prefix); -- 2.9.3

Introduce src/conf/storage_adapter_conf.{c,h}. Separate out the <adapter> parsing/formatting and management into a separate conf module. Move the virStoragePoolSourceAdapter from storage_conf.h and rename to virStorageAdapter. This includes creating 'scsi_host' and 'fchost' specific structs as virStorageAdapterSCSIHost and virStorageAdapterFCHost which allows code using the structs to use the typedefs rather than needing to take the lengthy union paths. Modify code in the various places which formerly used the pool structure to use the new model. This includes some changes that will use the Ptr rather than just the struct (try to shorten the number of times one has to type adapter.data.fchost or adapter.data.scsi_host as well as [pool->]def->source.adapter. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/storage_adapter_conf.c | 291 ++++++++++++++++++++++++++++++++ src/conf/storage_adapter_conf.h | 89 ++++++++++ src/conf/storage_conf.c | 333 +++++++++---------------------------- src/conf/storage_conf.h | 35 +--- src/libvirt_private.syms | 16 +- src/phyp/phyp_driver.c | 3 +- src/storage/storage_backend_scsi.c | 129 +++++++------- src/test/test_driver.c | 5 +- 10 files changed, 538 insertions(+), 365 deletions(-) create mode 100644 src/conf/storage_adapter_conf.c create mode 100644 src/conf/storage_adapter_conf.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 7c7f530..0320918 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -40,6 +40,7 @@ src/conf/object_event.c src/conf/secret_conf.c src/conf/snapshot_conf.c src/conf/storage_conf.c +src/conf/storage_adapter_conf.c src/conf/virchrdev.c src/conf/virdomainobjlist.c src/conf/virnodedeviceobj.c diff --git a/src/Makefile.am b/src/Makefile.am index 7d42eac..74bcab3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -377,6 +377,7 @@ NWFILTER_CONF_SOURCES = \ # Storage driver generic impl APIs STORAGE_CONF_SOURCES = \ + conf/storage_adapter_conf.h conf/storage_adapter_conf.c \ conf/storage_conf.h conf/storage_conf.c # Interface driver generic impl APIs diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c new file mode 100644 index 0000000..9ee18a2 --- /dev/null +++ b/src/conf/storage_adapter_conf.c @@ -0,0 +1,291 @@ +/* + * storage_adapter_conf.c: helpers to handle storage pool adapter manipulation + * (derived from storage_conf.c) + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "storage_conf.h" + +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virstring.h" +#include "virutil.h" +#include "virxml.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +VIR_LOG_INIT("conf.storage_adapter_conf"); + +VIR_ENUM_IMPL(virStorageAdapter, + VIR_STORAGE_ADAPTER_TYPE_LAST, + "default", "scsi_host", "fc_host") + + +int +virStorageAdapterVHBAParseXML(xmlNodePtr node, + virStorageAdapterFCHostPtr fchost) +{ + char *managed = NULL; + + fchost->parent = virXMLPropString(node, "parent"); + + if ((managed = virXMLPropString(node, "managed"))) { + if ((fchost->managed = + virTristateBoolTypeFromString(managed)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown fc_host managed setting '%s'"), + managed); + VIR_FREE(managed); + return -1; + } + } + + fchost->parent_wwnn = virXMLPropString(node, "parent_wwnn"); + fchost->parent_wwpn = virXMLPropString(node, "parent_wwpn"); + fchost->parent_fabric_wwn = virXMLPropString(node, "parent_fabric_wwn"); + fchost->wwpn = virXMLPropString(node, "wwpn"); + fchost->wwnn = virXMLPropString(node, "wwnn"); + + VIR_FREE(managed); + return 0; +} + + +int +virStorageAdapterParseXML(virStorageAdapterPtr adapter, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + int ret = -1; + xmlNodePtr relnode = ctxt->node; + char *adapter_type = NULL; + virStorageAdapterSCSIHostPtr scsi_host; + + ctxt->node = node; + + if ((adapter_type = virXMLPropString(node, "type"))) { + if ((adapter->type = + virStorageAdapterTypeFromString(adapter_type)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown pool adapter type '%s'"), + adapter_type); + goto cleanup; + } + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + if (virStorageAdapterVHBAParseXML(node, &adapter->data.fchost) < 0) + goto cleanup; + } + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { + scsi_host = &adapter->data.scsi_host; + + scsi_host->name = virXMLPropString(node, "name"); + if (virXPathNode("./parentaddr", ctxt)) { + xmlNodePtr addrnode = virXPathNode("./parentaddr/address", + ctxt); + virPCIDeviceAddressPtr addr = &scsi_host->parentaddr; + if (!addrnode) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing scsi_host PCI address element")); + goto cleanup; + } + scsi_host->has_parent = true; + if (virPCIDeviceAddressParseXML(addrnode, addr) < 0) + goto cleanup; + if ((virXPathInt("string(./parentaddr/@unique_id)", + ctxt, + &scsi_host->unique_id) < 0) || + (scsi_host->unique_id < 0)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing or invalid scsi adapter " + "'unique_id' value")); + goto cleanup; + } + } + } + } else { + char *wwnn = virXMLPropString(node, "wwnn"); + char *wwpn = virXMLPropString(node, "wwpn"); + char *parent = virXMLPropString(node, "parent"); + + /* "type" was not specified in the XML, so we must verify that + * "wwnn", "wwpn", "parent", or "parentaddr" are also not in the + * XML. If any are found, then we cannot just use "name" alone". + */ + if (wwnn || wwpn || parent) { + VIR_FREE(wwnn); + VIR_FREE(wwpn); + VIR_FREE(parent); + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Use of 'wwnn', 'wwpn', and 'parent' attributes " + "requires use of the adapter 'type'")); + goto cleanup; + } + + if (virXPathNode("./parentaddr", ctxt)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Use of 'parent' element requires use " + "of the adapter 'type'")); + goto cleanup; + } + + /* To keep back-compat, 'type' is not required to specify + * for scsi_host adapter. + */ + if ((adapter->data.scsi_host.name = virXMLPropString(node, "name"))) + adapter->type = VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST; + } + + ret = 0; + + cleanup: + ctxt->node = relnode; + VIR_FREE(adapter_type); + return ret; +} + + +void +virStorageAdapterVHBAFormat(virBufferPtr buf, + virStorageAdapterFCHostPtr fchost) +{ + virBufferEscapeString(buf, " parent='%s'", fchost->parent); + virBufferEscapeString(buf, " parent_wwnn='%s'", fchost->parent_wwnn); + virBufferEscapeString(buf, " parent_wwpn='%s'", fchost->parent_wwpn); + virBufferEscapeString(buf, " parent_fabric_wwn='%s'", + fchost->parent_fabric_wwn); + if (fchost->managed != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, " managed='%s'", + virTristateBoolTypeToString(fchost->managed)); + + virBufferAsprintf(buf, " wwnn='%s' wwpn='%s'/>\n", + fchost->wwnn, fchost->wwpn); +} + + +void +virStorageAdapterFormat(virBufferPtr buf, + virStorageAdapterPtr adapter) +{ + virBufferAsprintf(buf, "<adapter type='%s'", + virStorageAdapterTypeToString(adapter->type)); + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + virStorageAdapterVHBAFormat(buf, &adapter->data.fchost); + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { + virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host; + + if (scsi_host->name) { + virBufferAsprintf(buf, " name='%s'/>\n", scsi_host->name); + } else { + virPCIDeviceAddress addr; + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<parentaddr unique_id='%d'>\n", + scsi_host->unique_id); + virBufferAdjustIndent(buf, 2); + addr = scsi_host->parentaddr; + ignore_value(virPCIDeviceAddressFormat(buf, addr, false)); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</parentaddr>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</adapter>\n"); + } + } +} + + +void +virStorageAdapterVHBAClear(virStorageAdapterFCHostPtr fchost) +{ + VIR_FREE(fchost->parent); + VIR_FREE(fchost->parent_wwnn); + VIR_FREE(fchost->parent_wwpn); + VIR_FREE(fchost->parent_fabric_wwn); + VIR_FREE(fchost->wwnn); + VIR_FREE(fchost->wwpn); +} + + +void +virStorageAdapterClear(virStorageAdapterPtr adapter) +{ + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + virStorageAdapterVHBAClear(&adapter->data.fchost); + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) + VIR_FREE(adapter->data.scsi_host.name); +} + + +int +virStorageAdapterVHBAParseValidate(virStorageAdapterFCHostPtr fchost) +{ + if (!fchost->wwnn || !fchost->wwpn) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'wwnn' and 'wwpn' must be specified for " + "a VHBA adapter")); + return -1; + } + + if (!virValidateWWN(fchost->wwnn) || !virValidateWWN(fchost->wwpn)) + return -1; + + if (fchost->parent_wwnn && !virValidateWWN(fchost->parent_wwnn)) + return -1; + + if (fchost->parent_wwpn && !virValidateWWN(fchost->parent_wwpn)) + return -1; + + if (fchost->parent_fabric_wwn && + !virValidateWWN(fchost->parent_fabric_wwn)) + return -1; + + return 0; +} + + +int +virStorageAdapterParseValidate(virStorageAdapterPtr adapter) +{ + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + return virStorageAdapterVHBAParseValidate(&adapter->data.fchost); + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { + virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host; + + if (!scsi_host->name && !scsi_host->has_parent) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Either 'name' or 'parent' must be specified " + "for the 'scsi_host' adapter")); + return -1; + } + + if (scsi_host->name && scsi_host->has_parent) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Both 'name' and 'parent' cannot be specified " + "for the 'scsi_host' adapter")); + return -1; + } + } + + return 0; +} diff --git a/src/conf/storage_adapter_conf.h b/src/conf/storage_adapter_conf.h new file mode 100644 index 0000000..d566fc4 --- /dev/null +++ b/src/conf/storage_adapter_conf.h @@ -0,0 +1,89 @@ +/* + * storage_adapter_conf.h: helpers to handle storage pool adapter manipulation + * (derived from storage_conf.h) + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_STORAGE_ADAPTER_CONF_H__ +# define __VIR_STORAGE_ADAPTER_CONF_H__ + +# include "virpci.h" +# include "virxml.h" + + +typedef enum { + VIR_STORAGE_ADAPTER_TYPE_DEFAULT = 0, + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST, + VIR_STORAGE_ADAPTER_TYPE_FC_HOST, + + VIR_STORAGE_ADAPTER_TYPE_LAST, +} virStorageAdapterType; +VIR_ENUM_DECL(virStorageAdapter) + +typedef struct _virStorageAdapterSCSIHost virStorageAdapterSCSIHost; +typedef virStorageAdapterSCSIHost *virStorageAdapterSCSIHostPtr; +struct _virStorageAdapterSCSIHost { + char *name; + virPCIDeviceAddress parentaddr; /* host address */ + int unique_id; + bool has_parent; +}; + +typedef struct _virStorageAdapterFCHost virStorageAdapterFCHost; +typedef virStorageAdapterFCHost *virStorageAdapterFCHostPtr; +struct _virStorageAdapterFCHost { + char *parent; + char *parent_wwnn; + char *parent_wwpn; + char *parent_fabric_wwn; + char *wwnn; + char *wwpn; + int managed; /* enum virTristateSwitch */ +}; + +typedef struct _virStorageAdapter virStorageAdapter; +typedef virStorageAdapter *virStorageAdapterPtr; +struct _virStorageAdapter { + int type; /* virStorageAdapterType */ + + union { + virStorageAdapterSCSIHost scsi_host; + virStorageAdapterFCHost fchost; + } data; +}; + + +int virStorageAdapterVHBAParseXML(xmlNodePtr node, + virStorageAdapterFCHostPtr fchost); + +int virStorageAdapterParseXML(virStorageAdapterPtr adapter, + xmlNodePtr node, + xmlXPathContextPtr ctxt); + +void virStorageAdapterVHBAFormat(virBufferPtr buf, + virStorageAdapterFCHostPtr fchost); + +void virStorageAdapterFormat(virBufferPtr buf, virStorageAdapterPtr adapter); + +void virStorageAdapterVHBAClear(virStorageAdapterFCHostPtr fchost); + +void virStorageAdapterClear(virStorageAdapterPtr adapter); + +int virStorageAdapterVHBAParseValidate(virStorageAdapterFCHostPtr fchost); + +int virStorageAdapterParseValidate(virStorageAdapterPtr adapter); + +#endif /* __VIR_STORAGE_ADAPTER_CONF_H__ */ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a52eeba..ffc5b5e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -100,10 +100,6 @@ VIR_ENUM_IMPL(virStoragePartedFs, "ext2", "ext2", "extended") -VIR_ENUM_IMPL(virStoragePoolSourceAdapter, - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST, - "default", "scsi_host", "fc_host") - typedef const char *(*virStorageVolFormatToString)(int format); typedef int (*virStorageVolFormatFromString)(const char *format); @@ -342,21 +338,6 @@ virStorageVolDefFree(virStorageVolDefPtr def) VIR_FREE(def); } -static void -virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter) -{ - if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - 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); - } -} void virStoragePoolSourceDeviceClear(virStoragePoolSourceDevicePtr dev) @@ -382,7 +363,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source) VIR_FREE(source->devices); VIR_FREE(source->dir); VIR_FREE(source->name); - virStoragePoolSourceAdapterClear(&source->adapter); + virStorageAdapterClear(&source->adapter); VIR_FREE(source->initiator.iqn); virStorageAuthDefFree(source->auth); VIR_FREE(source->vendor); @@ -462,6 +443,7 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, } } + static int virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, virStoragePoolSourcePtr source, @@ -470,14 +452,13 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, { int ret = -1; xmlNodePtr relnode, authnode, *nodeset = NULL; + xmlNodePtr adapternode; int nsource; size_t i; virStoragePoolOptionsPtr options; virStorageAuthDefPtr authdef = NULL; char *name = NULL; char *port = NULL; - char *adapter_type = NULL; - char *managed = NULL; int n; relnode = ctxt->node; @@ -583,109 +564,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, VIR_STRDUP(source->dir, "/") < 0) goto cleanup; - if ((adapter_type = virXPathString("string(./adapter/@type)", ctxt))) { - if ((source->adapter.type = - virStoragePoolSourceAdapterTypeFromString(adapter_type)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown pool adapter type '%s'"), - adapter_type); + if ((adapternode = virXPathNode("./adapter", ctxt))) { + if (virStorageAdapterParseXML(&source->adapter, adapternode, ctxt) < 0) goto cleanup; - } - - if (source->adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - source->adapter.data.fchost.parent = - virXPathString("string(./adapter/@parent)", ctxt); - managed = virXPathString("string(./adapter/@managed)", ctxt); - if (managed) { - source->adapter.data.fchost.managed = - virTristateBoolTypeFromString(managed); - if (source->adapter.data.fchost.managed < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown fc_host managed setting '%s'"), - managed); - goto cleanup; - } - } - - 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) { - - source->adapter.data.scsi_host.name = - virXPathString("string(./adapter/@name)", ctxt); - if (virXPathNode("./adapter/parentaddr", ctxt)) { - xmlNodePtr addrnode = virXPathNode("./adapter/parentaddr/address", - ctxt); - virPCIDeviceAddressPtr addr = - &source->adapter.data.scsi_host.parentaddr; - - if (!addrnode) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing scsi_host PCI address element")); - goto cleanup; - } - source->adapter.data.scsi_host.has_parent = true; - if (virPCIDeviceAddressParseXML(addrnode, addr) < 0) - goto cleanup; - if ((virXPathInt("string(./adapter/parentaddr/@unique_id)", - ctxt, - &source->adapter.data.scsi_host.unique_id) < 0) || - (source->adapter.data.scsi_host.unique_id < 0)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing or invalid scsi adapter " - "'unique_id' value")); - goto cleanup; - } - } - } - } else { - char *wwnn = NULL; - char *wwpn = NULL; - char *parent = NULL; - - /* "type" was not specified in the XML, so we must verify that - * "wwnn", "wwpn", "parent", or "parentaddr" are also not in the - * XML. If any are found, then we cannot just use "name" alone". - */ - wwnn = virXPathString("string(./adapter/@wwnn)", ctxt); - wwpn = virXPathString("string(./adapter/@wwpn)", ctxt); - parent = virXPathString("string(./adapter/@parent)", ctxt); - - if (wwnn || wwpn || parent) { - VIR_FREE(wwnn); - VIR_FREE(wwpn); - VIR_FREE(parent); - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Use of 'wwnn', 'wwpn', and 'parent' attributes " - "requires use of the adapter 'type'")); - goto cleanup; - } - - if (virXPathNode("./adapter/parentaddr", ctxt)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Use of 'parent' element requires use " - "of the adapter 'type'")); - goto cleanup; - } - - /* To keep back-compat, 'type' is not required to specify - * for scsi_host adapter. - */ - if ((source->adapter.data.scsi_host.name = - virXPathString("string(./adapter/@name)", ctxt))) - source->adapter.type = - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST; } if ((authnode = virXPathNode("./auth", ctxt))) { @@ -711,8 +592,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, VIR_FREE(port); VIR_FREE(nodeset); - VIR_FREE(adapter_type); - VIR_FREE(managed); virStorageAuthDefFree(authdef); return ret; } @@ -928,37 +807,8 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) goto error; } - if (ret->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - if (!ret->source.adapter.data.fchost.wwnn || - !ret->source.adapter.data.fchost.wwpn) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'wwnn' and 'wwpn' must be specified for adapter " - "type 'fchost'")); - goto error; - } - - if (!virValidateWWN(ret->source.adapter.data.fchost.wwnn) || - !virValidateWWN(ret->source.adapter.data.fchost.wwpn)) - goto error; - } else if (ret->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (!ret->source.adapter.data.scsi_host.name && - !ret->source.adapter.data.scsi_host.has_parent) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Either 'name' or 'parent' must be specified " - "for the 'scsi_host' adapter")); - goto error; - } - - if (ret->source.adapter.data.scsi_host.name && - ret->source.adapter.data.scsi_host.has_parent) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Both 'name' and 'parent' cannot be specified " - "for the 'scsi_host' adapter")); - goto error; - } - } + if (virStorageAdapterParseValidate(&ret->source.adapter) < 0) + goto error; } /* If DEVICE is the only source type, then its required */ @@ -1112,49 +962,10 @@ virStoragePoolSourceFormat(virBufferPtr buf, if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) virBufferEscapeString(buf, "<dir path='%s'/>\n", src->dir); - if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) { - if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST || - src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) - virBufferAsprintf(buf, "<adapter type='%s'", - virStoragePoolSourceAdapterTypeToString(src->adapter.type)); - - if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - virBufferEscapeString(buf, " parent='%s'", - src->adapter.data.fchost.parent); - 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); - } else if (src->adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (src->adapter.data.scsi_host.name) { - virBufferAsprintf(buf, " name='%s'/>\n", - src->adapter.data.scsi_host.name); - } else { - virPCIDeviceAddress addr; - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<parentaddr unique_id='%d'>\n", - src->adapter.data.scsi_host.unique_id); - virBufferAdjustIndent(buf, 2); - addr = src->adapter.data.scsi_host.parentaddr; - ignore_value(virPCIDeviceAddressFormat(buf, addr, false)); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</parentaddr>\n"); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</adapter>\n"); - } - } - } + if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) && + (src->adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST || + src->adapter.type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST)) + virStorageAdapterFormat(buf, &src->adapter); if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) virBufferEscapeString(buf, "<name>%s</name>\n", src->name); @@ -2279,16 +2090,16 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, static int -getSCSIHostNumber(virStoragePoolSourceAdapter adapter, +getSCSIHostNumber(virStorageAdapterSCSIHostPtr scsi_host, unsigned int *hostnum) { int ret = -1; unsigned int num; char *name = NULL; - if (adapter.data.scsi_host.has_parent) { - virPCIDeviceAddress addr = adapter.data.scsi_host.parentaddr; - unsigned int unique_id = adapter.data.scsi_host.unique_id; + if (scsi_host->has_parent) { + virPCIDeviceAddress addr = scsi_host->parentaddr; + unsigned int unique_id = scsi_host->unique_id; if (!(name = virSCSIHostGetNameByParentaddr(addr.domain, addr.bus, @@ -2299,7 +2110,7 @@ getSCSIHostNumber(virStoragePoolSourceAdapter adapter, if (virSCSIHostGetNumber(name, &num) < 0) goto cleanup; } else { - if (virSCSIHostGetNumber(adapter.data.scsi_host.name, &num) < 0) + if (virSCSIHostGetNumber(scsi_host->name, &num) < 0) goto cleanup; } @@ -2330,7 +2141,7 @@ virStorageIsSameHostnum(const char *name, * matchFCHostToSCSIHost: * * @conn: Connection pointer - * @fc_adapter: fc_host adapter (either def or pool->def) + * @fchost: fc_host adapter ptr (either def or pool->def) * @scsi_hostnum: Already determined "scsi_pool" hostnum * * Returns true/false whether there is a match between the incoming @@ -2338,7 +2149,7 @@ virStorageIsSameHostnum(const char *name, */ static bool matchFCHostToSCSIHost(virConnectPtr conn, - virStoragePoolSourceAdapter fc_adapter, + virStorageAdapterFCHostPtr fchost, unsigned int scsi_hostnum) { bool ret = false; @@ -2349,16 +2160,14 @@ matchFCHostToSCSIHost(virConnectPtr conn, /* If we have a parent defined, get its hostnum, and compare to the * scsi_hostnum. If they are the same, then we have a match */ - if (fc_adapter.data.fchost.parent && - virStorageIsSameHostnum(fc_adapter.data.fchost.parent, scsi_hostnum)) + if (fchost->parent && + virStorageIsSameHostnum(fchost->parent, scsi_hostnum)) return true; - /* If we find an fc_adapter name, then either libvirt created a vHBA + /* If we find an fc adapter name, then either libvirt created a vHBA * for this fc_host or a 'virsh nodedev-create' generated a vHBA. */ - if ((name = virVHBAGetHostByWWN(NULL, fc_adapter.data.fchost.wwnn, - fc_adapter.data.fchost.wwpn))) { - + if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { /* Get the scsi_hostN for the vHBA in order to see if it * matches our scsi_hostnum */ @@ -2372,7 +2181,7 @@ matchFCHostToSCSIHost(virConnectPtr conn, * If the parent fc_hostnum is the same as the scsi_hostnum, we * have a match. */ - if (conn && !fc_adapter.data.fchost.parent) { + if (conn && !fchost->parent) { if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) goto cleanup; if ((parent_name = virNodeDeviceGetParentName(conn, @@ -2404,25 +2213,21 @@ matchFCHostToSCSIHost(virConnectPtr conn, return ret; } + static bool -matchSCSIAdapterParent(virStoragePoolObjPtr pool, - virStoragePoolDefPtr def) +matchSCSIAdapterParent(virStorageAdapterSCSIHostPtr pool_scsi_host, + virStorageAdapterSCSIHostPtr def_scsi_host) { - virPCIDeviceAddressPtr pooladdr = - &pool->def->source.adapter.data.scsi_host.parentaddr; - virPCIDeviceAddressPtr defaddr = - &def->source.adapter.data.scsi_host.parentaddr; - int pool_unique_id = - pool->def->source.adapter.data.scsi_host.unique_id; - int def_unique_id = - def->source.adapter.data.scsi_host.unique_id; + virPCIDeviceAddressPtr pooladdr = &pool_scsi_host->parentaddr; + virPCIDeviceAddressPtr defaddr = &def_scsi_host->parentaddr; + if (pooladdr->domain == defaddr->domain && pooladdr->bus == defaddr->bus && pooladdr->slot == defaddr->slot && pooladdr->function == defaddr->function && - pool_unique_id == def_unique_id) { + pool_scsi_host->unique_id == def_scsi_host->unique_id) return true; - } + return false; } @@ -2465,6 +2270,8 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, int ret = 1; virStoragePoolObjPtr pool = NULL; virStoragePoolObjPtr matchpool = NULL; + virStorageAdapterPtr pool_adapter; + virStorageAdapterPtr def_adapter; /* Check the pool list for duplicate underlying storage */ for (i = 0; i < pools->count; i++) { @@ -2500,62 +2307,70 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, break; case VIR_STORAGE_POOL_SCSI: - if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST && - def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - if (STREQ(pool->def->source.adapter.data.fchost.wwnn, - def->source.adapter.data.fchost.wwnn) && - STREQ(pool->def->source.adapter.data.fchost.wwpn, - def->source.adapter.data.fchost.wwpn)) + pool_adapter = &pool->def->source.adapter; + def_adapter = &def->source.adapter; + if (pool_adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST && + def_adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + virStorageAdapterFCHostPtr pool_fchost = + &pool_adapter->data.fchost; + virStorageAdapterFCHostPtr def_fchost = + &def_adapter->data.fchost; + if (STREQ(pool_fchost->wwnn, def_fchost->wwnn) && + STREQ(pool_fchost->wwpn, def_fchost->wwpn)) matchpool = pool; - } else if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST && - def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + } else if (pool_adapter->type == + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST && + def_adapter->type == + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { + virStorageAdapterSCSIHostPtr pool_scsi_host = + &pool_adapter->data.scsi_host; + virStorageAdapterSCSIHostPtr def_scsi_host = + &def_adapter->data.scsi_host; unsigned int pool_hostnum, def_hostnum; - if (pool->def->source.adapter.data.scsi_host.has_parent && - def->source.adapter.data.scsi_host.has_parent && - matchSCSIAdapterParent(pool, def)) { + if (pool_scsi_host->has_parent && + def_scsi_host->has_parent && + matchSCSIAdapterParent(pool_scsi_host, def_scsi_host)) { matchpool = pool; break; } - if (getSCSIHostNumber(pool->def->source.adapter, - &pool_hostnum) < 0 || - getSCSIHostNumber(def->source.adapter, &def_hostnum) < 0) + if (getSCSIHostNumber(pool_scsi_host, &pool_hostnum) < 0 || + getSCSIHostNumber(def_scsi_host, &def_hostnum) < 0) break; if (pool_hostnum == def_hostnum) matchpool = pool; - } else if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST && - def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + } else if (pool_adapter->type == + VIR_STORAGE_ADAPTER_TYPE_FC_HOST && + def_adapter->type == + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { + virStorageAdapterFCHostPtr pool_fchost = + &pool_adapter->data.fchost; + virStorageAdapterSCSIHostPtr def_scsi_host = + &def_adapter->data.scsi_host; unsigned int scsi_hostnum; /* Get the scsi_hostN for the scsi_host source adapter def */ - if (getSCSIHostNumber(def->source.adapter, - &scsi_hostnum) < 0) + if (getSCSIHostNumber(def_scsi_host, &scsi_hostnum) < 0) break; - if (matchFCHostToSCSIHost(conn, pool->def->source.adapter, - scsi_hostnum)) { + if (matchFCHostToSCSIHost(conn, pool_fchost, scsi_hostnum)) { matchpool = pool; break; } - } else if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST && - def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + } else if (pool_adapter->type == + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST && + def_adapter->type == + VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + virStorageAdapterSCSIHostPtr pool_scsi_host = + &pool_adapter->data.scsi_host; unsigned int scsi_hostnum; - if (getSCSIHostNumber(pool->def->source.adapter, - &scsi_hostnum) < 0) + if (getSCSIHostNumber(pool_scsi_host, &scsi_hostnum) < 0) break; - if (matchFCHostToSCSIHost(conn, def->source.adapter, + if (matchFCHostToSCSIHost(conn, &def_adapter->data.fchost, scsi_hostnum)) { matchpool = pool; break; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 1723afc..7c4c684 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "device_conf.h" # include "object_event.h" +# include "storage_adapter_conf.h" # include <libxml/tree.h> @@ -170,38 +171,6 @@ struct _virStoragePoolSourceDevice { } geometry; }; -typedef enum { - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_DEFAULT = 0, - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST, - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST, - - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST, -} virStoragePoolSourceAdapterType; -VIR_ENUM_DECL(virStoragePoolSourceAdapter) - -typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter; -typedef virStoragePoolSourceAdapter *virStoragePoolSourceAdapterPtr; -struct _virStoragePoolSourceAdapter { - int type; /* virStoragePoolSourceAdapterType */ - - union { - struct { - char *name; - virPCIDeviceAddress parentaddr; /* host address */ - int unique_id; - bool has_parent; - } scsi_host; - struct { - char *parent; - char *parent_wwnn; - char *parent_wwpn; - char *parent_fabric_wwn; - char *wwnn; - char *wwpn; - int managed; /* enum virTristateSwitch */ - } fchost; - } data; -}; typedef struct _virStoragePoolSource virStoragePoolSource; typedef virStoragePoolSource *virStoragePoolSourcePtr; @@ -218,7 +187,7 @@ struct _virStoragePoolSource { char *dir; /* Or an adapter */ - virStoragePoolSourceAdapter adapter; + virStorageAdapter adapter; /* Or a name */ char *name; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aed1d3d..b89e392 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -863,6 +863,19 @@ virDomainSnapshotStateTypeToString; virDomainSnapshotUpdateRelations; +# conf/storage_adapter_conf.h +virStorageAdapterClear; +virStorageAdapterFormat; +virStorageAdapterParseValidate; +virStorageAdapterParseXML; +virStorageAdapterTypeFromString; +virStorageAdapterTypeToString; +virStorageAdapterVHBAClear; +virStorageAdapterVHBAFormat; +virStorageAdapterVHBAParseValidate; +virStorageAdapterVHBAParseXML; + + # conf/storage_conf.h virStoragePartedFsTypeToString; virStoragePoolDefFormat; @@ -892,8 +905,6 @@ virStoragePoolObjSaveDef; virStoragePoolObjUnlock; virStoragePoolSaveConfig; virStoragePoolSaveState; -virStoragePoolSourceAdapterTypeFromString; -virStoragePoolSourceAdapterTypeToString; virStoragePoolSourceClear; virStoragePoolSourceDeviceClear; virStoragePoolSourceFindDuplicate; @@ -2442,6 +2453,7 @@ virSocketAddrSetIPv6Addr; virSocketAddrSetIPv6AddrNetOrder; virSocketAddrSetPort; + # util/virstorageencryption.h virStorageEncryptionFormat; virStorageEncryptionFree; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 7a5df3f..39fa026 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2467,8 +2467,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; - if (source.adapter.type != - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + if (source.adapter.type != VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Only 'scsi_host' adapter is supported")); goto cleanup; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 642e795..ff17409 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -176,15 +176,17 @@ virStoragePoolFCRefreshThread(void *opaque) } static char * -getAdapterName(virStoragePoolSourceAdapter adapter) +getAdapterName(virStorageAdapterPtr adapter) { char *name = NULL; char *parentaddr = NULL; - if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (adapter.data.scsi_host.has_parent) { - virPCIDeviceAddress addr = adapter.data.scsi_host.parentaddr; - unsigned int unique_id = adapter.data.scsi_host.unique_id; + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { + virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host; + + if (scsi_host->has_parent) { + virPCIDeviceAddress addr = scsi_host->parentaddr; + unsigned int unique_id = scsi_host->unique_id; if (!(name = virSCSIHostGetNameByParentaddr(addr.domain, addr.bus, @@ -193,16 +195,17 @@ getAdapterName(virStoragePoolSourceAdapter adapter) unique_id))) goto cleanup; } else { - ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name)); + ignore_value(VIR_STRDUP(name, scsi_host->name)); } - } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - if (!(name = virVHBAGetHostByWWN(NULL, - adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) { + } + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + virStorageAdapterFCHostPtr fchost = &adapter->data.fchost; + + if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { virReportError(VIR_ERR_XML_ERROR, _("Failed to find SCSI host with wwnn='%s', " - "wwpn='%s'"), adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn); + "wwpn='%s'"), fchost->wwnn, fchost->wwpn); } } @@ -254,10 +257,10 @@ checkParent(virConnectPtr conn, static int createVport(virConnectPtr conn, - virStoragePoolObjPtr pool) + virStoragePoolDefPtr def, + const char *configFile, + virStorageAdapterFCHostPtr fchost) { - const char *configFile = pool->configFile; - virStoragePoolSourceAdapterPtr adapter = &pool->def->source.adapter; unsigned int parent_host; char *name = NULL; char *parent_hoststr = NULL; @@ -266,45 +269,37 @@ createVport(virConnectPtr conn, virThread thread; int ret = -1; - if (adapter->type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) - return 0; - VIR_DEBUG("conn=%p, configFile='%s' parent='%s', wwnn='%s' wwpn='%s'", - conn, NULLSTR(configFile), NULLSTR(adapter->data.fchost.parent), - adapter->data.fchost.wwnn, adapter->data.fchost.wwpn); + conn, NULLSTR(configFile), NULLSTR(fchost->parent), + fchost->wwnn, fchost->wwpn); /* If we find an existing HBA/vHBA within the fc_host sysfs * using the wwnn/wwpn, then a nodedev is already created for * this pool and we don't have to create the vHBA */ - if ((name = virVHBAGetHostByWWN(NULL, adapter->data.fchost.wwnn, - adapter->data.fchost.wwpn))) { + if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { /* If a parent was provided, let's make sure the 'name' we've * retrieved has the same parent */ - if (adapter->data.fchost.parent && - checkParent(conn, name, adapter->data.fchost.parent)) + if (fchost->parent && checkParent(conn, name, fchost->parent)) ret = 0; goto cleanup; } - if (adapter->data.fchost.parent) { - if (VIR_STRDUP(parent_hoststr, adapter->data.fchost.parent) < 0) + if (fchost->parent) { + if (VIR_STRDUP(parent_hoststr, fchost->parent) < 0) goto cleanup; - } else if (adapter->data.fchost.parent_wwnn && - adapter->data.fchost.parent_wwpn) { - if (!(parent_hoststr = - virVHBAGetHostByWWN(NULL, adapter->data.fchost.parent_wwnn, - adapter->data.fchost.parent_wwpn))) { + } else if (fchost->parent_wwnn && fchost->parent_wwpn) { + if (!(parent_hoststr = virVHBAGetHostByWWN(NULL, fchost->parent_wwnn, + 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) { + } else if (fchost->parent_fabric_wwn) { if (!(parent_hoststr = - virVHBAGetHostByFabricWWN(NULL, - adapter->data.fchost.parent_fabric_wwn))) { + virVHBAGetHostByFabricWWN(NULL, fchost->parent_fabric_wwn))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot find parent using provided fabric_wwn")); goto cleanup; @@ -323,8 +318,8 @@ createVport(virConnectPtr conn, goto cleanup; /* NOTE: - * We do not save the parent_hoststr in adapter->data.fchost.parent - * since we could be writing out the 'def' to the saved XML config. + * We do not save the parent_hoststr in fchost->parent since + * we could be writing out the 'def' to the saved XML config. * If we wrote out the name in the XML, then future starts would * always use the same parent rather than finding the "best available" * parent. Besides we have a way to determine the parent based on @@ -342,16 +337,16 @@ createVport(virConnectPtr conn, * restart, we need to save the persistent configuration. So if not * already defined as YES, then force the issue. */ - if (adapter->data.fchost.managed != VIR_TRISTATE_BOOL_YES) { - adapter->data.fchost.managed = VIR_TRISTATE_BOOL_YES; + if (fchost->managed != VIR_TRISTATE_BOOL_YES) { + fchost->managed = VIR_TRISTATE_BOOL_YES; if (configFile) { - if (virStoragePoolSaveConfig(configFile, pool->def) < 0) + if (virStoragePoolSaveConfig(configFile, def) < 0) goto cleanup; } } - if (virVHBAManageVport(parent_host, adapter->data.fchost.wwpn, - adapter->data.fchost.wwnn, VPORT_CREATE) < 0) + if (virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, + VPORT_CREATE) < 0) goto cleanup; virFileWaitForDevices(); @@ -361,10 +356,9 @@ createVport(virConnectPtr conn, * retry logic set to true. If the thread isn't created, then no big * deal since it's still possible to refresh the pool later. */ - if ((name = virVHBAGetHostByWWN(NULL, adapter->data.fchost.wwnn, - adapter->data.fchost.wwpn))) { + if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { if (VIR_ALLOC(cbdata) == 0) { - memcpy(cbdata->pool_uuid, pool->def->uuid, VIR_UUID_BUFLEN); + memcpy(cbdata->pool_uuid, def->uuid, VIR_UUID_BUFLEN); VIR_STEAL_PTR(cbdata->fchost_name, name); if (virThreadCreate(&thread, false, virStoragePoolFCRefreshThread, @@ -384,9 +378,10 @@ createVport(virConnectPtr conn, return ret; } + static int deleteVport(virConnectPtr conn, - virStoragePoolSourceAdapter adapter) + virStorageAdapterFCHostPtr fchost) { unsigned int parent_host; char *name = NULL; @@ -394,25 +389,19 @@ deleteVport(virConnectPtr conn, char *vhba_parent = NULL; int ret = -1; - if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) - return 0; - VIR_DEBUG("conn=%p parent='%s', managed='%d' wwnn='%s' wwpn='%s'", - conn, NULLSTR(adapter.data.fchost.parent), - adapter.data.fchost.managed, - adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn); + conn, NULLSTR(fchost->parent), fchost->managed, + fchost->wwnn, fchost->wwpn); /* If we're not managing the deletion of the vHBA, then just return */ - if (adapter.data.fchost.managed != VIR_TRISTATE_BOOL_YES) + if (fchost->managed != VIR_TRISTATE_BOOL_YES) return 0; /* Find our vHBA by searching the fc_host sysfs tree for our wwnn/wwpn */ - if (!(name = virVHBAGetHostByWWN(NULL, adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) { + if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to find fc_host for wwnn='%s' and wwpn='%s'"), - adapter.data.fchost.wwnn, adapter.data.fchost.wwpn); + fchost->wwnn, fchost->wwpn); goto cleanup; } @@ -420,8 +409,8 @@ deleteVport(virConnectPtr conn, * get the parent_host value; otherwise, we have to determine * the parent scsi_host which we did not save at startup time */ - if (adapter.data.fchost.parent) { - if (virSCSIHostGetNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (fchost->parent) { + if (virSCSIHostGetNumber(fchost->parent, &parent_host) < 0) goto cleanup; } else { if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) @@ -434,8 +423,8 @@ deleteVport(virConnectPtr conn, goto cleanup; } - if (virVHBAManageVport(parent_host, adapter.data.fchost.wwpn, - adapter.data.fchost.wwnn, VPORT_DELETE) < 0) + if (virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, + VPORT_DELETE) < 0) goto cleanup; ret = 0; @@ -458,13 +447,13 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, *isActive = false; - if (!(name = getAdapterName(pool->def->source.adapter))) { + if (!(name = getAdapterName(&pool->def->source.adapter))) { /* It's normal for the pool with "fc_host" type source * adapter fails to get the adapter name, since the vHBA * the adapter based on might be not created yet. */ if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { virResetLastError(); return 0; } else { @@ -498,7 +487,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, pool->def->allocation = pool->def->capacity = pool->def->available = 0; - if (!(name = getAdapterName(pool->def->source.adapter))) + if (!(name = getAdapterName(&pool->def->source.adapter))) return -1; if (virSCSIHostGetNumber(name, &host) < 0) @@ -518,19 +507,27 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } + static int virStorageBackendSCSIStartPool(virConnectPtr conn, virStoragePoolObjPtr pool) { - return createVport(conn, pool); + if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + return createVport(conn, pool->def, pool->configFile, + &pool->def->source.adapter.data.fchost); + + return 0; } + static int virStorageBackendSCSIStopPool(virConnectPtr conn, virStoragePoolObjPtr pool) { - virStoragePoolSourceAdapter adapter = pool->def->source.adapter; - return deleteVport(conn, adapter); + if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + return deleteVport(conn, &pool->def->source.adapter.data.fchost); + + return 0; } virStorageBackend virStorageBackendSCSI = { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a9bb5a6..a18a1de 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4424,8 +4424,7 @@ testStoragePoolCreateXML(virConnectPtr conn, goto cleanup; def = NULL; - if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { /* In the real code, we'd call virVHBAManageVport followed by * find_new_device, but we cannot do that here since we're not * mocking udev. The mock routine will copy an existing vHBA and @@ -4645,7 +4644,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool) privpool->active = 0; if (privpool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { if (testDestroyVport(privconn, privpool->def->source.adapter.data.fchost.wwnn, privpool->def->source.adapter.data.fchost.wwpn) < 0) -- 2.9.3

On 03/09/2017 10:35 AM, Ján Tomko wrote: <sigh> OK... this was one of those that I just go going and wasn't careful to stop at particular steps. Things compounded fairly quickly. In any case, in a be careful for what you wish type comment - I've done a split of patch2 and now there's 12 patches to replace it. Of course that leads me down the path of well there's too many patches to review so it'll languish on list... Oh well, it's a small price to pay to make progress I guess. Hmm... True... Neither was parent_wwnn/parent_wwpn validation. See what I mean about compounding things! It certainly became more obvious the deeper I went. In any case, I split this change out into a separate patch that goes before all the patches that will perform the step by step split. I also added one more from a bz that I have in which a tester only supplied one of parent_wwnn/parent_wwpn... Just putting the finishing touches on all of them before posting... Tks - John

Move the bulk of the code to the node_device_conf and rename to virNodeDevice{Create|Delete}Vport. Alter the create algorithm slightly in order to return the name of the scsi_host vHBA that was created. The existing code would fetch the name and if it exists would start a thread looking for the LUNs; however, in no way did it validate the device was created for the storage pool even though it would be used thereafter. This slight change allows the code that checks if the node device by wwnn/wwpn already exists and return that name. This fixes a second yet seen issue that if the nodedev was present, but the parent by name wasn't provided (perhaps parent by wwnn/wwpn or by fabric_name), then a failure would be returned. For this path it shouldn't be an error - we should just be happy that something else is managing the device and we don't have to create/delete it. The end result is that the startVport code can now just start the thread once it gets a non NULL name back. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/node_device_conf.c | 211 +++++++++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 9 ++ src/libvirt_private.syms | 2 + src/storage/storage_backend_scsi.c | 192 +++------------------------------ 4 files changed, 235 insertions(+), 179 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 3565aec..66cb78d 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1870,3 +1870,214 @@ virNodeDeviceGetParentName(virConnectPtr conn, return parent; } + + +/* + * Using the host# name found via wwnn/wwpn lookup in the fc_host + * sysfs tree to get the parent 'scsi_host#' to ensure it matches. + */ +static bool +nodeDeviceCheckParent(virConnectPtr conn, + const char *name, + const char *parent_name) +{ + char *scsi_host_name = NULL; + char *vhba_parent = NULL; + bool retval = false; + + VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name); + + /* autostarted pool - assume we're OK */ + if (!conn) + return true; + + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) + goto cleanup; + + if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) + goto cleanup; + + if (STRNEQ(parent_name, vhba_parent)) { + virReportError(VIR_ERR_XML_ERROR, + _("Parent attribute '%s' does not match parent '%s' " + "determined for the '%s' wwnn/wwpn lookup."), + parent_name, vhba_parent, name); + goto cleanup; + } + + retval = true; + + cleanup: + VIR_FREE(vhba_parent); + VIR_FREE(scsi_host_name); + return retval; +} + + +/** + * @conn: Connection pointer + * @fchost: Pointer to vHBA adapter + * + * Create a vHBA for Storage. This code accomplishes this via searching + * through the sysfs for scsi_host/fc_host in order to first ensure some + * vHBA doesn't already exist for the requested wwnn/wwpn (e.g. an unmanaged + * vHBA) and to search for the parent vport capable scsi_host by name, + * wwnn/wwpn, or fabric_wwn (if provided). If no parent is provided, then + * a vport capable scsi_host will be selected. + * + * Returns vHBA name on success, NULL on failure with an error message set + */ +char * +virNodeDeviceCreateVport(virConnectPtr conn, + virStorageAdapterFCHostPtr fchost) +{ + unsigned int parent_host; + char *name = NULL; + char *parent_hoststr = NULL; + bool skip_capable_check = false; + + VIR_DEBUG("conn=%p, parent='%s', wwnn='%s' wwpn='%s'", + conn, NULLSTR(fchost->parent), fchost->wwnn, fchost->wwpn); + + /* If we find an existing HBA/vHBA within the fc_host sysfs + * using the wwnn/wwpn, then a nodedev is already created for + * this pool and we don't have to create the vHBA + */ + if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { + /* If a parent was provided, let's make sure the 'name' we've + * retrieved has the same parent. If not this will cause failure. */ + if (fchost->parent && nodeDeviceCheckParent(conn, name, fchost->parent)) + VIR_FREE(name); + + return name; + } + + if (fchost->parent) { + if (VIR_STRDUP(parent_hoststr, fchost->parent) < 0) + goto cleanup; + } else if (fchost->parent_wwnn && fchost->parent_wwpn) { + if (!(parent_hoststr = virVHBAGetHostByWWN(NULL, fchost->parent_wwnn, + fchost->parent_wwpn))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("cannot find parent using provided wwnn/wwpn")); + goto cleanup; + } + } else if (fchost->parent_fabric_wwn) { + if (!(parent_hoststr = + virVHBAGetHostByFabricWWN(NULL, fchost->parent_fabric_wwn))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("cannot find parent using provided fabric_wwn")); + goto cleanup; + } + } else { + if (!(parent_hoststr = virVHBAFindVportHost(NULL))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'parent' for vHBA not specified, and " + "cannot find one on this host")); + goto cleanup; + } + skip_capable_check = true; + } + + if (virSCSIHostGetNumber(parent_hoststr, &parent_host) < 0) + goto cleanup; + + /* NOTE: + * We do not save the parent_hoststr in fchost->parent since + * we could be writing out the 'def' to the saved XML config. + * If we wrote out the name in the XML, then future starts would + * always use the same parent rather than finding the "best available" + * parent. Besides we have a way to determine the parent based on + * the 'name' field. + */ + if (!skip_capable_check && !virVHBAPathExists(NULL, parent_host)) { + virReportError(VIR_ERR_XML_ERROR, + _("parent '%s' specified for vHBA does not exist"), + parent_hoststr); + goto cleanup; + } + + if (virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, + VPORT_CREATE) < 0) + goto cleanup; + + /* Let's ensure the device was created */ + virWaitForDevices(); + if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { + ignore_value(virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, + VPORT_DELETE)); + goto cleanup; + } + + cleanup: + VIR_FREE(parent_hoststr); + return name; +} + + +/** + * @conn: Connection pointer + * @fchost: Pointer to vHBA adapter + * + * As long as the vHBA is being managed, search for the scsi_host via the + * provided wwnn/wwpn and then find the corresponding parent scsi_host in + * order to send the delete request. + * + * Returns 0 on success, -1 on failure + */ +int +virNodeDeviceDeleteVport(virConnectPtr conn, + virStorageAdapterFCHostPtr fchost) +{ + char *name = NULL; + char *scsi_host_name = NULL; + unsigned int parent_host; + char *vhba_parent = NULL; + int ret = -1; + + VIR_DEBUG("conn=%p parent='%s', managed='%d' wwnn='%s' wwpn='%s'", + conn, NULLSTR(fchost->parent), fchost->managed, + fchost->wwnn, fchost->wwpn); + + /* If we're not managing the deletion of the vHBA, then just return */ + if (fchost->managed != VIR_TRISTATE_BOOL_YES) + return 0; + + /* Find our vHBA by searching the fc_host sysfs tree for our wwnn/wwpn */ + if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find fc_host for wwnn='%s' and wwpn='%s'"), + fchost->wwnn, fchost->wwpn); + goto cleanup; + } + + /* If at startup time we provided a parent, then use that to + * get the parent_host value; otherwise, we have to determine + * the parent scsi_host which we did not save at startup time + */ + if (fchost->parent) { + if (virSCSIHostGetNumber(fchost->parent, &parent_host) < 0) + goto cleanup; + } else { + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) + goto cleanup; + + if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) + goto cleanup; + + if (virSCSIHostGetNumber(vhba_parent, &parent_host) < 0) + goto cleanup; + } + + if (virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, + VPORT_DELETE) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(name); + VIR_FREE(vhba_parent); + VIR_FREE(scsi_host_name); + return ret; +} diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 7aed1f4..27b5db0 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -28,8 +28,11 @@ # include "internal.h" # include "virbitmap.h" # include "virutil.h" +# include "virscsihost.h" # include "virpci.h" +# include "virvhba.h" # include "device_conf.h" +# include "storage_adapter_conf.h" # include <libxml/tree.h> @@ -352,4 +355,10 @@ char * virNodeDeviceGetParentName(virConnectPtr conn, const char *nodedev_name); +char *virNodeDeviceCreateVport(virConnectPtr conn, + virStorageAdapterFCHostPtr fchost); + +int virNodeDeviceDeleteVport(virConnectPtr conn, + virStorageAdapterFCHostPtr fchost); + #endif /* __VIR_NODE_DEVICE_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4620d68..8a1fdf5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -694,11 +694,13 @@ virNetDevIPRouteParseXML; virNodeDevCapsDefFree; virNodeDevCapTypeFromString; virNodeDevCapTypeToString; +virNodeDeviceCreateVport; virNodeDeviceDefFormat; virNodeDeviceDefFree; virNodeDeviceDefParseFile; virNodeDeviceDefParseNode; virNodeDeviceDefParseString; +virNodeDeviceDeleteVport; virNodeDeviceGetParentName; virNodeDeviceGetWWNs; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index abbd38d..666e473 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -33,9 +33,7 @@ #include "virlog.h" #include "virfile.h" #include "vircommand.h" -#include "virscsihost.h" #include "virstring.h" -#include "virvhba.h" #include "storage_util.h" #include "node_device_conf.h" @@ -214,57 +212,13 @@ getAdapterName(virStorageAdapterPtr adapter) return name; } -/* - * Using the host# name found via wwnn/wwpn lookup in the fc_host - * sysfs tree to get the parent 'scsi_host#' to ensure it matches. - */ -static bool -checkParent(virConnectPtr conn, - const char *name, - const char *parent_name) -{ - char *scsi_host_name = NULL; - char *vhba_parent = NULL; - bool retval = false; - - VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name); - - /* autostarted pool - assume we're OK */ - if (!conn) - return true; - - if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) - goto cleanup; - - if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) - goto cleanup; - - if (STRNEQ(parent_name, vhba_parent)) { - virReportError(VIR_ERR_XML_ERROR, - _("Parent attribute '%s' does not match parent '%s' " - "determined for the '%s' wwnn/wwpn lookup."), - parent_name, vhba_parent, name); - goto cleanup; - } - - retval = true; - - cleanup: - VIR_FREE(vhba_parent); - VIR_FREE(scsi_host_name); - return retval; -} - static int createVport(virConnectPtr conn, virStoragePoolDefPtr def, const char *configFile, virStorageAdapterFCHostPtr fchost) { - unsigned int parent_host; char *name = NULL; - char *parent_hoststr = NULL; - bool skip_capable_check = false; virStoragePoolFCRefreshInfoPtr cbdata = NULL; virThread thread; int ret = -1; @@ -273,66 +227,11 @@ createVport(virConnectPtr conn, conn, NULLSTR(configFile), NULLSTR(fchost->parent), fchost->wwnn, fchost->wwpn); - /* If we find an existing HBA/vHBA within the fc_host sysfs - * using the wwnn/wwpn, then a nodedev is already created for - * this pool and we don't have to create the vHBA - */ - if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { - /* If a parent was provided, let's make sure the 'name' we've - * retrieved has the same parent - */ - if (fchost->parent && checkParent(conn, name, fchost->parent)) - ret = 0; + if (!(name = virNodeDeviceCreateVport(conn, fchost))) goto cleanup; - } - if (fchost->parent) { - if (VIR_STRDUP(parent_hoststr, fchost->parent) < 0) - goto cleanup; - } else if (fchost->parent_wwnn && fchost->parent_wwpn) { - if (!(parent_hoststr = virVHBAGetHostByWWN(NULL, fchost->parent_wwnn, - fchost->parent_wwpn))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("cannot find parent using provided wwnn/wwpn")); - goto cleanup; - } - } else if (fchost->parent_fabric_wwn) { - if (!(parent_hoststr = - virVHBAGetHostByFabricWWN(NULL, fchost->parent_fabric_wwn))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("cannot find parent using provided fabric_wwn")); - goto cleanup; - } - } else { - if (!(parent_hoststr = virVHBAFindVportHost(NULL))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'parent' for vHBA not specified, and " - "cannot find one on this host")); - goto cleanup; - } - skip_capable_check = true; - } - - if (virSCSIHostGetNumber(parent_hoststr, &parent_host) < 0) - goto cleanup; - - /* NOTE: - * We do not save the parent_hoststr in fchost->parent since - * we could be writing out the 'def' to the saved XML config. - * If we wrote out the name in the XML, then future starts would - * always use the same parent rather than finding the "best available" - * parent. Besides we have a way to determine the parent based on - * the 'name' field. - */ - if (!skip_capable_check && !virVHBAPathExists(NULL, parent_host)) { - virReportError(VIR_ERR_XML_ERROR, - _("parent '%s' specified for vHBA does not exist"), - parent_hoststr); - goto cleanup; - } - - /* Since we're creating the vHBA, then we need to manage removing it + /* Since we've created the vHBA, then we need to manage removing it * as well. Since we need this setting to "live" through a libvirtd * restart, we need to save the persistent configuration. So if not * already defined as YES, then force the issue. @@ -345,28 +244,20 @@ createVport(virConnectPtr conn, } } - if (virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, - VPORT_CREATE) < 0) - goto cleanup; - - virWaitForDevices(); - /* Creating our own VPORT didn't leave enough time to find any LUN's, * so, let's create a thread whose job it is to call the FindLU's with * retry logic set to true. If the thread isn't created, then no big * deal since it's still possible to refresh the pool later. */ - if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { - if (VIR_ALLOC(cbdata) == 0) { - memcpy(cbdata->pool_uuid, def->uuid, VIR_UUID_BUFLEN); - VIR_STEAL_PTR(cbdata->fchost_name, name); - - if (virThreadCreate(&thread, false, virStoragePoolFCRefreshThread, - cbdata) < 0) { - /* Oh well - at least someone can still refresh afterwards */ - VIR_DEBUG("Failed to create FC Pool Refresh Thread"); - virStoragePoolFCRefreshDataFree(cbdata); - } + if (VIR_ALLOC(cbdata) == 0) { + memcpy(cbdata->pool_uuid, def->uuid, VIR_UUID_BUFLEN); + VIR_STEAL_PTR(cbdata->fchost_name, name); + + if (virThreadCreate(&thread, false, virStoragePoolFCRefreshThread, + cbdata) < 0) { + /* Oh well - at least someone can still refresh afterwards */ + VIR_DEBUG("Failed to create FC Pool Refresh Thread"); + virStoragePoolFCRefreshDataFree(cbdata); } } @@ -374,64 +265,6 @@ createVport(virConnectPtr conn, cleanup: VIR_FREE(name); - VIR_FREE(parent_hoststr); - return ret; -} - - -static int -deleteVport(virConnectPtr conn, - virStorageAdapterFCHostPtr fchost) -{ - unsigned int parent_host; - char *name = NULL; - char *scsi_host_name = NULL; - char *vhba_parent = NULL; - int ret = -1; - - VIR_DEBUG("conn=%p parent='%s', managed='%d' wwnn='%s' wwpn='%s'", - conn, NULLSTR(fchost->parent), fchost->managed, - fchost->wwnn, fchost->wwpn); - - /* If we're not managing the deletion of the vHBA, then just return */ - if (fchost->managed != VIR_TRISTATE_BOOL_YES) - return 0; - - /* Find our vHBA by searching the fc_host sysfs tree for our wwnn/wwpn */ - if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to find fc_host for wwnn='%s' and wwpn='%s'"), - fchost->wwnn, fchost->wwpn); - goto cleanup; - } - - /* If at startup time we provided a parent, then use that to - * get the parent_host value; otherwise, we have to determine - * the parent scsi_host which we did not save at startup time - */ - if (fchost->parent) { - if (virSCSIHostGetNumber(fchost->parent, &parent_host) < 0) - goto cleanup; - } else { - if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) - goto cleanup; - - if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) - goto cleanup; - - if (virSCSIHostGetNumber(vhba_parent, &parent_host) < 0) - goto cleanup; - } - - if (virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn, - VPORT_DELETE) < 0) - goto cleanup; - - ret = 0; - cleanup: - VIR_FREE(name); - VIR_FREE(vhba_parent); - VIR_FREE(scsi_host_name); return ret; } @@ -525,7 +358,8 @@ virStorageBackendSCSIStopPool(virConnectPtr conn, virStoragePoolObjPtr pool) { if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) - return deleteVport(conn, &pool->def->source.adapter.data.fchost); + return virNodeDeviceDeleteVport(conn, + &pool->def->source.adapter.data.fchost); return 0; } -- 2.9.3

Add the parent wwnn/wwpn and fabric_wwn tests for the storage pool now that we have the capability to use the node device driver to handle the searches via a connection. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/fchosttest.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index c024ae5..c080c9b 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -100,6 +100,30 @@ static const char test12_xml[] = "</pool>"; +/* virStoragePoolCreateXML parent wwnn/wwpn to find the vport capable HBA */ +static const char test13_xml[] = +"<pool type='scsi'>" +" <name>vhba_pool</name>" +" <source>" +" <adapter type='fc_host' parent_wwnn='2000000012341234' parent_wwpn='1000000012341234' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>" +" </source>" +" <target>" +" <path>/dev/disk/by-path</path>" +" </target>" +"</pool>"; + +/* virStoragePoolCreateXML parent fabric_wwn to find the vport capable HBA */ +static const char test14_xml[] = +"<pool type='scsi'>" +" <name>vhba_pool</name>" +" <source>" +" <adapter type='fc_host' parent_fabric_wwn='2000000043214321' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>" +" </source>" +" <target>" +" <path>/dev/disk/by-path</path>" +" </target>" +"</pool>"; + /* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) @@ -389,6 +413,14 @@ mymain(void) if (virTestRun("manageVHBAByStoragePool-no-parent", manageVHBAByStoragePool, test12_xml) < 0) ret = -1; + if (virTestRun("manageVHBAByStoragePool-parent-wwn", + manageVHBAByStoragePool, + test13_xml) < 0) + ret = -1; + if (virTestRun("manageVHBAByStoragePool-parent-fabric-wwn", + manageVHBAByStoragePool, + test14_xml) < 0) + ret = -1; cleanup: VIR_FREE(fchost_prefix); -- 2.9.3