Gilad Chaplik has posted comments on this change.

Change subject: core: support NUMA in oVirt Scheduler
......................................................................


Patch Set 6:

(3 comments)

new patch to follow.

http://gerrit.ovirt.org/#/c/28467/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/MemoryPolicyUnit.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/MemoryPolicyUnit.java:

Line 51:             // * memory tune is strict (vNode memory cannot be spread 
across several pNodes' memory)
Line 52:             // [and]
Line 53:             // * host support NUMA configuration
Line 54:             // * there isn't enough memory for pinned vNode in pNode
Line 55:             if (isVmNumaPinned(vmNumaNodes) && vm.getNumaTuneMode() == 
NumaTuneMode.STRICT
> its better to chcek first if its STRICT (the cheapest check)
Done
Line 56:                     && (!vds.isNumaSupport() || 
!canVmNumaPinnedToVds(vm, vmNumaNodes, vds))) {
Line 57:                 log.debugFormat("host '{0}' cannot accommodate memory 
of VM's pinned virtual NUMA nodes within host's physical NUMA nodes",
Line 58:                         vds.getName());
Line 59:                 messages.addMessage(vds.getId(), 
VdcBllMessages.VAR__DETAIL__NOT_ENOUGH_MEMORY.toString());


Line 55:             if (isVmNumaPinned(vmNumaNodes) && vm.getNumaTuneMode() == 
NumaTuneMode.STRICT
Line 56:                     && (!vds.isNumaSupport() || 
!canVmNumaPinnedToVds(vm, vmNumaNodes, vds))) {
Line 57:                 log.debugFormat("host '{0}' cannot accommodate memory 
of VM's pinned virtual NUMA nodes within host's physical NUMA nodes",
Line 58:                         vds.getName());
Line 59:                 messages.addMessage(vds.getId(), 
VdcBllMessages.VAR__DETAIL__NOT_ENOUGH_MEMORY.toString());
> that's a very general error for this 4 steps condition
Done
Line 60:                 continue;
Line 61:             }
Line 62:             list.add(vds);
Line 63:         }


Line 80:                     }
Line 81:                 }
Line 82:             }
Line 83:         }
Line 84:         return true;
> why not create a boolean procedure which will check that for us?
do you have enough paper :-) ?
Line 85:     }
Line 86: 
Line 87:     private Map<Integer, VdsNumaNode> toMap(List<VdsNumaNode> pNodes) {
Line 88:         Map<Integer, VdsNumaNode> map = new HashMap<>();


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b41e5ac9d633953dec7bfab5621ace8bb1b13cd
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Chuan Liao <chuan.l...@hp.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Jason Liao <chuan.l...@hp.com>
Gerrit-Reviewer: Martin Sivák <msi...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Xiaolei Shi <xiao-lei....@hp.com>
Gerrit-Reviewer: automat...@ovirt.org
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

Reply via email to