Doron Fediuck has posted comments on this change.

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


Patch Set 3:

(2 comments)

1. Is it verified? If so, please mark as such.

2. For future work, please  see inline.

http://gerrit.ovirt.org/#/c/28467/3/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 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
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",
This is a really confusing message, as it may occur for several reasons.

I suggest that as a followup bug this will be enhanced to:

- Real reason
- Relevant details: no numa configured / needed node size XXX, only YYY 
available.
Line 58:                         vds.getName());
Line 59:                 messages.addMessage(vds.getId(), 
VdcBllMessages.VAR__DETAIL__NOT_ENOUGH_MEMORY.toString());
Line 60:                 continue;
Line 61:             }


Line 63:         }
Line 64:         return list;
Line 65:     }
Line 66: 
Line 67:     private boolean canVmNumaPinnedToVds(VM vm, List<VmNumaNode> 
nodes, VDS vds) {
This will become another 'black hole' where we cannot explain why something was 
ruled out. Should be improved later on.
Line 68:         List<VdsNumaNode> pNodes = 
DbFacade.getInstance().getVdsNumaNodeDAO().getAllVdsNumaNodeByVdsId(vds.getId());
Line 69:         if (pNodes == null || pNodes.isEmpty()) {
Line 70:             return false;
Line 71:         }


-- 
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: 3
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: 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