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

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.