Saggi Mizrahi has posted comments on this change. Change subject: BZ#754445 - Defer grant until all resource manager locks are freed ......................................................................
Patch Set 2: After talking with Danken I see there is a proper misunderstanding. First of all the fix is a two liner using the same trick used in registerResource to prevent the very same problem. This is not a big complex change. It should have been there in the first place. It's part of the desing. What Ayal is suggesting would work, but it would only work for this particular instance which is not the true case the callbacks where designed for. So it's not idiomatic. The resource should be put in the callback. This is the only logical way it can work otherwise you wouldn't have an resource object to release(). There is no reason we should prevent release() in the callback. If the operation is simple there is no need to put over complicated queues and messaging and waiting threads and other stuff to actually be able to release the resource once you're done with it. I really don't see why there was even an argument, a two line proper fix is better then any hack. Even if I didn't stress all of the above, callbacks should not be trusted because you have no idea what they do so you should try and keep yourself from deadlocking because of them. -- To view, visit http://gerrit.ovirt.org/254 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic12d170e64399e37a555960c03804778ad7d053b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
