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(-)
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(-)
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.