On 04/10/2018 04:47 AM, Marc Hartmayer wrote: > On Wed, Mar 28, 2018 at 11:19 PM +0200, John Ferlan <jferlan@xxxxxxxxxx> 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 > > Side note: > Wouldn’t is be useful to refactor all the vir*ObjList* things as they’re > looking quite similar? Not sure if it’s easily feasible in all places. > Well - that was the point of what I started last year, but there hasn't been any general agreement or acceptance of patches for that. My changes made use of objects and more generic naming to unify things; however, they weren't acceptable with (IIRC and in my words) the preference being more specific naming using switch/case statements and shim API's. The last full posting (a v5) of what I had done is here: https://www.redhat.com/archives/libvir-list/2017-August/msg00659.html If you feel so inclined you can follow the history of comments through v4: https://www.redhat.com/archives/libvir-list/2017-August/msg00537.html and v3: https://www.redhat.com/archives/libvir-list/2017-June/msg00916.html w/ review comments starting here: https://www.redhat.com/archives/libvir-list/2017-July/msg01032.html Maybe once the domain code is modified to be more common (in process now) and if nwfilter ever could gain acceptance, something could be done. Still I have my doubts it'll happen especially since nwfilter patches just cannot get any sort of agreement, last try here: https://www.redhat.com/archives/libvir-list/2018-February/msg00325.html John > […snip] > > -- > Beste Grüße / Kind regards > Marc Hartmayer > > IBM Deutschland Research & Development GmbH > Vorsitzende des Aufsichtsrats: Martina Koederitz > Geschäftsführung: Dirk Wittkopp > Sitz der Gesellschaft: Böblingen > Registergericht: Amtsgericht Stuttgart, HRB 243294 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
