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