Tomer Saban has posted comments on this change.

Change subject: Added affinity Group collision test to canDoAction
......................................................................


Patch Set 3:

(5 comments)

Added some comments after talking with Martin.

https://gerrit.ovirt.org/#/c/41805/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java:

Line 82:         Set<Set<VM>> uag = new HashSet();
Line 83: 
Line 84:         VmDAO vm_dao = DbFacade.getInstance().getVmDao();
Line 85: 
Line 86:         //uag = {{vm} for each vm}
change to comment (only adding VMs for each vm in any affinity group
Line 87:         for(AffinityGroup ag: affinityGroups) {
Line 88:             for(Guid id : ag.getEntityIds()) {
Line 89:                 Set<VM> temp = new HashSet<VM>();
Line 90:                 temp.add(vm_dao.get(id));


Line 90:                 temp.add(vm_dao.get(id));
Line 91:                 uag.add(temp);
Line 92:             }
Line 93:         }
Line 94: 
Change "unify" to "merge"
Line 95:         //# For each (+) affinity group(Sorted by group id):
Line 96:         //## unify VMs from the group in UAG(Sorted by vm id).
Line 97:         for(AffinityGroup ag : affinityGroups) {
Line 98:             if(ag.isPositive()) {


Line 92:             }
Line 93:         }
Line 94: 
Line 95:         //# For each (+) affinity group(Sorted by group id):
Line 96:         //## unify VMs from the group in UAG(Sorted by vm id).
Clarify how the unification algorithm works.
Line 97:         for(AffinityGroup ag : affinityGroups) {
Line 98:             if(ag.isPositive()) {
Line 99:                 Set<VM> vm_set;
Line 100:                 vm_set = new HashSet<VM>();


Line 96:         //## unify VMs from the group in UAG(Sorted by vm id).
Line 97:         for(AffinityGroup ag : affinityGroups) {
Line 98:             if(ag.isPositive()) {
Line 99:                 Set<VM> vm_set;
Line 100:                 vm_set = new HashSet<VM>();
s 's/vm_set/mergeToUag/g' (Or to something involving merging
Line 101: 
Line 102:                 for(Guid id : ag.getEntityIds()) {
Line 103:                     Set<VM> vm_group = getVmGroupByGuid(uag, 
vm_dao.get(id));
Line 104:                     vm_set.addAll(vm_group);


Line 99:                 Set<VM> vm_set;
Line 100:                 vm_set = new HashSet<VM>();
Line 101: 
Line 102:                 for(Guid id : ag.getEntityIds()) {
Line 103:                     Set<VM> vm_group = getVmGroupByGuid(uag, 
vm_dao.get(id));
-make getVmGroupByGuid remove the group.
-use Camel case for variables(Naming conventions).
Line 104:                     vm_set.addAll(vm_group);
Line 105:                 }
Line 106: 
Line 107:                 uag.add(vm_set);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56c483023ed14e238c8a67ec0890812158ecd0b2
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomer Saban <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tomer Saban <[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