[libvirt] [PATCH v5 3/3] nodedev: Remove driver locks around object list mgmt code

John Ferlan jferlan at redhat.com
Mon Jul 17 15:02:24 UTC 2017


Since virnodedeviceobj now has a self-lockable hash table, there's no
need to lock the table from the driver for processing. Thus remove the
locks from the driver for NodeDeviceObjList mgmt.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/node_device/node_device_driver.c | 61 +++++++-----------------------------
 src/node_device/node_device_hal.c    | 16 ++--------
 src/node_device/node_device_udev.c   | 13 +++-----
 3 files changed, 18 insertions(+), 72 deletions(-)

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 1f888fb..1e6709d 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -174,19 +174,13 @@ nodeNumOfDevices(virConnectPtr conn,
                  const char *cap,
                  unsigned int flags)
 {
-    int ndevs = 0;
-
     if (virNodeNumOfDevicesEnsureACL(conn) < 0)
         return -1;
 
     virCheckFlags(0, -1);
 
-    nodeDeviceLock();
-    ndevs = virNodeDeviceObjListNumOfDevices(driver->devs, conn, cap,
-                                             virNodeNumOfDevicesCheckACL);
-    nodeDeviceUnlock();
-
-    return ndevs;
+    return virNodeDeviceObjListNumOfDevices(driver->devs, conn, cap,
+                                            virNodeNumOfDevicesCheckACL);
 }
 
 
@@ -197,20 +191,14 @@ nodeListDevices(virConnectPtr conn,
                 int maxnames,
                 unsigned int flags)
 {
-    int nnames;
-
     if (virNodeListDevicesEnsureACL(conn) < 0)
         return -1;
 
     virCheckFlags(0, -1);
 
-    nodeDeviceLock();
-    nnames = virNodeDeviceObjListGetNames(driver->devs, conn,
-                                          virNodeListDevicesCheckACL,
-                                          cap, names, maxnames);
-    nodeDeviceUnlock();
-
-    return nnames;
+    return virNodeDeviceObjListGetNames(driver->devs, conn,
+                                        virNodeListDevicesCheckACL,
+                                        cap, names, maxnames);
 }
 
 
@@ -219,19 +207,14 @@ nodeConnectListAllNodeDevices(virConnectPtr conn,
                               virNodeDevicePtr **devices,
                               unsigned int flags)
 {
-    int ret = -1;
-
     virCheckFlags(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP, -1);
 
     if (virConnectListAllNodeDevicesEnsureACL(conn) < 0)
         return -1;
 
-    nodeDeviceLock();
-    ret = virNodeDeviceObjListExport(conn, driver->devs, devices,
-                                     virConnectListAllNodeDevicesCheckACL,
-                                     flags);
-    nodeDeviceUnlock();
-    return ret;
+    return virNodeDeviceObjListExport(conn, driver->devs, devices,
+                                      virConnectListAllNodeDevicesCheckACL,
+                                      flags);
 }
 
 
@@ -240,11 +223,7 @@ nodeDeviceObjFindByName(const char *name)
 {
     virNodeDeviceObjPtr obj;
 
-    nodeDeviceLock();
-    obj = virNodeDeviceObjListFindByName(driver->devs, name);
-    nodeDeviceUnlock();
-
-    if (!obj) {
+    if (!(obj = virNodeDeviceObjListFindByName(driver->devs, name))) {
         virReportError(VIR_ERR_NO_NODE_DEVICE,
                        _("no node device with matching name '%s'"),
                        name);
@@ -294,11 +273,8 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
 
     virCheckFlags(0, NULL);
 
-    nodeDeviceLock();
-    obj = virNodeDeviceObjListFindSCSIHostByWWNs(driver->devs, wwnn, wwpn);
-    nodeDeviceUnlock();
-
-    if (!obj)
+    if (!(obj = virNodeDeviceObjListFindSCSIHostByWWNs(driver->devs,
+                                                       wwnn, wwpn)))
         return NULL;
 
     def = virNodeDeviceObjGetDef(obj);
@@ -509,13 +485,6 @@ nodeDeviceFindNewDevice(virConnectPtr conn,
     virNodeDevicePtr device = NULL;
     time_t start = 0, now = 0;
 
-    /* The thread that creates the device takes the driver lock, so we
-     * must release it in order to allow the device to be created.
-     * We're not doing anything with the driver pointer at this point,
-     * so it's safe to release it, assuming that the pointer itself
-     * doesn't become invalid.  */
-    nodeDeviceUnlock();
-
     nodeDeviceGetTime(&start);
 
     while ((now - start) < LINUX_NEW_DEVICE_WAIT_TIME) {
@@ -532,8 +501,6 @@ nodeDeviceFindNewDevice(virConnectPtr conn,
             break;
     }
 
-    nodeDeviceLock();
-
     return device;
 }
 
@@ -552,8 +519,6 @@ nodeDeviceCreateXML(virConnectPtr conn,
     virCheckFlags(0, NULL);
     virt_type  = virConnectGetType(conn);
 
-    nodeDeviceLock();
-
     if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type)))
         goto cleanup;
 
@@ -586,7 +551,6 @@ nodeDeviceCreateXML(virConnectPtr conn,
                          "wwnn '%s' and wwpn '%s'"),
                        def->name, wwnn, wwpn);
  cleanup:
-    nodeDeviceUnlock();
     virNodeDeviceDefFree(def);
     VIR_FREE(wwnn);
     VIR_FREE(wwpn);
@@ -612,8 +576,6 @@ nodeDeviceDestroy(virNodeDevicePtr device)
         return -1;
     def = virNodeDeviceObjGetDef(obj);
 
-    nodeDeviceLock();
-
     if (virNodeDeviceDestroyEnsureACL(device->conn, def) < 0)
         goto cleanup;
 
@@ -647,7 +609,6 @@ nodeDeviceDestroy(virNodeDevicePtr device)
     ret = 0;
 
  cleanup:
-    nodeDeviceUnlock();
     virNodeDeviceObjEndAPI(&obj);
     VIR_FREE(name);
     VIR_FREE(parent);
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index b220798..8731e3b 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -508,20 +508,15 @@ dev_refresh(const char *udi)
     const char *name = hal_name(udi);
     virNodeDeviceObjPtr obj;
 
-    nodeDeviceLock();
     if ((obj = virNodeDeviceObjListFindByName(driver->devs, name))) {
         /* Simply "rediscover" device -- incrementally handling changes
          * to sub-capabilities (like net.80203) is nasty ... so avoid it.
          */
         virNodeDeviceObjListRemove(driver->devs, obj);
-    } else {
-        VIR_DEBUG("no device named %s", name);
-    }
-    nodeDeviceUnlock();
-
-    if (obj) {
         virObjectUnref(obj);
         dev_create(udi);
+    } else {
+        VIR_DEBUG("no device named %s", name);
     }
 }
 
@@ -541,14 +536,12 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
     const char *name = hal_name(udi);
     virNodeDeviceObjPtr obj;
 
-    nodeDeviceLock();
     obj = virNodeDeviceObjListFindByName(driver->devs, name);
     VIR_DEBUG("%s", name);
     if (obj)
         virNodeDeviceObjListRemove(driver->devs, obj);
     else
         VIR_DEBUG("no device named %s", name);
-    nodeDeviceUnlock();
     virObjectUnref(obj);
 }
 
@@ -561,11 +554,8 @@ device_cap_added(LibHalContext *ctx,
     virNodeDeviceObjPtr obj;
     virNodeDeviceDefPtr def;
 
-    nodeDeviceLock();
-    obj = virNodeDeviceObjListFindByName(driver->devs, name);
-    nodeDeviceUnlock();
     VIR_DEBUG("%s %s", cap, name);
-    if (obj) {
+    if ((obj = virNodeDeviceObjListFindByName(driver->devs, name))) {
         def = virNodeDeviceObjGetDef(obj);
         (void)gather_capability(ctx, udi, cap, &def->caps);
         virNodeDeviceObjEndAPI(&obj);
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 1b10c16..434efd7 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1607,7 +1607,6 @@ udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
     const char *action = NULL;
     int udev_fd = -1;
 
-    nodeDeviceLock();
     udev_fd = udev_monitor_get_fd(udev_monitor);
     if (fd != udev_fd) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1639,7 +1638,6 @@ udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
 
  cleanup:
     udev_device_unref(device);
-    nodeDeviceUnlock();
     return;
 }
 
@@ -1767,7 +1765,6 @@ nodeStateInitialize(bool privileged,
 {
     udevPrivate *priv = NULL;
     struct udev *udev = NULL;
-    int ret = -1;
 
     if (VIR_ALLOC(priv) < 0)
         return -1;
@@ -1847,18 +1844,16 @@ nodeStateInitialize(bool privileged,
         goto cleanup;
 
     /* Populate with known devices */
-
+    nodeDeviceUnlock();
     if (udevEnumerateDevices(udev) != 0)
         goto cleanup;
 
-    ret = 0;
+    return 0;
 
  cleanup:
     nodeDeviceUnlock();
-
-    if (ret == -1)
-        nodeStateCleanup();
-    return ret;
+    nodeStateCleanup();
+    return -1;
 }
 
 
-- 
2.9.4




More information about the libvir-list mailing list