ping^^2 Again, don't bother with patches 4-6. Tks, John On 08/02/2017 07:27 AM, John Ferlan wrote: > > ping - > > perhaps at least the first 3... > > I'm now beginning to think/wonder if using the rwlock_rdlock would be > the solution at least for nwfilter objs. It seems from a quick scan of > the man page that they are designed to be recursive as long as the > consumer guarantees that there is an Unlock for every LockRead. A lot > better than rolling my own recursive lock algorithm that I tried in > patch 4. Would require some other adjustments (and concessions) along > the way, but seemingly possible. > > Tks - > > John > > > On 07/18/2017 04:57 PM, John Ferlan wrote: >> v1: https://www.redhat.com/archives/libvir-list/2017-June/msg00079.html >> >> Changes since v1 (if I can recall all of them!): >> >> Patches 1, 4, 8-13 were pushed >> Patches 2, 3, 5-7 are dropped >> >> This this is a rework of patches 14-17 >> >> Patch 1 (former patch14): >> * Requested adjustments made to ACK'd patch, but since this and the >> remaining ones were related I didn't yet push it. >> >> Patch 2 (new): >> * From review though... As it turns out, virNWFilterDefEqual doesn't >> require the @cmpUUIDs patch. >> >> Patch 3 (fromer patch 15): >> * Fixed the line as requested. Patch was ACK'd by like Patch 1 I held >> onto it since it was related. >> >> Patch 4 (former patch 16): >> * Let's call it a complete rewrite. Rather than rely solely on the >> refcnt of the object, I've added/implemented a 'trylock' mechanism >> which essentially will allow the subsequent patch to use the >> virObjectLockable (e.g. a non recursive lock). Of course as I got >> further into the code - I think I've come to the conclusion that >> there isn't a way for a @def to disappear between threads with a >> refcnt only mechanism because there's a few other serialized locks >> which would need to be hurdled before hand. Still as I found out >> while running the Avocado test 'nwfilter_update_vm_running.update_arp_rule' >> the recursion would occur because the AssignDef code would call the >> Instantiation with the lock from the def being updated and that's >> where all the awful magic needs to occur. Additionally, I found that >> one wouldn't want to attempt to lock the nwfilters list inside the >> virNWFilterObjListFindInstantiateFilter because AssignDef already >> had that lock. I debated needing a recursive lock there until I >> came to the conclusion that the list couldn't change because the >> DefineXML is protected by a driver level lock (as is the Undefine >> and Reload paths). >> >> Patch 5 (former patch 17): >> * No changes, it was ACK'd, but without 1-4 is useless >> >> Patch 6 (NEW): >> * Remove the need for the driver level lock for a few API's since >> we have self locking nwfilters list. Also left comments in the >> 3 places where that lock remained to hopefully cause someone great >> anxiety if they decided to attempt to remove the lock without >> first consulting a specialist. >> >> NB: Ran all of the changes through the 'nwfilter' tests found in >> the Avocado test suite. >> >> John Ferlan (6): >> nwfilter: Add @refs logic to __virNWFilterObj >> nwfilter: Remove unnecessary UUID comparison bypass >> nwfilter: Convert _virNWFilterObjList to be a virObjectLockable >> nwfilter: Remove recursive locking for nwfilter instantiation >> nwfilter: Convert virNWFilterObj to use virObjectLockable >> nwfilter: Remove need for nwfilterDriverLock in some API's >> >> src/conf/virnwfilterobj.c | 635 ++++++++++++++++++++++++--------- >> src/conf/virnwfilterobj.h | 12 +- >> src/libvirt_private.syms | 6 +- >> src/nwfilter/nwfilter_driver.c | 66 ++-- >> src/nwfilter/nwfilter_gentech_driver.c | 66 +++- >> src/util/virobject.c | 24 ++ >> src/util/virobject.h | 4 + >> src/util/virthread.c | 5 + >> src/util/virthread.h | 1 + >> 9 files changed, 586 insertions(+), 233 deletions(-) >> > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
