Liron Aravot has posted comments on this change. Change subject: core: Remove global lock on SPM calls from engine side ......................................................................
Patch Set 5: (1 inline comment) .................................................... File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java Line 442: } Line 443: Line 444: public boolean failover(Guid currentCommandSpmId, Set<Guid> mTriedVdssList) { Line 445: synchronized (syncObj) { Line 446: mTriedVdssList.add(currentCommandSpmId); I asked because not all blocks that update it (mCurrentVdsId) perform synchronization on the same block now , for example seems to me that it can be updated from Execute method here (getIrsProxy) - was previously done under same synchronized to syncobject and now it doesn't. I know what volatile is, but if you update the variable not always under the same synchronized block, you may have obselete values. are you sure that it cannot be updated not under this lock and therefore cause you a race and wrong values here? otherwise if it can it should be volatile regardless of this lock. regarding the 'splitted' review - you are right, next time will do my best to have it all in one review. Line 447: if (currentCommandSpmId.equals(mCurrentVdsId)) { Line 448: if (getHasVdssForSpmSelection(mCurrentVdsId, mTriedVdssList)) { Line 449: Guid vdsId = mCurrentVdsId; Line 450: nullifyInternalProxies(); -- To view, visit http://gerrit.ovirt.org/9298 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9779fa4b5a4da2ef3ca1d62fe5203a3949ce278 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Daniel Paikov <pai...@gmail.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches