diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index bc976d0e47..31ca45646c 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1975,17 +1975,15 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, */ /* virNodeDeviceFindFCParentHost: * @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 * in order to set the parent_host value. * * Returns: - * 0 on success with parent_host set, -1 otherwise; + * parent_host value on success (>= 0), -1 otherwise. */ static int -virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent, - int *parent_host) +virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent) { virNodeDevCapsDefPtr cap = virNodeDeviceFindVPORTCapDef(parent); @@ -1997,16 +1995,14 @@ virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent, return -1; } - *parent_host = cap->data.scsi_host.host; - return 0; + return cap->data.scsi_host.host; } -int -virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, - const char *dev_name, - const char *parent_name, - int *parent_host) +static int +virNodeDeviceGetParentHostByParent(virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name) { virNodeDeviceObjPtr parent = NULL; int ret; @@ -2018,7 +2014,7 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -2026,12 +2022,11 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, } -int +static int virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, const char *dev_name, const char *parent_wwnn, - const char *parent_wwpn, - int *parent_host) + const char *parent_wwpn) { virNodeDeviceObjPtr parent = NULL; int ret; @@ -2043,7 +2038,7 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -2051,11 +2046,10 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, } -int +static int virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, const char *dev_name, - const char *parent_fabric_wwn, - int *parent_host) + const char *parent_fabric_wwn) { virNodeDeviceObjPtr parent = NULL; int ret; @@ -2067,7 +2061,7 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -2075,9 +2069,8 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, } -int -virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, - int *parent_host) +static int +virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs) { virNodeDeviceObjPtr parent = NULL; const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS); @@ -2089,7 +2082,7 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(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) { size_t i = 0; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 6c43546926..8213c27a1d 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -299,23 +299,8 @@ int virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, char **wwpn); int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, - const char *dev_name, - 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); + virNodeDeviceDefPtr def, + int create); void virNodeDeviceDefFree(virNodeDeviceDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c90093a710..e6ccd697d2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -702,10 +702,7 @@ virNodeDeviceDefParseNode; virNodeDeviceDefParseString; virNodeDeviceFindByName; virNodeDeviceFindBySysfsPath; -virNodeDeviceFindVportParentHost; virNodeDeviceGetParentHost; -virNodeDeviceGetParentHostByFabricWWN; -virNodeDeviceGetParentHostByWWNs; virNodeDeviceGetParentName; virNodeDeviceGetWWNs; virNodeDeviceHasCap; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 32022ebca9..5d57006f2c 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -581,8 +581,7 @@ nodeDeviceCreateXML(virConnectPtr conn, nodeDeviceLock(); - def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type); - if (def == NULL) + if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type))) goto cleanup; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) @@ -591,30 +590,9 @@ nodeDeviceCreateXML(virConnectPtr conn, if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) goto cleanup; - if (def->parent) { - if (virNodeDeviceGetParentHost(&driver->devs, - def->name, - 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 ((parent_host = virNodeDeviceGetParentHost(&driver->devs, def, + CREATE_DEVICE)) < 0) + goto cleanup; if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) goto cleanup; @@ -642,7 +620,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev) { int ret = -1; virNodeDeviceObjPtr obj = NULL; - char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; + virNodeDeviceDefPtr def; + char *wwnn = NULL, *wwpn = NULL; int parent_host = -1; nodeDeviceLock(); @@ -659,32 +638,26 @@ nodeDeviceDestroy(virNodeDevicePtr dev) if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) < 0) goto cleanup; - - /* virNodeDeviceGetParentHost will cause the device object's lock to be - * taken, so we have to dup the parent's name and drop the lock - * before calling it. We don't need the reference to the object - * any more once we have the parent's name. */ - if (VIR_STRDUP(parent_name, obj->def->parent) < 0) { - virNodeDeviceObjUnlock(obj); - obj = NULL; - goto cleanup; - } + /* virNodeDeviceGetParentHost will cause the device object's lock + * to be taken, so grab the object def which will have the various + * fields used to search (name, parent, parent_wwnn, parent_wwpn, + * or parent_fabric_wwn) and drop the object lock. */ + def = obj->def; virNodeDeviceObjUnlock(obj); obj = NULL; - - if (virNodeDeviceGetParentHost(&driver->devs, dev->name, parent_name, - &parent_host) < 0) + if ((parent_host = virNodeDeviceGetParentHost(&driver->devs, def, + EXISTING_DEVICE)) < 0) goto cleanup; if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) goto cleanup; ret = 0; + cleanup: nodeDeviceUnlock(); if (obj) virNodeDeviceObjUnlock(obj); - VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); return ret; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index acc4d340d9..5fef3f10b9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5712,7 +5712,6 @@ testNodeDeviceCreateXML(virConnectPtr conn, testDriverPtr driver = conn->privateData; virNodeDeviceDefPtr def = NULL; char *wwnn = NULL, *wwpn = NULL; - int parent_host = -1; virNodeDevicePtr dev = NULL; virNodeDeviceDefPtr newdef = NULL; @@ -5723,15 +5722,16 @@ testNodeDeviceCreateXML(virConnectPtr conn, if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL))) goto cleanup; - /* We run these next two simply for validation - they are essentially - * 'validating' that the input XML either has a wwnn/wwpn or the - * virNodeDevCapSCSIHostParseXML generated a wwnn/wwpn and that the - * input XML has a parent host defined. */ + /* We run this simply for validation - it essentially validates that + * the input XML either has a wwnn/wwpn or virNodeDevCapSCSIHostParseXML + * generated a wwnn/wwpn */ if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) goto cleanup; - if (virNodeDeviceGetParentHost(&driver->devs, def->name, - def->parent, &parent_host) < 0) + /* Unlike the "real" code we don't need the parent_host in order to + * 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; /* In the real code, we'd call virVHBAManageVport followed by @@ -5762,7 +5762,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) testDriverPtr driver = dev->conn->privateData; virNodeDeviceObjPtr obj = NULL; char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; - int parent_host = -1; virObjectEventPtr event = NULL; testDriverLock(driver); @@ -5788,11 +5787,10 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) * any more once we have the parent's name. */ virNodeDeviceObjUnlock(obj); - /* We do this just for basic validation */ - if (virNodeDeviceGetParentHost(&driver->devs, - dev->name, - parent_name, - &parent_host) == -1) { + /* We do this just for basic validation, but also avoid finding a + * vport capable HBA if for some reason our vHBA doesn't exist */ + if (virNodeDeviceGetParentHost(&driver->devs, obj->def, + EXISTING_DEVICE) < 0) { obj = NULL; goto out; }