Moti Asayag has posted comments on this change.

Change subject: engine: Make NetworkPolicyUnit aware of multiple clusters
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/27990/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java:

Line 29
Line 30
Line 31
Line 32
Line 33
> Well I killed the variables in the patch (your comment is in the existing c
Any cache for this class will be error-prone. This is a non synchronized class 
which is invoked in a multi-threaded environment. I can't see a way to solve it 
safely without introducing synchronization or changing this class not to have 
any state (my preferred option).

So I'd suggest to find the displayNetwork in the filter() method and pass it 
all the way through to validateDisplayNetworkAvailability().

As a side note, I think that PolicyUnitImpl should include some javadoc to 
describe that restriction.


-- 
To view, visit http://gerrit.ovirt.org/27990
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6e85a81fa4a2248750f480e85e63096c118d25cb
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Martin Sivák <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to