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.
Michal