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 <smizrahi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>