[libvirt] [PATCH] util: don't check for parallel iteration in hash-related functions

[Michal Privoznik]

On 04/06/2018 12:01 PM, Michal Privoznik wrote:
> On 04/06/2018 09:56 AM, Vincent Bernat wrote:
>> This is the responsability of the caller to apply the correct lock
>> before using these functions. Moreover, the use of a simple boolean
>> was still racy: two threads may check the boolean and "lock" it
>> simultaneously.
>>
>> Users of functions from src/util/virhash.c have to be checked for
>> correctness. Lookups and iteration should hold a RO
>> lock. Modifications should hold a RW lock.
>>
>> Most important uses seem to be covered. Callers have now a greater
>> responsability, notably the ability to execute some operations while
>> iterating were reliably forbidden before are now accepted.
>> ---
>>  src/util/virhash.c  | 37 ----------------------
>>  tests/virhashtest.c | 75 ---------------------------------------------
>>  2 files changed, 112 deletions(-)
> 
> So I went through all callbacks (even transitive ones) and I've found
> two problems:
> 
> umlProcessAutoDestroyRun -> umlProcessAutoDestroyDom -> virHashRemoveEntry
> 
> qemuDomainSnapshotDiscardAllMetadata -> qemuDomainSnapshotDiscardAll ->
> qemuDomainSnapshotDiscard -> virDomainSnapshotObjListRemove ->
> virHashRemoveEntry
> 
> While me (and probably Peter :-)) don't care about the first one, the
> second one is a real issue. I guess we need to fix that one before this
> can be merged.
> 
> On a positive side, I haven't spotted any other problem. So once qemu
> (and possibly uml) are fixed this can be merged as is.

Oh, I forgot to add: this function needs to be removed too
testHashIter() because you're removing its only caller.

diff --git i/tests/virhashtest.c w/tests/virhashtest.c
index a013bc7169..e9c03c1afb 100644
--- i/tests/virhashtest.c
+++ w/tests/virhashtest.c
@@ -277,14 +277,6 @@ testHashSteal(const void *data ATTRIBUTE_UNUSED)
 }
 
 
-static int
-testHashIter(void *payload ATTRIBUTE_UNUSED,
-             const void *name ATTRIBUTE_UNUSED,
-             void *data ATTRIBUTE_UNUSED)
-{
-    return 0;
-}
-



Michal