[libvirt] [PATCH 6/6] conf: Clean up object referencing for Add and Remove

[Erik Skultety]

On Tue, Apr 24, 2018 at 08:28:09AM -0400, John Ferlan wrote:
> When adding a new object to the domain object list, there should
> have been 2 virObjectRef calls made one for each list into which
> the object was placed to match the 2 virObjectUnref calls that
> would occur during Remove as part of virHashRemoveEntry when
> virObjectFreeHashData is called when the element is removed from
> the hash table as set up in virDomainObjListNew.
>
> Some drivers (libxl, lxc, qemu, and vz) handled this inconsistency
> by calling virObjectRef upon successful return from virDomainObjListAdd
> in order to use virDomainObjEndAPI when done with the returned @vm.
> While others (bhyve, openvz, test, and vmware) handled this via only
> calling virObjectUnlock upon successful return from virDomainObjListAdd.
>
> This patch will "unify" the approach to use virDomainObjEndAPI
> for any @vm successfully returned from virDomainObjListAdd.
>
> Because list removal is so tightly coupled with list addition,
> this patch fixes the list removal algorithm to return the object
> as entered - "locked and reffed".  This way, the callers can then
> decide how to uniformly handle add/remove success and failure.
> This removes the onus on the caller to "specially handle" the
> @vm during removal processing.
>
> The Add/Remove logic allows for some logic simplification such
> as in libxl where we can Remove the @vm directly rather than
> needing to set a @remove_dom boolean and removing after the
> libxlDomainObjEndJob completes as the @vm is locked/reffed.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---

...

>
>
> @@ -386,8 +386,7 @@ virDomainObjListRemoveLocked(virDomainObjListPtr doms,
>   * no one else is either waiting for 'dom' or still using it.
>   *
>   * When this function returns, @dom will be removed from the hash
> - * tables, unlocked, and returned with the refcnt that was present
> - * upon entry.
> + * tables and returned with lock and refcnt that was present upon entry.

I know ^this is pre-existing, but I reads strange, when we get here, refcnt is
3 (after your patches), when we get out, refcnt is 1, I know what the idea is,
but it can be confusing, it sounds like the refcnt isn't changing when in fact
it is.
I went through this twice and figured that even if we missed something
somewhere, it's so early in the new release-cycle that we have plenty of time
to catch it, the changes look very good and it's an improvement.

Reviewed-by: Erik Skultety <eskultet at redhat.com>