Comment 26 for bug 1822096

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Michal might have already had that understanding, but for me it was worth to track that down.
The problem with the patch so far is that it allows priv->devs to go away and that is what is causing the issues with the patch applied (also as seen in comment #24 only one thread is left locking on the bad struct, but no two thread deadlock scenario anymore.

Considering the above it seems that FDST can not be dropped without the risk of loosing priv->devs content. I think we need a clear ordering of FDST>>devs and in this stack that could be done by modifying virChrdevFree.

The lower layers should lock priv->devs as needed (and it seems they do already).
But virChrdevFree should not do the callbacks with devs->lock held.
Only later for free/destroy.
The only one registered atm as callback to clean is virChrdevFDStreamCloseCbFree which frees priv->path and priv, but does not access dev. So it might be save to do not hold this while calling.

There might be other issues with this e.g. if my assumption that the lock is ok to not be held for the callback is wrong, but for now that is worth a try.