Roy Golan has posted comments on this change. Change subject: engine: forbid single (vm mem) > (host mem) ......................................................................
Patch Set 5: (6 comments) please add unit test on that method. overall is ok. except from swap null check the rest of comments are cosmetic. http://gerrit.ovirt.org/#/c/37429/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SlaValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SlaValidator.java: Line 18: Line 19: public boolean hasMemoryToRunVM(VDS curVds, VM vm) { Line 20: boolean retVal = false; Line 21: if (curVds.getMemCommited() != null && curVds.getPhysicalMemMb() != null && curVds.getReservedMem() != null Line 22: && curVds.getSwapTotal() != null) { swap should be removed Line 23: double vdsCurrentMem = getVdsCurrentReservedMemory(curVds) + vm.getMinAllocatedMem(); Line 24: double vdsMemLimit = getVdsMemLimit(curVds); Line 25: log.debug("hasMemoryToRunVM: host '{}' physical vmem size is : {}, swap total vmem size is : {} MB", Line 26: curVds.getName(), Line 24: double vdsMemLimit = getVdsMemLimit(curVds); Line 25: log.debug("hasMemoryToRunVM: host '{}' physical vmem size is : {}, swap total vmem size is : {} MB", Line 26: curVds.getName(), Line 27: curVds.getPhysicalMemMb(), Line 28: curVds.getSwapTotal()); and here Line 29: log.debug("Host Mem Conmmitted: '{}', pending vmem size is : {}, Host Guest Overhead {}, Host Reserved Mem: {}, VM Min Allocated Mem {}", Line 30: curVds.getMemCommited(), Line 31: curVds.getPendingVmemSize(), Line 32: curVds.getGuestOverhead(), Line 39: } Line 40: Line 41: public int getHostAvailableMemoryLimit(VDS curVds) { Line 42: if (curVds.getMemCommited() != null && curVds.getPhysicalMemMb() != null && curVds.getReservedMem() != null Line 43: && curVds.getSwapTotal() != null) { swap should be removed Line 44: double vdsCurrentMem = getVdsCurrentReservedMemory(curVds); Line 45: double vdsMemLimit = getVdsMemLimit(curVds); Line 46: return (int) (vdsMemLimit - vdsCurrentMem); Line 47: } Line 50: Line 51: private double getVdsMemLimit(VDS curVds) { Line 52: // if single vm on host. Disregard memory over commitment Line 53: int computedMemoryOverCommit = (curVds.getVmCount() == 0) ? 100 : curVds.getMaxVdsMemoryOverCommit(); Line 54: double vdsMemLimit = computedMemoryOverCommit * curVds.getPhysicalMemMb() / 100.0; also here the variable can be left outuua Line 55: return vdsMemLimit; Line 56: } Line 57: Line 58: private double getVdsCurrentReservedMemory(VDS curVds) { Line 54: double vdsMemLimit = computedMemoryOverCommit * curVds.getPhysicalMemMb() / 100.0; Line 55: return vdsMemLimit; Line 56: } Line 57: Line 58: private double getVdsCurrentReservedMemory(VDS curVds) { method name is confusing. have a property called vds.reservedMem... can it be just getUsedMem Line 59: double vdsCurrentMem = Line 60: curVds.getMemCommited() + curVds.getPendingVmemSize() + curVds.getGuestOverhead() Line 61: + curVds.getReservedMem(); Line 62: return vdsCurrentMem; Line 55: return vdsMemLimit; Line 56: } Line 57: Line 58: private double getVdsCurrentReservedMemory(VDS curVds) { Line 59: double vdsCurrentMem = spare the variable and just return the answer? would be easily inlined as well Line 60: curVds.getMemCommited() + curVds.getPendingVmemSize() + curVds.getGuestOverhead() Line 61: + curVds.getReservedMem(); Line 62: return vdsCurrentMem; Line 63: } -- To view, visit http://gerrit.ovirt.org/37429 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5f5280c43820732a36a235024ec5c887c9fcb98 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Dudi Maroshi <d...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Dudi Maroshi <d...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.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
