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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
