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