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

[John Ferlan]

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