[libvirt] [PATCH v2 0/9] Be consistent with vir*Obj*Remove* APIs

[John Ferlan]

ping?

Tks -

John

On 03/28/2018 05:19 PM, John Ferlan wrote:
> v1: https://www.redhat.com/archives/libvir-list/2018-March/msg01295.html
> 
> NB: This can wait until 4.2.0 is release, but I figured I'd post this
>     now just to put it on the radar and of course in hopes that someone
>     will look during the idle moment or two before the release.
> 
> Changes since v1:
> 
>  Short story: Rework the processing of the code
> 
>  Longer story:
> 
>  In his review Erik noted that there's a "fire dance" when processing
>  the vir*Obj*Remove APIs of requiring a locked object upon entry, then
>  adding a reference to that object, unlocking the object, locking the
>  list to which it is contained, and then relocking the object.
> 
>  So it took some time to think about it and during one lengthy meeting
>  today I had the aha moment that the *Remove API's could take the same
>  key (e.g., uuid or name) used to Add or Find the object and use it for
>  the Remove API. This allows the Remove API to not require a locked (and
>  reffed) object upon entry and perform the lock dance, remove the object,
>  and return just just a reffed object forcing the caller to know to Unref
>  object.
> 
>  Instead, let's simplify things. The *Remove API can take the key, Find
>  the object in the list, remove it from the hash tables, and dispose of
>  the object. In essence the antecedent to the Add or AssignDef API's
>  taking a def, creating an object, and adding it the object to the hash
>  table(s). If there are two *Remove threads competing, one will win and
>  perform the removal, while the other will call *Remove, but won't find
>  the object in the hash table, and just return none the wiser.
> 
>  And yes, I think this can also work for the Domain code, but it's going
>  to take a few patch series to get there as that code is not consistent
>  between consumers.
> 
> John Ferlan (9):
>   secret: Rework LoadAllConfigs
>   secret: Alter virSecretObjListRemove processing
>   interface: Alter virInterfaceObjListRemove processing
>   nodedev: Alter virNodeDeviceObjListRemove processing
>   conf: Clean up virStoragePoolObjLoad error processing
>   storage: Clean up storagePoolCreateXML error processing
>   test: Clean up testStoragePoolCreateXML error processing
>   conf: Move virStoragePoolObjRemove closer to AssignDef
>   storagepool: Alter virStoragePoolObjRemove processing
> 
>  src/conf/virinterfaceobj.c         | 26 +++++++----
>  src/conf/virinterfaceobj.h         |  2 +-
>  src/conf/virnodedeviceobj.c        | 29 +++++++-----
>  src/conf/virnodedeviceobj.h        |  2 +-
>  src/conf/virsecretobj.c            | 58 +++++++++++-------------
>  src/conf/virsecretobj.h            |  2 +-
>  src/conf/virstorageobj.c           | 92 +++++++++++++++++++++++---------------
>  src/conf/virstorageobj.h           |  2 +-
>  src/node_device/node_device_hal.c  | 16 ++++---
>  src/node_device/node_device_udev.c | 13 ++++--
>  src/secret/secret_driver.c         | 15 ++++---
>  src/storage/storage_driver.c       | 65 +++++++++++++++------------
>  src/test/test_driver.c             | 78 +++++++++++++++++---------------
>  13 files changed, 225 insertions(+), 175 deletions(-)
>