nodedev: Alter virNodeDeviceObjRemove

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
and how to handle the logic afterwards. This includes free'ing the object
and/or setting the local variable to NULL to prevent subsequent unexpected
usage (via something like virNodeDeviceObjRemove in testNodeDeviceDestroy).

For now this function will just handle the remove of the object from the
list for which it was placed during virNodeDeviceObjAssignDef.

This essentially reverts logic from commit id '61148074' that free'd the
device entry on list, set *dev = NULL and returned. Thus fixing a bug in
node_device_hal.c/dev_refresh() which would never call dev_create(udi)
since @dev would have been set to NULL.

Signed-off-by: John Ferlan <jferlan@redhat.com>

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]);
     }
 }
 

src/conf/virnodedeviceobj.h

@@ -58,7 +58,7 @@ virNodeDeviceObjAssignDef(virNodeDeviceObjListPtr devs,
 
 void
 virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                       virNodeDeviceObjPtr *dev);
+                       virNodeDeviceObjPtr dev);
 
 int
 virNodeDeviceObjGetParentHost(virNodeDeviceObjListPtr devs,

src/libvirt_private.syms

@@ -966,6 +966,7 @@ virNetworkObjUpdateAssignDef;
 virNodeDeviceObjAssignDef;
 virNodeDeviceObjFindByName;
 virNodeDeviceObjFindBySysfsPath;
+virNodeDeviceObjFree;
 virNodeDeviceObjGetDef;
 virNodeDeviceObjGetNames;
 virNodeDeviceObjGetParentHost;

src/node_device/node_device_hal.c

@@ -514,14 +514,16 @@ 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);
     }
     nodeDeviceUnlock();
 
-    if (dev)
+    if (dev) {
+        virNodeDeviceObjFree(dev);
         dev_create(udi);
+    }
 }
 
 static void
@@ -544,10 +546,11 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
     dev = virNodeDeviceObjFindByName(&driver->devs, name);
     VIR_DEBUG("%s", name);
     if (dev)
-        virNodeDeviceObjRemove(&driver->devs, &dev);
+        virNodeDeviceObjRemove(&driver->devs, dev);
     else
         VIR_DEBUG("no device named %s", name);
     nodeDeviceUnlock();
+    virNodeDeviceObjFree(dev);
 }
 
 

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);

src/test/test_driver.c

@@ -4571,7 +4571,9 @@ testDestroyVport(testDriverPtr privconn,
                                            VIR_NODE_DEVICE_EVENT_DELETED,
                                            0);
 
-    virNodeDeviceObjRemove(&privconn->devs, &obj);
+    virNodeDeviceObjRemove(&privconn->devs, obj);
+    virNodeDeviceObjFree(obj);
+    obj = NULL;
 
     ret = 0;
 
@@ -5665,7 +5667,9 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
                                            0);
 
     virNodeDeviceObjLock(obj);
-    virNodeDeviceObjRemove(&driver->devs, &obj);
+    virNodeDeviceObjRemove(&driver->devs, obj);
+    virNodeDeviceObjFree(obj);
+    obj = NULL;
 
  out:
     if (obj)