Gilad Chaplik has posted comments on this change. Change subject: engine: Numa feature entities ......................................................................
Patch Set 12: (8 comments) > in general, CPU topology reporting is under work, thus we all should be in > sync - make sure you add Vinzenz to your patches both backend/vdsm +1. Vinzenz, please sync-up :) (hope I've added the right guy ;-) ) http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/CpuStatistics.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/CpuStatistics.java: Line 98: public void setCpuUsagePercent(int cpuUsagePercent) { Line 99: this.cpuUsagePercent = cpuUsagePercent; Line 100: } Line 101: Line 102: @Override > is really needed? IMO yes. Line 103: public int hashCode() { Line 104: final int prime = 31; Line 105: int result = 1; Line 106: result = prime * result + cpuCoreId; Line 114: return result; Line 115: } Line 116: Line 117: @Override Line 118: public boolean equals(Object obj) { > is really needed? IMO yes. Line 119: if (this == obj) Line 120: return true; Line 121: if (obj == null) Line 122: return false; http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/GuestNumaNode.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/GuestNumaNode.java: Line 28: Line 29: private List<NumaNode> pinnedNumaNodeList; Line 30: Line 31: public GuestNumaNode() { Line 32: vcpuCount = 0; > primitives int defaults to zero so this is redundant leave it as it is, unless Roy checks with Vojtech and verifies it's not needed. Line 33: memTotal = 0L; Line 34: pinnedNumaNodeList = new ArrayList<NumaNode>(); Line 35: } Line 36: http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaTuneMode.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaTuneMode.java: Line 12: private String value; Line 13: private static Map<String, NumaTuneMode> mappings; Line 14: Line 15: static { Line 16: mappings = new HashMap<String, NumaTuneMode>(); > valueOf of enum does the same. you can leave this enum with just the three since all enums are implement in this fashion, this is a cross change, please don't start here (ref: VMStatus.java). Line 17: for (NumaTuneMode mode : values()) { Line 18: mappings.put(mode.getValue(), mode); Line 19: } Line 20: } Line 19: } Line 20: } Line 21: Line 22: private NumaTuneMode() { Line 23: value= name().toLowerCase(); format. just in case, please format all files Line 24: } Line 25: Line 26: private NumaTuneMode(String value) { Line 27: this.value = value; http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java: Line 1314: mVdsStatic.setDisablePowerManagementPolicy(disablePowerManagementPolicy); Line 1315: } Line 1316: Line 1317: public int getNumaNodeCount() { Line 1318: return mVdsDynamic.getNumaNodeCount(); > @Gilad explain your point afaik, NUMA host topology is static. Line 1319: } Line 1320: Line 1321: public List<NumaNode> getNumaNodeList() { Line 1322: return mVdsDynamic.getNumaNodeList(); http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsStatistics.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsStatistics.java: Line 39: Line 40: private List<NumaNodeStatistics> numaNodeStatisticsList; Line 41: Line 42: private List<CpuStatistics> cpuStatisticsList; Line 43: > the same here. +1 ^^ Line 44: public VdsStatistics() { Line 45: this.cpu_idle = BigDecimal.ZERO; Line 46: this.cpu_load = BigDecimal.ZERO; Line 47: this.cpu_sys = BigDecimal.ZERO; http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java: Line 55: private Guid imageTypeId; Line 56: Line 57: @EditableField Line 58: private boolean useLatestVersion; Line 59: > if all those fields should be editable (which I guess they should) then the unless causes a regression, please add it in a later patch (data-entry patch) Line 60: private boolean numaManualBinding; Line 61: Line 62: private NumaTuneMode numaTuneMode; Line 63: -- To view, visit http://gerrit.ovirt.org/23702 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifefade432e7955503980bdc6fc5d73ea32818a95 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Xiaolei Shi <xiao-lei....@hp.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Eli Mesika <elimes...@gmail.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <gustavo.pedr...@eldorado.org.br> Gerrit-Reviewer: Jiří Moskovčák <jmosk...@redhat.com> Gerrit-Reviewer: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Leonardo Bianconi <leonardo.bianc...@eldorado.org.br> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Vitor de Lima <vitor.l...@eldorado.org.br> 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
