Gilad Chaplik has posted comments on this change. Change subject: core: scheduling: VM Affinity Group filter and weight module ......................................................................
Patch Set 5: (4 comments) http://gerrit.ovirt.org/#/c/22793/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java: Line 83: // Only one host is allowed for positive affinity, i.e. if the VM contained in a positive Line 84: // affinity group he must run on the host that all the other members are running, if the Line 85: // VMs spread across hosts, the affinity rule isn't applied. Line 86: } else if (acceptableHosts.size() > 1) { Line 87: messages.add(VdcBllMessages.ACTION_TYPE_FAILED_POSITIVE_AFFINITY_GROUP.toString()); > Which host / group? Done Line 88: return null; Line 89: } Line 90: // Handle negative affinity Line 91: for (Guid id : allVmIdsNegative) { Line 90: // Handle negative affinity Line 91: for (Guid id : allVmIdsNegative) { Line 92: VM runVm = runningVMsMap.get(id); Line 93: if (runVm != null && runVm.getRunOnVds() != null) { Line 94: acceptableHosts.remove(runVm.getRunOnVds()); > Please aggregate the removal into a string which will be outputted in a log Done Line 95: } Line 96: } Line 97: if (acceptableHosts.isEmpty()) { Line 98: if (hasPositiveConstraint) { Line 97: if (acceptableHosts.isEmpty()) { Line 98: if (hasPositiveConstraint) { Line 99: messages.add(VdcBllMessages.ACTION_TYPE_FAILED_MIX_POSITIVE_NEGATIVE_AFFINITY_GROUP.toString()); Line 100: } else { Line 101: messages.add(VdcBllMessages.ACTION_TYPE_FAILED_NEGATIVE_AFFINITY_GROUP.toString()); > This should have the list from above. Done Line 102: } Line 103: return null; Line 104: } Line 105: Line 105: Line 106: List<VDS> retList = new ArrayList<VDS>(); Line 107: for (VDS host : hosts) { Line 108: if (acceptableHosts.contains(host.getId())) { Line 109: retList.add(host); > RetainAll should make this loop (and similar) redundant RetainAll uses object.equals, and here I'm comparing vds to guid. java doesn't let you set the comparator. so in order to use it I need to transform the vds list into guid list, use RetainAll, and then transform it back to vds list. I guess it doesn't worth the trouble. Line 110: } Line 111: } Line 112: Line 113: return retList; -- To view, visit http://gerrit.ovirt.org/22793 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9716523591852635496799412ad387850d6c372d Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
