Martin Sivák has posted comments on this change.

Change subject: core: Change how Affinity behaves if the assumptions are invalid
......................................................................


Patch Set 3:

(2 comments)

The refactoring is the side effect of the fix. They need to be together, 
because I am changing the looping. I readded the improved audit log reporting 
in a later patch already.

http://gerrit.ovirt.org/#/c/26619/3/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 98:         // contradicting rules to the log
Line 99:         unacceptableHosts.retainAll(acceptableHosts);
Line 100:         for (Guid id: unacceptableHosts) {
Line 101:             log.warnFormat("Host {1} ({2}) belongs to both positive 
and negative affinity list" +
Line 102:                     " while scheduling VM {3} ({4})",
> both? when acceptableHosts.isEmpty() == true, this log msg is inaccurate.
When acceptableHosts is empty, then the intersection is empty as well.
Line 103:                     hostMap.get(id).getName(), id.toString(),
Line 104:                     vm.getName(), vm.getId());
Line 105:         }
Line 106: 


Line 111:         else if (acceptableHosts.size() > 1) {
Line 112:             log.warnFormat("Invalid affinity situation was detected 
while scheduling VM {1} ({2})." +
Line 113:                     " VMs belonging to the same affinity groups are 
running on more than one host.",
Line 114:                     vm.getName(), vm.getId());
Line 115:         }
> Here acceptableHosts.size() > 1, means we should filter out all hosts.
No, that is the point of this patch. When the size is > 1, it means we have 
something wrong in the cluster and we have to allow the user to consolidate the 
VMs and fix the situation.

If you filter out all the hosts, you will prohibit any migrations and therefore 
no fix will be possible.
Line 116: 
Line 117:         // Remove hosts that contain VMs with negaive affinity to the 
currently scheduled Vm
Line 118:         for (Guid id : allVmIdsNegative) {
Line 119:             VM runVm = runningVMsMap.get(id);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I506bcb6e96c622694e6bf7b8ce7a0f54a82e5713
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Jiří Moskovčák <[email protected]>
Gerrit-Reviewer: Kobi Ianko <[email protected]>
Gerrit-Reviewer: Martin Sivák <[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