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

John Ferlan jferlan at redhat.com
Tue Apr 24 12:28:03 UTC 2018


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(-)

-- 
2.13.6




More information about the libvir-list mailing list