Doron Fediuck has posted comments on this change.
Change subject: WIP core: adding a filter policy for CPU level
......................................................................
Patch Set 1:
(3 comments)
Oved, please see inline comments.
Also it seems that all lower should be higher?
As for default, it should be a db upgrade which adds this filter to the
standard policies.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/CpuLevelFilterPolicyUnit.java
Line 25: int compareResult =
CpuFlagsManagerHandler.compareCpuLevels(vm.getCpuName(),
host.getCpuName().getCpuName(), vm.getVdsGroupCompatibilityVersion());
Line 26: if (compareResult >= 0) {
Line 27: hostsToRunOn.add(host);
Line 28: } else {
Line 29:
messages.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CPU_LEVEL.toString());
This will add a message per host.
So if we have 100 hosts and 2 are fine, we still get 98 messages.
Wouldn't it be better to collect all failed hosts to the same message?
Also, please check with Martin Sivak how he makes sure to collect all
filtered-out hosts, so when there are no resources we present an informative
message per filter.
Line 30: log.debugFormat("Host {0} has CPU level ({1})
which is lower than the CPU level the VM was run with ({2})",
Line 31: host.getName(),
Line 32: host.getCpuName(),
Line 33: vm.getCpuName());
Line 26: if (compareResult >= 0) {
Line 27: hostsToRunOn.add(host);
Line 28: } else {
Line 29:
messages.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CPU_LEVEL.toString());
Line 30: log.debugFormat("Host {0} has CPU level ({1})
which is lower than the CPU level the VM was run with ({2})",
Based on the code it should be-
s/lower/higher/ ???
Line 31: host.getName(),
Line 32: host.getCpuName(),
Line 33: vm.getCpuName());
Line 34: }
Line 33: vm.getCpuName());
Line 34: }
Line 35: }
Line 36:
Line 37: return hostsToRunOn;
Before you return, please log to file all hosts you selected here with an info
that they have a lower cpu level.
Line 38: } else {
Line 39: return hosts;
Line 40: }
Line 41: }
--
To view, visit http://gerrit.ovirt.org/19563
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42b803fe0d2e9389d10196cd44c3fd21342927f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@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
