[libvirt] [PATCH v3 10/12] nodedev: Introduce virNodeDeviceObjListFindSCSIHostByWWNs

John Ferlan posted 12 patches 7 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH v3 10/12] nodedev: Introduce virNodeDeviceObjListFindSCSIHostByWWNs
Posted by John Ferlan 7 years, 11 months ago
Alter the nodeDeviceLookupSCSIHostByWWN to use the new API in order
to find what it's looking for.

Signed-off-by: John Ferlan <jferlan@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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/12] nodedev: Introduce virNodeDeviceObjListFindSCSIHostByWWNs
Posted by Erik Skultety 7 years, 10 months ago
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

> Signed-off-by: John Ferlan <jferlan@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@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/12] nodedev: Introduce virNodeDeviceObjListFindSCSIHostByWWNs
Posted by John Ferlan 7 years, 10 months ago

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@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@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list