[PATCH v3 01/12] nodedev: Alter virNodeDeviceObjRemove

[John Ferlan <jferlan@xxxxxxxxxx>]

Rather than passing the object to be removed by reference, pass by value
and then let the caller decide whether or not the object should be free'd.
This function should just handle the remove of the object from the list
for which it was placed during virNodeDeviceObjAssignDef.

One caller in node_device_hal would fail to go through the dev_create path
since the @dev would have been NULL after returning from the Remove API.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/conf/virnodedeviceobj.c        | 14 ++++++--------
 src/conf/virnodedeviceobj.h        |  2 +-
 src/libvirt_private.syms           |  1 +
 src/node_device/node_device_hal.c  | 10 ++++++----
 src/node_device/node_device_udev.c |  3 ++-
 src/test/test_driver.c             |  8 ++++++--
 6 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index e78f451..fa73de1 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -301,23 +301,21 @@ virNodeDeviceObjAssignDef(virNodeDeviceObjListPtr devs,
 
 void
 virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                       virNodeDeviceObjPtr *dev)
+                       virNodeDeviceObjPtr dev)
 {
     size_t i;
 
-    virNodeDeviceObjUnlock(*dev);
+    virNodeDeviceObjUnlock(dev);
 
     for (i = 0; i < devs->count; i++) {
-        virNodeDeviceObjLock(*dev);
-        if (devs->objs[i] == *dev) {
-            virNodeDeviceObjUnlock(*dev);
-            virNodeDeviceObjFree(devs->objs[i]);
-            *dev = NULL;
+        virNodeDeviceObjLock(devs->objs[i]);
+        if (devs->objs[i] == dev) {
+            virNodeDeviceObjUnlock(devs->objs[i]);
 
             VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
             break;
         }
-        virNodeDeviceObjUnlock(*dev);
+        virNodeDeviceObjUnlock(devs->objs[i]);
     }
 }
 
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 05a9d11..9bc02ee 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -58,7 +58,7 @@ virNodeDeviceObjAssignDef(virNodeDeviceObjListPtr devs,
 
 void
 virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                       virNodeDeviceObjPtr *dev);
+                       virNodeDeviceObjPtr dev);
 
 int
 virNodeDeviceObjGetParentHost(virNodeDeviceObjListPtr devs,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 683a232..4a10508 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -959,6 +959,7 @@ virNetworkObjUpdateAssignDef;
 virNodeDeviceObjAssignDef;
 virNodeDeviceObjFindByName;
 virNodeDeviceObjFindBySysfsPath;
+virNodeDeviceObjFree;
 virNodeDeviceObjGetDef;
 virNodeDeviceObjGetNames;
 virNodeDeviceObjGetParentHost;
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index f468e42..c354cd3 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -514,7 +514,7 @@ dev_refresh(const char *udi)
         /* Simply "rediscover" device -- incrementally handling changes
          * to sub-capabilities (like net.80203) is nasty ... so avoid it.
          */
-        virNodeDeviceObjRemove(&driver->devs, &dev);
+        virNodeDeviceObjRemove(&driver->devs, dev);
     } else {
         VIR_DEBUG("no device named %s", name);
     }
@@ -543,10 +543,12 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
     nodeDeviceLock();
     dev = virNodeDeviceObjFindByName(&driver->devs, name);
     VIR_DEBUG("%s", name);
-    if (dev)
-        virNodeDeviceObjRemove(&driver->devs, &dev);
-    else
+    if (dev) {
+        virNodeDeviceObjRemove(&driver->devs, dev);
+        virNodeDeviceObjFree(dev);
+    } else {
         VIR_DEBUG("no device named %s", name);
+    }
     nodeDeviceUnlock();
 }
 
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 174124a..819e4e7 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1333,7 +1333,8 @@ udevRemoveOneDevice(struct udev_device *device)
 
     VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
               def->name, name);
-    virNodeDeviceObjRemove(&driver->devs, &dev);
+    virNodeDeviceObjRemove(&driver->devs, dev);
+    virNodeDeviceObjFree(dev);
 
     if (event)
         virObjectEventStateQueue(driver->nodeDeviceEventState, event);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e5938f5..e323619 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4530,7 +4530,9 @@ testDestroyVport(testDriverPtr privconn,
                                            VIR_NODE_DEVICE_EVENT_DELETED,
                                            0);
 
-    virNodeDeviceObjRemove(&privconn->devs, &obj);
+    virNodeDeviceObjRemove(&privconn->devs, obj);
+    virNodeDeviceObjFree(obj);
+    obj = NULL;
 
     ret = 0;
 
@@ -5624,7 +5626,9 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
                                            0);
 
     virNodeDeviceObjLock(obj);
-    virNodeDeviceObjRemove(&driver->devs, &obj);
+    virNodeDeviceObjRemove(&driver->devs, obj);
+    virNodeDeviceObjFree(obj);
+    obj = NULL;
 
  out:
     if (obj)
-- 
2.9.4

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