Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.

This also involves creating a virNodeDeviceEndAPI in order to handle
the object cleaup for API's that use the Add or Find API's in order
to get a locked/reffed object. The EndAPI will unlock and unref the
object returning NULL to indicate to the caller to not use the obj.

For now this also involves a forward reference to the Unlock, but
that'll be removed shortly when the object is convert to use the
virObjectLockable shortly.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/virnodedeviceobj.c          | 156 ++++++++++++++++++-----------------
 src/conf/virnodedeviceobj.h          |  11 +--
 src/libvirt_private.syms             |   4 +-
 src/node_device/node_device_driver.c |  17 ++--
 src/node_device/node_device_hal.c    |   6 +-
 src/node_device/node_device_udev.c   |  12 +--
 src/test/test_driver.c               |  40 ++++-----
 7 files changed, 122 insertions(+), 124 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index c19f1e4..3787b23 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -33,7 +33,7 @@
 VIR_LOG_INIT("conf.virnodedeviceobj");
 
 struct _virNodeDeviceObj {
-    virMutex lock;
+    virObjectLockable parent;
 
     virNodeDeviceDefPtr def;		/* device definition */
 };
@@ -44,27 +44,63 @@ struct _virNodeDeviceObjList {
 };
 
 
+static virClassPtr virNodeDeviceObjClass;
+static void virNodeDeviceObjDispose(void *opaque);
+
+static int
+virNodeDeviceObjOnceInit(void)
+{
+    if (!(virNodeDeviceObjClass = virClassNew(virClassForObjectLockable(),
+                                              "virNodeDeviceObj",
+                                              sizeof(virNodeDeviceObj),
+                                              virNodeDeviceObjDispose)))
+        return -1;
+
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virNodeDeviceObj)
+
+
+static void
+virNodeDeviceObjDispose(void *opaque)
+{
+    virNodeDeviceObjPtr obj = opaque;
+
+    virNodeDeviceDefFree(obj->def);
+}
+
+
 static virNodeDeviceObjPtr
 virNodeDeviceObjNew(virNodeDeviceDefPtr def)
 {
     virNodeDeviceObjPtr obj;
 
-    if (VIR_ALLOC(obj) < 0)
+    if (virNodeDeviceObjInitialize() < 0)
         return NULL;
 
-    if (virMutexInit(&obj->lock) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("cannot initialize mutex"));
-        VIR_FREE(obj);
+    if (!(obj = virObjectLockableNew(virNodeDeviceObjClass)))
         return NULL;
-    }
-    virNodeDeviceObjLock(obj);
+
+    virObjectLock(obj);
     obj->def = def;
 
     return obj;
 }
 
 
+void
+virNodeDeviceObjEndAPI(virNodeDeviceObjPtr *obj)
+{
+    if (!*obj)
+        return;
+
+    virObjectUnlock(*obj);
+    virObjectUnref(*obj);
+    *obj = NULL;
+}
+
+
 virNodeDeviceDefPtr
 virNodeDeviceObjGetDef(virNodeDeviceObjPtr obj)
 {
@@ -186,13 +222,13 @@ virNodeDeviceObjListFindBySysfsPath(virNodeDeviceObjListPtr devs,
         virNodeDeviceObjPtr obj = devs->objs[i];
         virNodeDeviceDefPtr def;
 
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         def = obj->def;
         if ((def->sysfs_path != NULL) &&
             (STREQ(def->sysfs_path, sysfs_path))) {
-            return obj;
+            return virObjectRef(obj);
         }
-        virNodeDeviceObjUnlock(obj);
+        virObjectUnlock(obj);
     }
 
     return NULL;
@@ -209,11 +245,11 @@ virNodeDeviceObjListFindByName(virNodeDeviceObjListPtr devs,
         virNodeDeviceObjPtr obj = devs->objs[i];
         virNodeDeviceDefPtr def;
 
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         def = obj->def;
         if (STREQ(def->name, name))
-            return obj;
-        virNodeDeviceObjUnlock(obj);
+            return virObjectRef(obj);
+        virObjectUnlock(obj);
     }
 
     return NULL;
@@ -231,13 +267,13 @@ virNodeDeviceObjListFindByWWNs(virNodeDeviceObjListPtr devs,
         virNodeDeviceObjPtr obj = devs->objs[i];
         virNodeDevCapsDefPtr cap;
 
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         if ((cap = virNodeDeviceFindFCCapDef(obj)) &&
             STREQ_NULLABLE(cap->data.scsi_host.wwnn, parent_wwnn) &&
             STREQ_NULLABLE(cap->data.scsi_host.wwpn, parent_wwpn) &&
             virNodeDeviceFindVPORTCapDef(obj))
-            return obj;
-        virNodeDeviceObjUnlock(obj);
+            return virObjectRef(obj);
+        virObjectUnlock(obj);
     }
 
     return NULL;
@@ -254,12 +290,12 @@ virNodeDeviceObjListFindByFabricWWN(virNodeDeviceObjListPtr devs,
         virNodeDeviceObjPtr obj = devs->objs[i];
         virNodeDevCapsDefPtr cap;
 
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         if ((cap = virNodeDeviceFindFCCapDef(obj)) &&
             STREQ_NULLABLE(cap->data.scsi_host.fabric_wwn, parent_fabric_wwn) &&
             virNodeDeviceFindVPORTCapDef(obj))
-            return obj;
-        virNodeDeviceObjUnlock(obj);
+            return virObjectRef(obj);
+        virObjectUnlock(obj);
     }
 
     return NULL;
@@ -275,10 +311,10 @@ virNodeDeviceObjListFindByCap(virNodeDeviceObjListPtr devs,
     for (i = 0; i < devs->count; i++) {
         virNodeDeviceObjPtr obj = devs->objs[i];
 
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         if (virNodeDeviceObjHasCap(obj, cap))
-            return obj;
-        virNodeDeviceObjUnlock(obj);
+            return virObjectRef(obj);
+        virObjectUnlock(obj);
     }
 
     return NULL;
@@ -296,7 +332,7 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs,
         virNodeDeviceObjPtr obj = devs->objs[i];
         virNodeDevCapsDefPtr cap;
 
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         cap = obj->def->caps;
 
         while (cap) {
@@ -306,32 +342,18 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs,
                     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;
+                        return virObjectRef(obj);
                 }
             }
             cap = cap->next;
         }
-        virNodeDeviceObjUnlock(obj);
+        virObjectUnlock(obj);
     }
 
     return NULL;
 }
 
 
-void
-virNodeDeviceObjFree(virNodeDeviceObjPtr obj)
-{
-    if (!obj)
-        return;
-
-    virNodeDeviceDefFree(obj->def);
-
-    virMutexDestroy(&obj->lock);
-
-    VIR_FREE(obj);
-}
-
-
 virNodeDeviceObjListPtr
 virNodeDeviceObjListNew(void)
 {
@@ -348,7 +370,7 @@ virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs)
 {
     size_t i;
     for (i = 0; i < devs->count; i++)
-        virNodeDeviceObjFree(devs->objs[i]);
+        virObjectUnref(devs->objs[i]);
     VIR_FREE(devs->objs);
     VIR_FREE(devs);
 }
@@ -371,12 +393,11 @@ virNodeDeviceObjListAssignDef(virNodeDeviceObjListPtr devs,
 
     if (VIR_APPEND_ELEMENT_COPY(devs->objs, devs->count, obj) < 0) {
         obj->def = NULL;
-        virNodeDeviceObjUnlock(obj);
-        virNodeDeviceObjFree(obj);
+        virNodeDeviceObjEndAPI(&obj);
         return NULL;
     }
 
-    return obj;
+    return virObjectRef(obj);
 }
 
 
@@ -386,17 +407,18 @@ virNodeDeviceObjListRemove(virNodeDeviceObjListPtr devs,
 {
     size_t i;
 
-    virNodeDeviceObjUnlock(obj);
+    virObjectUnlock(obj);
 
     for (i = 0; i < devs->count; i++) {
-        virNodeDeviceObjLock(devs->objs[i]);
+        virObjectLock(devs->objs[i]);
         if (devs->objs[i] == obj) {
-            virNodeDeviceObjUnlock(devs->objs[i]);
+            virObjectUnlock(devs->objs[i]);
+            virObjectUnref(devs->objs[i]);
 
             VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
             break;
         }
-        virNodeDeviceObjUnlock(devs->objs[i]);
+        virObjectUnlock(devs->objs[i]);
     }
 }
 
@@ -447,7 +469,7 @@ virNodeDeviceObjListGetParentHostByParent(virNodeDeviceObjListPtr devs,
 
     ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     return ret;
 }
@@ -472,7 +494,7 @@ virNodeDeviceObjListGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
 
     ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     return ret;
 }
@@ -495,7 +517,7 @@ virNodeDeviceObjListGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
 
     ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     return ret;
 }
@@ -516,7 +538,7 @@ virNodeDeviceObjListFindVportParentHost(virNodeDeviceObjListPtr devs)
 
     ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     return ret;
 }
@@ -549,20 +571,6 @@ virNodeDeviceObjListGetParentHost(virNodeDeviceObjListPtr devs,
 }
 
 
-void
-virNodeDeviceObjLock(virNodeDeviceObjPtr obj)
-{
-    virMutexLock(&obj->lock);
-}
-
-
-void
-virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj)
-{
-    virMutexUnlock(&obj->lock);
-}
-
-
 static bool
 virNodeDeviceCapMatch(virNodeDeviceObjPtr obj,
                       int type)
@@ -624,11 +632,11 @@ virNodeDeviceObjListNumOfDevices(virNodeDeviceObjListPtr devs,
 
     for (i = 0; i < devs->count; i++) {
         virNodeDeviceObjPtr obj = devs->objs[i];
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         if ((!aclfilter || aclfilter(conn, obj->def)) &&
             (!cap || virNodeDeviceObjHasCap(obj, cap)))
             ++ndevs;
-        virNodeDeviceObjUnlock(obj);
+        virObjectUnlock(obj);
     }
 
     return ndevs;
@@ -648,16 +656,16 @@ virNodeDeviceObjListGetNames(virNodeDeviceObjListPtr devs,
 
     for (i = 0; i < devs->count && nnames < maxnames; i++) {
         virNodeDeviceObjPtr obj = devs->objs[i];
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         if ((!aclfilter || aclfilter(conn, obj->def)) &&
             (!cap || virNodeDeviceObjHasCap(obj, cap))) {
             if (VIR_STRDUP(names[nnames], obj->def->name) < 0) {
-                virNodeDeviceObjUnlock(obj);
+                virObjectUnlock(obj);
                 goto failure;
             }
             nnames++;
         }
-        virNodeDeviceObjUnlock(obj);
+        virObjectUnlock(obj);
     }
 
     return nnames;
@@ -719,21 +727,21 @@ virNodeDeviceObjListExport(virConnectPtr conn,
 
     for (i = 0; i < devs->count; i++) {
         virNodeDeviceObjPtr obj = devs->objs[i];
-        virNodeDeviceObjLock(obj);
+        virObjectLock(obj);
         if ((!aclfilter || aclfilter(conn, obj->def)) &&
             virNodeDeviceMatch(obj, flags)) {
             if (devices) {
                 if (!(device = virGetNodeDevice(conn, obj->def->name)) ||
                     VIR_STRDUP(device->parent, obj->def->parent) < 0) {
                     virObjectUnref(device);
-                    virNodeDeviceObjUnlock(obj);
+                    virObjectUnlock(obj);
                     goto cleanup;
                 }
                 tmp_devices[ndevices] = device;
             }
             ndevices++;
         }
-        virNodeDeviceObjUnlock(obj);
+        virObjectUnlock(obj);
     }
 
     if (tmp_devices) {
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 1122b67..788fb66 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -45,6 +45,8 @@ struct _virNodeDeviceDriverState {
     virObjectEventStatePtr nodeDeviceEventState;
 };
 
+void
+virNodeDeviceObjEndAPI(virNodeDeviceObjPtr *obj);
 
 virNodeDeviceDefPtr
 virNodeDeviceObjGetDef(virNodeDeviceObjPtr obj);
@@ -76,21 +78,12 @@ virNodeDeviceObjListGetParentHost(virNodeDeviceObjListPtr devs,
                                   virNodeDeviceDefPtr def,
                               int create);
 
-void
-virNodeDeviceObjFree(virNodeDeviceObjPtr dev);
-
 virNodeDeviceObjListPtr
 virNodeDeviceObjListNew(void);
 
 void
 virNodeDeviceObjListFree(virNodeDeviceObjListPtr devs);
 
-void
-virNodeDeviceObjLock(virNodeDeviceObjPtr obj);
-
-void
-virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj);
-
 typedef bool
 (*virNodeDeviceObjListFilter)(virConnectPtr conn,
                               virNodeDeviceDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d415888..1eb4502 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -957,7 +957,7 @@ virNetworkObjUpdateAssignDef;
 
 
 # conf/virnodedeviceobj.h
-virNodeDeviceObjFree;
+virNodeDeviceObjEndAPI;
 virNodeDeviceObjGetDef;
 virNodeDeviceObjListAssignDef;
 virNodeDeviceObjListExport;
@@ -970,8 +970,6 @@ virNodeDeviceObjListGetParentHost;
 virNodeDeviceObjListNew;
 virNodeDeviceObjListNumOfDevices;
 virNodeDeviceObjListRemove;
-virNodeDeviceObjLock;
-virNodeDeviceObjUnlock;
 
 
 # conf/virnwfilterobj.h
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 4a5f168..788b8bc 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -277,7 +277,7 @@ nodeDeviceLookupByName(virConnectPtr conn,
     }
 
  cleanup:
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return device;
 }
 
@@ -314,7 +314,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
     }
 
  cleanup:
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return device;
 }
 
@@ -345,7 +345,7 @@ nodeDeviceGetXMLDesc(virNodeDevicePtr device,
     ret = virNodeDeviceDefFormat(def);
 
  cleanup:
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ret;
 }
 
@@ -373,7 +373,7 @@ nodeDeviceGetParent(virNodeDevicePtr device)
     }
 
  cleanup:
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ret;
 }
 
@@ -411,7 +411,7 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device)
     ret = ncaps;
 
  cleanup:
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ret;
 }
 
@@ -460,7 +460,7 @@ nodeDeviceListCaps(virNodeDevicePtr device,
     ret = ncaps;
 
  cleanup:
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     if (ret == -1) {
         --ncaps;
         while (--ncaps >= 0)
@@ -613,8 +613,7 @@ nodeDeviceDestroy(virNodeDevicePtr device)
      * to be taken, so grab the object def which will have the various
      * fields used to search (name, parent, parent_wwnn, parent_wwpn,
      * or parent_fabric_wwn) and drop the object lock. */
-    virNodeDeviceObjUnlock(obj);
-    obj = NULL;
+    virNodeDeviceObjEndAPI(&obj);
     if ((parent_host = virNodeDeviceObjListGetParentHost(driver->devs, def,
                                                          EXISTING_DEVICE)) < 0)
         goto cleanup;
@@ -626,7 +625,7 @@ nodeDeviceDestroy(virNodeDevicePtr device)
 
  cleanup:
     nodeDeviceUnlock();
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     VIR_FREE(wwnn);
     VIR_FREE(wwpn);
     return ret;
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index 5521287..d92b47a 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -490,7 +490,7 @@ dev_create(const char *udi)
 
     objdef->sysfs_path = devicePath;
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     nodeDeviceUnlock();
     return;
@@ -545,7 +545,7 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
     VIR_DEBUG("%s", name);
     if (obj) {
         virNodeDeviceObjListRemove(driver->devs, obj);
-        virNodeDeviceObjFree(obj);
+        virObjectUnref(obj);
     } else {
         VIR_DEBUG("no device named %s", name);
     }
@@ -568,7 +568,7 @@ device_cap_added(LibHalContext *ctx,
     if (obj) {
         def = virNodeDeviceObjGetDef(obj);
         (void)gather_capability(ctx, udi, cap, &def->caps);
-        virNodeDeviceObjUnlock(obj);
+        virNodeDeviceObjEndAPI(&obj);
     } else {
         VIR_DEBUG("no device named %s", name);
     }
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index a210fe1..8016d17 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1332,7 +1332,7 @@ udevRemoveOneDevice(struct udev_device *device)
     VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
               def->name, name);
     virNodeDeviceObjListRemove(driver->devs, obj);
-    virNodeDeviceObjFree(obj);
+    virObjectUnlock(obj);
 
     if (event)
         virObjectEventStateQueue(driver->nodeDeviceEventState, event);
@@ -1369,10 +1369,10 @@ udevSetParent(struct udev_device *device,
                                                        parent_sysfs_path))) {
             objdef = virNodeDeviceObjGetDef(obj);
             if (VIR_STRDUP(def->parent, objdef->name) < 0) {
-                virNodeDeviceObjUnlock(obj);
+                virNodeDeviceObjEndAPI(&obj);
                 goto cleanup;
             }
-            virNodeDeviceObjUnlock(obj);
+            virNodeDeviceObjEndAPI(&obj);
 
             if (VIR_STRDUP(def->parent_sysfs_path, parent_sysfs_path) < 0)
                 goto cleanup;
@@ -1426,7 +1426,7 @@ udevAddOneDevice(struct udev_device *device)
 
     obj = virNodeDeviceObjListFindByName(driver->devs, def->name);
     if (obj) {
-        virNodeDeviceObjUnlock(obj);
+        virNodeDeviceObjEndAPI(&obj);
         new_device = false;
     }
 
@@ -1443,7 +1443,7 @@ udevAddOneDevice(struct udev_device *device)
     else
         event = virNodeDeviceEventUpdateNew(objdef->name);
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     ret = 0;
 
@@ -1725,7 +1725,7 @@ udevSetupSystemDev(void)
     if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
         goto cleanup;
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     ret = 0;
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 67fe252..7b67523 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1175,7 +1175,7 @@ testParseNodedevs(testDriverPtr privconn,
             goto error;
         }
 
-        virNodeDeviceObjUnlock(obj);
+        virNodeDeviceObjEndAPI(&obj);
     }
 
     ret = 0;
@@ -4320,7 +4320,7 @@ testCreateVport(testDriverPtr driver,
      * create the vHBA. In the long run the result is the same. */
     if (!(obj = testNodeDeviceMockCreateVport(driver, wwnn, wwpn)))
         return -1;
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     return 0;
 }
@@ -4532,7 +4532,7 @@ testDestroyVport(testDriverPtr privconn,
                                            0);
 
     virNodeDeviceObjListRemove(privconn->devs, obj);
-    virNodeDeviceObjFree(obj);
+    virObjectUnref(obj);
     obj = NULL;
 
     testObjectEventQueue(privconn, event);
@@ -5334,7 +5334,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
         }
     }
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ret;
 }
 
@@ -5353,7 +5353,7 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
 
     ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj));
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ret;
 }
 
@@ -5376,7 +5376,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
                        "%s", _("no parent for this device"));
     }
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ret;
 }
 
@@ -5397,7 +5397,7 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev)
     for (caps = def->caps; caps; caps = caps->next)
         ++ncaps;
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ncaps;
 }
 
@@ -5422,13 +5422,13 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
         ncaps++;
     }
 
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return ncaps;
 
  error:
     while (--ncaps >= 0)
         VIR_FREE(names[ncaps]);
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     return -1;
 }
 
@@ -5459,7 +5459,7 @@ testNodeDeviceMockCreateVport(testDriverPtr driver,
         goto cleanup;
 
     xml = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(objcopy));
-    virNodeDeviceObjUnlock(objcopy);
+    virNodeDeviceObjEndAPI(&objcopy);
     if (!xml)
         goto cleanup;
 
@@ -5563,8 +5563,7 @@ testNodeDeviceCreateXML(virConnectPtr conn,
     dev = NULL;
 
  cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     testDriverUnlock(driver);
     virNodeDeviceDefFree(def);
     virObjectUnref(dev);
@@ -5597,28 +5596,29 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
      * taken, so we have to dup the parent's name and drop the lock
      * before calling it.  We don't need the reference to the object
      * any more once we have the parent's name.  */
-    virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
 
     /* We do this just for basic validation, but also avoid finding a
      * vport capable HBA if for some reason our vHBA doesn't exist */
     if (virNodeDeviceObjListGetParentHost(driver->devs, def,
-                                          EXISTING_DEVICE) < 0) {
-        obj = NULL;
+                                          EXISTING_DEVICE) < 0)
         goto cleanup;
-    }
 
     event = virNodeDeviceEventLifecycleNew(dev->name,
                                            VIR_NODE_DEVICE_EVENT_DELETED,
                                            0);
 
-    virNodeDeviceObjLock(obj);
+    /*
+     *
+     * REVIEW THIS
+     *
+     */
     virNodeDeviceObjListRemove(driver->devs, obj);
-    virNodeDeviceObjFree(obj);
+    virObjectUnref(obj);
     obj = NULL;
 
  cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjEndAPI(&obj);
     testObjectEventQueue(driver, event);
     VIR_FREE(parent_name);
     VIR_FREE(wwnn);
-- 
2.9.4

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

On 06/03/2017 09:12 AM, John Ferlan wrote:
> Now that we have a bit more control, let's convert our object into
> a lockable object and let that magic handle the create and lock/unlock.
> 
> This also involves creating a virNodeDeviceEndAPI in order to handle
> the object cleaup for API's that use the Add or Find API's in order
> to get a locked/reffed object. The EndAPI will unlock and unref the
> object returning NULL to indicate to the caller to not use the obj.
> 
> For now this also involves a forward reference to the Unlock, but
> that'll be removed shortly when the object is convert to use the
> virObjectLockable shortly.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/virnodedeviceobj.c          | 156 ++++++++++++++++++-----------------
>  src/conf/virnodedeviceobj.h          |  11 +--
>  src/libvirt_private.syms             |   4 +-
>  src/node_device/node_device_driver.c |  17 ++--
>  src/node_device/node_device_hal.c    |   6 +-
>  src/node_device/node_device_udev.c   |  12 +--
>  src/test/test_driver.c               |  40 ++++-----
>  7 files changed, 122 insertions(+), 124 deletions(-)
> 

[...]

> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 67fe252..7b67523 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c

[...]

Oh my... looks like I forgot to go back and revisit the following hunk
;-), but of course tripped across it while working through merging
changes for patch 1.

> @@ -5597,28 +5596,29 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
>       * taken, so we have to dup the parent's name and drop the lock
>       * before calling it.  We don't need the reference to the object
>       * any more once we have the parent's name.  */
> -    virNodeDeviceObjUnlock(obj);
> +    virNodeDeviceObjEndAPI(&obj);

This should be "virObjectUnlock(obj);"

>  
>      /* We do this just for basic validation, but also avoid finding a
>       * vport capable HBA if for some reason our vHBA doesn't exist */
>      if (virNodeDeviceObjListGetParentHost(driver->devs, def,
> -                                          EXISTING_DEVICE) < 0) {
> -        obj = NULL;
> +                                          EXISTING_DEVICE) < 0)

This would require calling virObjectLock(obj);

>          goto cleanup;
> -    }
>  
>      event = virNodeDeviceEventLifecycleNew(dev->name,
>                                             VIR_NODE_DEVICE_EVENT_DELETED,
>                                             0);
>  
> -    virNodeDeviceObjLock(obj);

and calling virObjectLock(obj) here prior to calling ObjListRemove and
setting obj = NULL;

> +    /*
> +     *
> +     * REVIEW THIS
> +     *
> +     */
>      virNodeDeviceObjListRemove(driver->devs, obj);
> -    virNodeDeviceObjFree(obj);
> +    virObjectUnref(obj);
>      obj = NULL;

Hope this makes sense... I'm guessing some of this series will need to
be reposted in a v4 anyway - including the "next" logical step to alter
the ObjList which is what you're after anyway, but was further down in
my original stack of patches.

John
>  
>   cleanup:
> -    if (obj)
> -        virNodeDeviceObjUnlock(obj);
> +    virNodeDeviceObjEndAPI(&obj);
>      testObjectEventQueue(driver, event);
>      VIR_FREE(parent_name);
>      VIR_FREE(wwnn);
> 

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

On Thu, Jun 29, 2017 at 12:11:47PM -0400, John Ferlan wrote:
>
>
> On 06/03/2017 09:12 AM, John Ferlan wrote:
> > Now that we have a bit more control, let's convert our object into
> > a lockable object and let that magic handle the create and lock/unlock.
> >
> > This also involves creating a virNodeDeviceEndAPI in order to handle
> > the object cleaup for API's that use the Add or Find API's in order
> > to get a locked/reffed object. The EndAPI will unlock and unref the
> > object returning NULL to indicate to the caller to not use the obj.
> >
> > For now this also involves a forward reference to the Unlock, but
> > that'll be removed shortly when the object is convert to use the
> > virObjectLockable shortly.
> >
> > Signed-off-by: John Ferlan <jferlan@redhat.com>
> > ---
> >  src/conf/virnodedeviceobj.c          | 156 ++++++++++++++++++-----------------
> >  src/conf/virnodedeviceobj.h          |  11 +--
> >  src/libvirt_private.syms             |   4 +-
> >  src/node_device/node_device_driver.c |  17 ++--
> >  src/node_device/node_device_hal.c    |   6 +-
> >  src/node_device/node_device_udev.c   |  12 +--
> >  src/test/test_driver.c               |  40 ++++-----
> >  7 files changed, 122 insertions(+), 124 deletions(-)
> >
>
> [...]
>
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 67fe252..7b67523 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
>
> [...]
>
> Oh my... looks like I forgot to go back and revisit the following hunk
> ;-), but of course tripped across it while working through merging
> changes for patch 1.
>
> > @@ -5597,28 +5596,29 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
> >       * taken, so we have to dup the parent's name and drop the lock
> >       * before calling it.  We don't need the reference to the object
> >       * any more once we have the parent's name.  */
> > -    virNodeDeviceObjUnlock(obj);
> > +    virNodeDeviceObjEndAPI(&obj);
>
> This should be "virObjectUnlock(obj);"
>
> >
> >      /* We do this just for basic validation, but also avoid finding a
> >       * vport capable HBA if for some reason our vHBA doesn't exist */
> >      if (virNodeDeviceObjListGetParentHost(driver->devs, def,
> > -                                          EXISTING_DEVICE) < 0) {
> > -        obj = NULL;
> > +                                          EXISTING_DEVICE) < 0)
>
> This would require calling virObjectLock(obj);
>
> >          goto cleanup;
> > -    }
> >
> >      event = virNodeDeviceEventLifecycleNew(dev->name,
> >                                             VIR_NODE_DEVICE_EVENT_DELETED,
> >                                             0);
> >
> > -    virNodeDeviceObjLock(obj);
>
> and calling virObjectLock(obj) here prior to calling ObjListRemove and
> setting obj = NULL;
>
> > +    /*
> > +     *
> > +     * REVIEW THIS
> > +     *
> > +     */
> >      virNodeDeviceObjListRemove(driver->devs, obj);
> > -    virNodeDeviceObjFree(obj);
> > +    virObjectUnref(obj);
> >      obj = NULL;
>
> Hope this makes sense... I'm guessing some of this series will need to
> be reposted in a v4 anyway - including the "next" logical step to alter
> the ObjList which is what you're after anyway, but was further down in
> my original stack of patches.

Well, these would be really hard to track, so I agree that posting a v4 would
make things easier, since 10 (11 technically) out of 12 patches have already
been ACK'd, so it wouldn't prolong the overall process more than just those 2
patches we discuss.

Erik

>
> John
> >
> >   cleanup:
> > -    if (obj)
> > -        virNodeDeviceObjUnlock(obj);
> > +    virNodeDeviceObjEndAPI(&obj);
> >      testObjectEventQueue(driver, event);
> >      VIR_FREE(parent_name);
> >      VIR_FREE(wwnn);
> >
>
> --
> 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