3:54 p.m.
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(a)redhat.com>
> ---
> 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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list