nodedev: Remove driver locks around object list mgmt code

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.

This includes the test driver as well.

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 920d8776407c20ffd713f72b528e66a728ea38f3..facfeb695755eda2696387fc0f7907ddff7a78c1 100644 (file)
--- 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;
 
@@ -579,7 +544,6 @@ nodeDeviceCreateXML(virConnectPtr conn,
                          "wwnn '%s' and wwpn '%s'"),
                        def->name, wwnn, wwpn);
  cleanup:
-    nodeDeviceUnlock();
     virNodeDeviceDefFree(def);
     VIR_FREE(wwnn);
     VIR_FREE(wwpn);
@@ -601,8 +565,6 @@ nodeDeviceDestroy(virNodeDevicePtr device)
         return -1;
     def = virNodeDeviceObjGetDef(obj);
 
-    nodeDeviceLock();
-
     if (virNodeDeviceDestroyEnsureACL(device->conn, def) < 0)
         goto cleanup;
 
@@ -634,7 +596,6 @@ nodeDeviceDestroy(virNodeDevicePtr device)
     ret = 0;
 
  cleanup:
-    nodeDeviceUnlock();
     virNodeDeviceObjEndAPI(&obj);
     VIR_FREE(parent);
     VIR_FREE(wwnn);

diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index 7f246f0564e0e73ad64038e8ce5e282450448796..c19e327c9693d0ea2b914a2cb9fbb9b72e25ea35 100644 (file)
--- 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 1b10c16ee2353b255796a5ffa1a07dd349306b0b..4762f19699f244d51636c736187be7c0c5dedb29 100644 (file)
--- 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;
@@ -1846,19 +1843,18 @@ nodeStateInitialize(bool privileged,
     if (udevSetupSystemDev() != 0)
         goto cleanup;
 
-    /* Populate with known devices */
+    nodeDeviceUnlock();
 
+    /* Populate with known devices */
     if (udevEnumerateDevices(udev) != 0)
         goto cleanup;
 
-    ret = 0;
+    return 0;
 
  cleanup:
     nodeDeviceUnlock();
-
-    if (ret == -1)
-        nodeStateCleanup();
-    return ret;
+    nodeStateCleanup();
+    return -1;
 }
 
 

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 94169c130ec84fca6c7d6ff0ad539305e1c2770d..1de30a040c297266b19b2cb27ff0a8d515e0734f 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5200,11 +5200,7 @@ testNodeDeviceObjFindByName(testDriverPtr driver,
 {
     virNodeDeviceObjPtr obj;
 
-    testDriverLock(driver);
-    obj = virNodeDeviceObjListFindByName(driver->devs, name);
-    testDriverUnlock(driver);
-
-    if (!obj)
+    if (!(obj = virNodeDeviceObjListFindByName(driver->devs, name)))
         virReportError(VIR_ERR_NO_NODE_DEVICE,
                        _("no node device with matching name '%s'"),
                        name);
@@ -5219,15 +5215,10 @@ testNodeNumOfDevices(virConnectPtr conn,
                      unsigned int flags)
 {
     testDriverPtr driver = conn->privateData;
-    int ndevs = 0;
 
     virCheckFlags(0, -1);
 
-    testDriverLock(driver);
-    ndevs = virNodeDeviceObjListNumOfDevices(driver->devs, conn, cap, NULL);
-    testDriverUnlock(driver);
-
-    return ndevs;
+    return virNodeDeviceObjListNumOfDevices(driver->devs, conn, cap, NULL);
 }
 
 
@@ -5239,16 +5230,11 @@ testNodeListDevices(virConnectPtr conn,
                     unsigned int flags)
 {
     testDriverPtr driver = conn->privateData;
-    int nnames = 0;
 
     virCheckFlags(0, -1);
 
-    testDriverLock(driver);
-    nnames = virNodeDeviceObjListGetNames(driver->devs, conn, NULL,
-                                          cap, names, maxnames);
-    testDriverUnlock(driver);
-
-    return nnames;
+    return virNodeDeviceObjListGetNames(driver->devs, conn, NULL,
+                                        cap, names, maxnames);
 }
 
 
@@ -5463,8 +5449,6 @@ testNodeDeviceCreateXML(virConnectPtr conn,
 
     virCheckFlags(0, NULL);
 
-    testDriverLock(driver);
-
     if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL)))
         goto cleanup;
 
@@ -5501,7 +5485,6 @@ testNodeDeviceCreateXML(virConnectPtr conn,
 
  cleanup:
     virNodeDeviceObjEndAPI(&obj);
-    testDriverUnlock(driver);
     virNodeDeviceDefFree(def);
     virObjectUnref(dev);
     VIR_FREE(wwnn);