Re: [libvirt] [PATCH v3 12/12] nodedev: Convert virNodeDeviceObj to use virObjectLockable

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(a)redhat.com&gt; > --- > 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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list