nodedev: Rework virNodeDeviceGetParentHost

Rework the code to perform the various searches by parent, parent_wwnn/
parent_wwpn, parent_fabric_wwn, or vport capable in order to return the
'parent_host' number that is vHBA capable.

The former virNodeDeviceGetParentHost is renamed to add the ByParent
on it fixes an issue where if no parent was supplied in the XML to
create the vHBA, then virNodeDeviceFindByName was called with a NULL
second parameter which had bad results.

The reworked code will make the various calls to fetch the NPIV host
by the passed parameter options or if none are provided find a vport
capable NPIV HBA to perform the create. If the call is from the delete
path, then this option won't be allowed.

Each of virNodeDeviceGetParentHostBy* functions is now static, so
remove them external definitions.

A secondary benefit of this is the test_driver now can make use of
the new API to add some new tests to test the various creation options.
This commit is contained in:
John Ferlan 2017-01-24 12:21:26 -05:00
parent ccb0d6e342
commit 7ad479d0bd
5 changed files with 71 additions and 98 deletions

View File

@ -1975,17 +1975,15 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
*/ */
/* virNodeDeviceFindFCParentHost: /* virNodeDeviceFindFCParentHost:
* @parent: Pointer to node device object * @parent: Pointer to node device object
* @parent_host: Pointer to return parent host number
* *
* Search the capabilities for the device to find the FC capabilities * Search the capabilities for the device to find the FC capabilities
* in order to set the parent_host value. * in order to set the parent_host value.
* *
* Returns: * Returns:
* 0 on success with parent_host set, -1 otherwise; * parent_host value on success (>= 0), -1 otherwise.
*/ */
static int static int
virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent, virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent)
int *parent_host)
{ {
virNodeDevCapsDefPtr cap = virNodeDeviceFindVPORTCapDef(parent); virNodeDevCapsDefPtr cap = virNodeDeviceFindVPORTCapDef(parent);
@ -1997,16 +1995,14 @@ virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent,
return -1; return -1;
} }
*parent_host = cap->data.scsi_host.host; return cap->data.scsi_host.host;
return 0;
} }
int static int
virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, virNodeDeviceGetParentHostByParent(virNodeDeviceObjListPtr devs,
const char *dev_name, const char *dev_name,
const char *parent_name, const char *parent_name)
int *parent_host)
{ {
virNodeDeviceObjPtr parent = NULL; virNodeDeviceObjPtr parent = NULL;
int ret; int ret;
@ -2018,7 +2014,7 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
return -1; return -1;
} }
ret = virNodeDeviceFindFCParentHost(parent, parent_host); ret = virNodeDeviceFindFCParentHost(parent);
virNodeDeviceObjUnlock(parent); virNodeDeviceObjUnlock(parent);
@ -2026,12 +2022,11 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
} }
int static int
virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
const char *dev_name, const char *dev_name,
const char *parent_wwnn, const char *parent_wwnn,
const char *parent_wwpn, const char *parent_wwpn)
int *parent_host)
{ {
virNodeDeviceObjPtr parent = NULL; virNodeDeviceObjPtr parent = NULL;
int ret; int ret;
@ -2043,7 +2038,7 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
return -1; return -1;
} }
ret = virNodeDeviceFindFCParentHost(parent, parent_host); ret = virNodeDeviceFindFCParentHost(parent);
virNodeDeviceObjUnlock(parent); virNodeDeviceObjUnlock(parent);
@ -2051,11 +2046,10 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
} }
int static int
virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
const char *dev_name, const char *dev_name,
const char *parent_fabric_wwn, const char *parent_fabric_wwn)
int *parent_host)
{ {
virNodeDeviceObjPtr parent = NULL; virNodeDeviceObjPtr parent = NULL;
int ret; int ret;
@ -2067,7 +2061,7 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
return -1; return -1;
} }
ret = virNodeDeviceFindFCParentHost(parent, parent_host); ret = virNodeDeviceFindFCParentHost(parent);
virNodeDeviceObjUnlock(parent); virNodeDeviceObjUnlock(parent);
@ -2075,9 +2069,8 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
} }
int static int
virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs)
int *parent_host)
{ {
virNodeDeviceObjPtr parent = NULL; virNodeDeviceObjPtr parent = NULL;
const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS); const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
@ -2089,7 +2082,7 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
return -1; return -1;
} }
ret = virNodeDeviceFindFCParentHost(parent, parent_host); ret = virNodeDeviceFindFCParentHost(parent);
virNodeDeviceObjUnlock(parent); virNodeDeviceObjUnlock(parent);
@ -2097,6 +2090,33 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
} }
int
virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
virNodeDeviceDefPtr def,
int create)
{
int parent_host = -1;
if (def->parent) {
parent_host = virNodeDeviceGetParentHostByParent(devs, def->name,
def->parent);
} else if (def->parent_wwnn && def->parent_wwpn) {
parent_host = virNodeDeviceGetParentHostByWWNs(devs, def->name,
def->parent_wwnn,
def->parent_wwpn);
} else if (def->parent_fabric_wwn) {
parent_host =
virNodeDeviceGetParentHostByFabricWWN(devs, def->name,
def->parent_fabric_wwn);
} else if (create == CREATE_DEVICE) {
/* Try to find a vport capable scsi_host when no parent supplied */
parent_host = virNodeDeviceFindVportParentHost(devs);
}
return parent_host;
}
void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
{ {
size_t i = 0; size_t i = 0;

View File

@ -299,23 +299,8 @@ int virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
char **wwpn); char **wwpn);
int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
const char *dev_name, virNodeDeviceDefPtr def,
const char *parent_name, int create);
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);
void virNodeDeviceDefFree(virNodeDeviceDefPtr def); void virNodeDeviceDefFree(virNodeDeviceDefPtr def);

View File

@ -702,10 +702,7 @@ virNodeDeviceDefParseNode;
virNodeDeviceDefParseString; virNodeDeviceDefParseString;
virNodeDeviceFindByName; virNodeDeviceFindByName;
virNodeDeviceFindBySysfsPath; virNodeDeviceFindBySysfsPath;
virNodeDeviceFindVportParentHost;
virNodeDeviceGetParentHost; virNodeDeviceGetParentHost;
virNodeDeviceGetParentHostByFabricWWN;
virNodeDeviceGetParentHostByWWNs;
virNodeDeviceGetParentName; virNodeDeviceGetParentName;
virNodeDeviceGetWWNs; virNodeDeviceGetWWNs;
virNodeDeviceHasCap; virNodeDeviceHasCap;

View File

@ -581,8 +581,7 @@ nodeDeviceCreateXML(virConnectPtr conn,
nodeDeviceLock(); nodeDeviceLock();
def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type); if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type)))
if (def == NULL)
goto cleanup; goto cleanup;
if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0)
@ -591,30 +590,9 @@ nodeDeviceCreateXML(virConnectPtr conn,
if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
goto cleanup; goto cleanup;
if (def->parent) { if ((parent_host = virNodeDeviceGetParentHost(&driver->devs, def,
if (virNodeDeviceGetParentHost(&driver->devs, CREATE_DEVICE)) < 0)
def->name, goto cleanup;
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)
goto cleanup;
}
if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0)
goto cleanup; goto cleanup;
@ -642,7 +620,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
{ {
int ret = -1; int ret = -1;
virNodeDeviceObjPtr obj = NULL; virNodeDeviceObjPtr obj = NULL;
char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; virNodeDeviceDefPtr def;
char *wwnn = NULL, *wwpn = NULL;
int parent_host = -1; int parent_host = -1;
nodeDeviceLock(); nodeDeviceLock();
@ -659,32 +638,26 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) < 0) if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) < 0)
goto cleanup; goto cleanup;
/* virNodeDeviceGetParentHost will cause the device object's lock
/* virNodeDeviceGetParentHost will cause the device object's lock to be * to be taken, so grab the object def which will have the various
* taken, so we have to dup the parent's name and drop the lock * fields used to search (name, parent, parent_wwnn, parent_wwpn,
* before calling it. We don't need the reference to the object * or parent_fabric_wwn) and drop the object lock. */
* any more once we have the parent's name. */ def = obj->def;
if (VIR_STRDUP(parent_name, obj->def->parent) < 0) {
virNodeDeviceObjUnlock(obj);
obj = NULL;
goto cleanup;
}
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
obj = NULL; obj = NULL;
if ((parent_host = virNodeDeviceGetParentHost(&driver->devs, def,
if (virNodeDeviceGetParentHost(&driver->devs, dev->name, parent_name, EXISTING_DEVICE)) < 0)
&parent_host) < 0)
goto cleanup; goto cleanup;
if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0)
goto cleanup; goto cleanup;
ret = 0; ret = 0;
cleanup: cleanup:
nodeDeviceUnlock(); nodeDeviceUnlock();
if (obj) if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
VIR_FREE(parent_name);
VIR_FREE(wwnn); VIR_FREE(wwnn);
VIR_FREE(wwpn); VIR_FREE(wwpn);
return ret; return ret;

View File

@ -5712,7 +5712,6 @@ testNodeDeviceCreateXML(virConnectPtr conn,
testDriverPtr driver = conn->privateData; testDriverPtr driver = conn->privateData;
virNodeDeviceDefPtr def = NULL; virNodeDeviceDefPtr def = NULL;
char *wwnn = NULL, *wwpn = NULL; char *wwnn = NULL, *wwpn = NULL;
int parent_host = -1;
virNodeDevicePtr dev = NULL; virNodeDevicePtr dev = NULL;
virNodeDeviceDefPtr newdef = NULL; virNodeDeviceDefPtr newdef = NULL;
@ -5723,15 +5722,16 @@ testNodeDeviceCreateXML(virConnectPtr conn,
if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL))) if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL)))
goto cleanup; goto cleanup;
/* We run these next two simply for validation - they are essentially /* We run this simply for validation - it essentially validates that
* 'validating' that the input XML either has a wwnn/wwpn or the * the input XML either has a wwnn/wwpn or virNodeDevCapSCSIHostParseXML
* virNodeDevCapSCSIHostParseXML generated a wwnn/wwpn and that the * generated a wwnn/wwpn */
* input XML has a parent host defined. */
if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0)
goto cleanup; goto cleanup;
if (virNodeDeviceGetParentHost(&driver->devs, def->name, /* Unlike the "real" code we don't need the parent_host in order to
def->parent, &parent_host) < 0) * call virVHBAManageVport, but still let's make sure the code finds
* something valid and no one messed up the mock environment. */
if (virNodeDeviceGetParentHost(&driver->devs, def, CREATE_DEVICE) < 0)
goto cleanup; goto cleanup;
/* In the real code, we'd call virVHBAManageVport followed by /* In the real code, we'd call virVHBAManageVport followed by
@ -5762,7 +5762,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
testDriverPtr driver = dev->conn->privateData; testDriverPtr driver = dev->conn->privateData;
virNodeDeviceObjPtr obj = NULL; virNodeDeviceObjPtr obj = NULL;
char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL;
int parent_host = -1;
virObjectEventPtr event = NULL; virObjectEventPtr event = NULL;
testDriverLock(driver); testDriverLock(driver);
@ -5788,11 +5787,10 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
* any more once we have the parent's name. */ * any more once we have the parent's name. */
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
/* We do this just for basic validation */ /* We do this just for basic validation, but also avoid finding a
if (virNodeDeviceGetParentHost(&driver->devs, * vport capable HBA if for some reason our vHBA doesn't exist */
dev->name, if (virNodeDeviceGetParentHost(&driver->devs, obj->def,
parent_name, EXISTING_DEVICE) < 0) {
&parent_host) == -1) {
obj = NULL; obj = NULL;
goto out; goto out;
} }