[libvirt] [PATCH v2 0/6] nwfilter adjustments for common object

[John Ferlan]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>