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

Reply via email to