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