On 07/03/2017 09:12 AM, Erik Skultety wrote: > On Sat, Jun 03, 2017 at 09:12:00AM -0400, John Ferlan wrote: >> Alter the nodeDeviceLookupSCSIHostByWWN to use the new API in order >> to find what it's looking for. >> > > Would you mind enhancing the commit message a bit? I assume the new API is > virNodeDeviceGetSCSIHostCaps but a random reader would lack the context. It > misses the main motivation to move nodeDeviceLookupSCSIHostByWWN out of the > driver simply because of the NodeDeviceObj privatization. Also, this got me > thinking, whether 9/10 and 10/10 is critical for this series, so I tried to > rewrite it without it and somewhere down the road I decided that it was an > ugly, pointless waste of time and I like it this way much more. > > ACK if you enhance the commit message a bit. > > Erik > Sure I'll add some details... My first hack at 9/12 wasn't well received either since I moved too much, see: https://www.redhat.com/archives/libvir-list/2017-May/msg00722.html and followups to that. So I took the move and call path of less resistance. The 10/12 change just follows other Lookup functions. It'll get even more interesting soon when you see patch 13. John >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> src/conf/virnodedeviceobj.c | 33 +++++++++++++++++++++ >> src/conf/virnodedeviceobj.h | 5 ++++ >> src/libvirt_private.syms | 1 + >> src/node_device/node_device_driver.c | 56 +++++++++++------------------------- >> 4 files changed, 55 insertions(+), 40 deletions(-) >> >> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c >> index a9187be..fa61a2d 100644 >> --- a/src/conf/virnodedeviceobj.c >> +++ b/src/conf/virnodedeviceobj.c >> @@ -274,6 +274,39 @@ virNodeDeviceObjListFindByCap(virNodeDeviceObjListPtr devs, >> } >> >> >> +virNodeDeviceObjPtr >> +virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs, >> + const char *wwnn, >> + const char *wwpn) >> +{ >> + size_t i; >> + >> + for (i = 0; i < devs->count; i++) { >> + virNodeDeviceObjPtr obj = devs->objs[i]; >> + virNodeDevCapsDefPtr cap; >> + >> + virNodeDeviceObjLock(obj); >> + cap = obj->def->caps; >> + >> + while (cap) { >> + if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { >> + virNodeDeviceGetSCSIHostCaps(&cap->data.scsi_host); >> + if (cap->data.scsi_host.flags & >> + VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { >> + if (STREQ(cap->data.scsi_host.wwnn, wwnn) && >> + STREQ(cap->data.scsi_host.wwpn, wwpn)) >> + return obj; >> + } >> + } >> + cap = cap->next; >> + } >> + virNodeDeviceObjUnlock(obj); >> + } >> + >> + return NULL; >> +} >> + >> + >> void >> virNodeDeviceObjFree(virNodeDeviceObjPtr obj) >> { >> diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h >> index 6194c6c..6ec5ee7 100644 >> --- a/src/conf/virnodedeviceobj.h >> +++ b/src/conf/virnodedeviceobj.h >> @@ -53,6 +53,11 @@ virNodeDeviceObjListFindBySysfsPath(virNodeDeviceObjListPtr devs, >> ATTRIBUTE_NONNULL(2); >> >> virNodeDeviceObjPtr >> +virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs, >> + const char *wwnn, >> + const char *wwpn); >> + >> +virNodeDeviceObjPtr >> virNodeDeviceObjListAssignDef(virNodeDeviceObjListPtr devs, >> virNodeDeviceDefPtr def); >> >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index 33fc9fc..d415888 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -963,6 +963,7 @@ virNodeDeviceObjListAssignDef; >> virNodeDeviceObjListExport; >> virNodeDeviceObjListFindByName; >> virNodeDeviceObjListFindBySysfsPath; >> +virNodeDeviceObjListFindSCSIHostByWWNs; >> virNodeDeviceObjListFree; >> virNodeDeviceObjListGetNames; >> virNodeDeviceObjListGetParentHost; >> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c >> index 348539f..4a5f168 100644 >> --- a/src/node_device/node_device_driver.c >> +++ b/src/node_device/node_device_driver.c >> @@ -288,9 +288,6 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, >> const char *wwpn, >> unsigned int flags) >> { >> - size_t i; >> - virNodeDeviceObjListPtr devs = driver->devs; >> - virNodeDevCapsDefPtr cap = NULL; >> virNodeDeviceObjPtr obj = NULL; >> virNodeDeviceDefPtr def; >> virNodeDevicePtr device = NULL; >> @@ -298,48 +295,27 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, >> virCheckFlags(0, NULL); >> >> nodeDeviceLock(); >> + obj = virNodeDeviceObjListFindSCSIHostByWWNs(driver->devs, wwnn, wwpn); >> + nodeDeviceUnlock(); >> >> - for (i = 0; i < devs->count; i++) { >> - obj = devs->objs[i]; >> - virNodeDeviceObjLock(obj); >> - def = virNodeDeviceObjGetDef(obj); >> - cap = def->caps; >> - >> - while (cap) { >> - if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { >> - nodeDeviceSysfsGetSCSIHostCaps(&cap->data.scsi_host); >> - if (cap->data.scsi_host.flags & >> - VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { >> - if (STREQ(cap->data.scsi_host.wwnn, wwnn) && >> - STREQ(cap->data.scsi_host.wwpn, wwpn)) { >> - >> - if (virNodeDeviceLookupSCSIHostByWWNEnsureACL(conn, def) < 0) >> - goto error; >> - >> - if ((device = virGetNodeDevice(conn, def->name))) { >> - if (VIR_STRDUP(device->parent, def->parent) < 0) { >> - virObjectUnref(device); >> - device = NULL; >> - } >> - } >> - virNodeDeviceObjUnlock(obj); >> - goto out; >> - } >> - } >> - } >> - cap = cap->next; >> - } >> + if (!obj) >> + return NULL; >> >> - virNodeDeviceObjUnlock(obj); >> - } >> + def = virNodeDeviceObjGetDef(obj); >> >> - out: >> - nodeDeviceUnlock(); >> - return device; >> + if (virNodeDeviceLookupSCSIHostByWWNEnsureACL(conn, def) < 0) >> + goto cleanup; >> >> - error: >> + if ((device = virGetNodeDevice(conn, def->name))) { >> + if (VIR_STRDUP(device->parent, def->parent) < 0) { >> + virObjectUnref(device); >> + device = NULL; >> + } >> + } >> + >> + cleanup: >> virNodeDeviceObjUnlock(obj); >> - goto out; >> + return device; >> } >> >> >> -- >> 2.9.4 >> >> -- >> libvir-list mailing list >> libvir-list@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list