Omer Frenkel has posted comments on this change.

Change subject: vdsbroker: reduced scope of synchronized blocks
......................................................................


Patch Set 1:

(1 comment)

i dont see why changing the lock in vdsId..CommandBase would make a big 
difference, we anyway lock for the whole execution.
i think the change risky, and we loose the nice way we had to instantly know 
which commands are using the monitoring lock and how to easily use it for new 
commands.

https://gerrit.ovirt.org/#/c/37947/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java:

Line 554: mFailedToRunVmAttempts
what is the risk of using mFailedToRunVmAttempts outside of the lock? did you 
check the scenarios that use this?


-- 
To view, visit https://gerrit.ovirt.org/37947
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1bfd7b1fc7bfcc6465eae62feda6f1a27ff455
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tim Speetjens <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to