[libvirt] [PATCH 0/6] Complete cleanup of domain object usage

[John Ferlan]

ping?

Still need review on 1-3, & 6

Tks

John

On 04/24/2018 08:28 AM, John Ferlan wrote:
> This should be the "last time" (famous last words) needing to alter
> this processing. I hope to get more than one pair of eyes looking
> at the series. I've looked at it long enough to feel comfortable
> with the changes, but I also know I probably have looked at it
> so long that I could have missed something!
> 
> The first 3 patches set up the necessary infrastructure. Patches
> 4 & 5 are just something I found in libxl while doing this - they
> could be extracted and considered separately if need be; however,
> since I was thinking about @vm referencing - it just seemed to fit.
> Since I altered @vm it only seemed right to do the same for @conn.
> While it could be said that neither condition could happen - I
> figured better safe than sorry and I think it follows how this type
> of logic has been handled elsewhere in qemu.
> 
> Patch 6 is where all the magic happens. I tried a few different ways
> to rework and split the Add/Remove processing, but either way I was
> left with rather ugly patches that required touching the same places,
> so I capitulated and combined them since there really is a delicate
> balance between those paths that have added a virObjectRef after the
> virDomainObjListAdd and those that just use virObjectUnlock on the
> returned ListAdd objects. Converting the latter to use Ref caused more
> "busy work" and equally large patches. The busy work was mosting around
> the processing required during ListAdd if something happened which
> resulted in ListRemove needing to be called.
> 
> The "general change" is that instead of having ListAdd return an object
> with 2 refs and 1 lock it will now return an object with 3 refs and 1
> lock. With that returned object, some drivers would perform an ObjectRef
> while others did not. For those that did not, they would use ObjectUnlock
> when done with the object so that when leaving whatever Create routine
> there was there would be at least 2 Refs on the object. The change here
> is to have them all be consistent. What follows is a "general description"
> of what was done for each.
> 
> For bhyve, openvz, test, uml, and vmware (e.g. domain drivers that
> do not use virObjectRef on the returned virDomainObjListAdd object):
> 
>   Create/Add processing:
> 
>     -> Use EndAPI instead of ObjectUnlock
>     -> Remove setting vm = NULL if the ListRemove was called to allow
>        EndAPI processing to perform last Unref and Unlock
> 
>        NB: ListRemove would remove 2 refs and unlock, so it "worked",
>            but would "consume" the returned object
> 
>   Undefine/Destroy processing:
> 
>     -> Alter ListRemove and ObjectLock to just be ListRemove allowing the
>        EndAPI processing from a FindBy* to perform last Unref and Unlock
> 
> For libxl, lxc, qemu, and vz (e.g. domain drivers that use virObjectRef
> on the returned virDomainObjListAdd object):
> 
>   Create/Add processing:
> 
>     -> Remove the ObjectRef
>     -> Remove the ObjectLock when needing to call ListRemove to allow
>        EndAPI processing to perform the last Unref and Unlock
> 
>        NB: ListRemove no longer Unlock's the returned object
> 
>   Undefine/Destroy processing:
> 
>     -> Alter ListRemove and ObjectLock to just be ListRemove allowing the
>        EndAPI processing from a FindBy* to perform last Unref and Unlock
> 
> NB: For the AutoDestroy type logic, no change is required since in the
>     long run the object was left with the correct number of refs after
>     create processing ran. The issue is more the direct mgmt of object
>     during Add/Remove rather than mgmt of object once defined.
> 
> John Ferlan (6):
>   conf: Split FindBy{UUID|Name} into locked helpers
>   conf: Use virDomainObjListFindBy*Locked for virDomainObjListAdd
>   conf: Move and use virDomainObjListRemoveLocked
>   libxl: Add refcnt for args->vm during migration
>   libxl: Add refcnt for args->conn during migration
>   conf: Clean up object referencing for Add and Remove
> 
>  src/bhyve/bhyve_driver.c    |  21 ++----
>  src/conf/virdomainobjlist.c | 165 ++++++++++++++++++++++++++++----------------
>  src/libxl/libxl_driver.c    |  64 ++++-------------
>  src/libxl/libxl_migration.c |  41 ++++-------
>  src/lxc/lxc_driver.c        |  21 ++----
>  src/lxc/lxc_process.c       |  17 +++--
>  src/openvz/openvz_conf.c    |   2 +-
>  src/openvz/openvz_driver.c  |  20 ++----
>  src/qemu/qemu_domain.c      |  17 +----
>  src/qemu/qemu_driver.c      |   6 +-
>  src/qemu/qemu_migration.c   |   3 +-
>  src/test/test_driver.c      |  56 +++++----------
>  src/uml/uml_driver.c        |  37 +++-------
>  src/vmware/vmware_conf.c    |   3 +-
>  src/vmware/vmware_driver.c  |  17 ++---
>  src/vz/vz_driver.c          |   1 -
>  src/vz/vz_sdk.c             |   3 -
>  17 files changed, 192 insertions(+), 302 deletions(-)
>