Gilad Chaplik has posted comments on this change. Change subject: engine: Numa feature entities ......................................................................
Patch Set 12: (29 comments) good start. 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 12: public class CpuStatistics implements Serializable { Line 13: Line 14: private static final long serialVersionUID = 3274786304152401306L; Line 15: Line 16: private Guid id; can you explain why you added id here? Line 17: Line 18: private Guid vdsId; Line 19: Line 20: private Guid vdsNumaNodeId; Line 14: private static final long serialVersionUID = 3274786304152401306L; Line 15: Line 16: private Guid id; Line 17: Line 18: private Guid vdsId; not needed, this class is contained by VDS. Line 19: Line 20: private Guid vdsNumaNodeId; Line 21: Line 22: private int cpuCoreId; Line 16: private Guid id; Line 17: Line 18: private Guid vdsId; Line 19: Line 20: private Guid vdsNumaNodeId; numa id is GUID? need to think if numa id is relevant to cpu statistics. Line 21: Line 22: private int cpuCoreId; Line 23: Line 24: private BigDecimal cpuSys; Line 24: private BigDecimal cpuSys; Line 25: Line 26: private BigDecimal cpuUser; Line 27: Line 28: private BigDecimal cpuIdle; why not using native objects (double)? ^^^ Line 29: Line 30: private int cpuUsagePercent; Line 31: Line 32: public CpuStatistics(){ Line 157: return false; Line 158: } else if (!vdsNumaNodeId.equals(other.vdsNumaNodeId)) Line 159: return false; Line 160: return true; Line 161: } +1 for equals and hashcode. I hope it's generated :-) Line 162: Line 163: @Override Line 164: public String toString() { Line 165: return "CpuStatistics [id=" + id + ", vdsId=" + vdsId + ", vdsNumaNodeId=" + vdsNumaNodeId + ", cpuCoreId=" Line 160: return true; Line 161: } Line 162: Line 163: @Override Line 164: public String toString() { I think toString() can be added by demand. Line 165: return "CpuStatistics [id=" + id + ", vdsId=" + vdsId + ", vdsNumaNodeId=" + vdsNumaNodeId + ", cpuCoreId=" Line 166: + cpuCoreId + ", cpuSys=" + cpuSys + ", cpuUser=" + cpuUser + ", cpuIdle=" + cpuIdle Line 167: + ", cpuUsagePercent=" + cpuUsagePercent + "]"; Line 168: } 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 9: /** Line 10: * Object which represents vm virtual NUMA node information Line 11: * Line 12: */ Line 13: public class GuestNumaNode implements Serializable { why we have 2 classes for NUMA node? Line 14: Line 15: private static final long serialVersionUID = 3751844852367478102L; Line 16: Line 17: private Guid id; Line 13: public class GuestNumaNode implements Serializable { Line 14: Line 15: private static final long serialVersionUID = 3751844852367478102L; Line 16: Line 17: private Guid id; need to think if we need an id here. Line 18: Line 19: private Guid vmGuid; Line 20: Line 21: private int index; Line 15: private static final long serialVersionUID = 3751844852367478102L; Line 16: Line 17: private Guid id; Line 18: Line 19: private Guid vmGuid; contained by VM, no need for this. Line 20: Line 21: private int index; Line 22: Line 23: private String vcpuIds; Line 19: private Guid vmGuid; Line 20: Line 21: private int index; Line 22: Line 23: private String vcpuIds; plural? string? Line 24: Line 25: private int vcpuCount; Line 26: Line 27: private long memTotal; Line 25: private int vcpuCount; Line 26: Line 27: private long memTotal; Line 28: Line 29: private List<NumaNode> pinnedNumaNodeList; id is sufficient no? Line 30: Line 31: public GuestNumaNode() { Line 32: vcpuCount = 0; Line 33: memTotal = 0L; Line 146: public String toString() { Line 147: return "GuestNumaNode [id=" + id + ", vmGuid=" + vmGuid + ", index=" + index + ", vcpuIds=" + vcpuIds Line 148: + ", vcpuCount=" + vcpuCount + ", memTotal=" + memTotal + ", pinnedNumaNodeList=" + pinnedNumaNodeList Line 149: + "]"; Line 150: } same. Line 151: http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNode.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNode.java: Line 11: public class NumaNode implements Serializable { Line 12: Line 13: private static final long serialVersionUID = -683066053231559224L; Line 14: Line 15: private Guid id; same q for id. Line 16: Line 17: private Guid vdsGuid; Line 18: Line 19: private int index; Line 13: private static final long serialVersionUID = -683066053231559224L; Line 14: Line 15: private Guid id; Line 16: Line 17: private Guid vdsGuid; same. Line 18: Line 19: private int index; Line 20: Line 21: private String cpuIds; Line 17: private Guid vdsGuid; Line 18: Line 19: private int index; Line 20: Line 21: private String cpuIds; same. Line 22: Line 23: private int cpuCount; Line 24: Line 25: private long memTotal; Line 140: @Override Line 141: public String toString() { Line 142: return "NumaNode [id=" + id + ", vdsGuid=" + vdsGuid + ", index=" + index + ", cpuIds=" + cpuIds Line 143: + ", cpuCount=" + cpuCount + ", memTotal=" + memTotal + ", vNumaNodeCount=" + vNumaNodeCount + "]"; Line 144: } same. Line 145: http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNodeStatistics.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNodeStatistics.java: Line 12: public class NumaNodeStatistics implements Serializable { Line 13: Line 14: private static final long serialVersionUID = 3274786304152401306L; Line 15: Line 16: private Guid id; same. Line 17: Line 18: private long memFree; Line 19: Line 20: private BigDecimal cpuSys; Line 20: private BigDecimal cpuSys; Line 21: Line 22: private BigDecimal cpuUser; Line 23: Line 24: private BigDecimal cpuIdle; same (double) ^^^ Line 25: Line 26: private int memUsagePercent; Line 27: Line 28: private int cpuUsagePercent; Line 24: private BigDecimal cpuIdle; Line 25: Line 26: private int memUsagePercent; Line 27: Line 28: private int cpuUsagePercent; lets use real num for %, and let the presentation layer decide on format. Line 29: Line 30: public NumaNodeStatistics(){ Line 31: memFree = 0L; Line 32: cpuSys = BigDecimal.ZERO; 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 5: Line 6: public enum NumaTuneMode { Line 7: STRICT, Line 8: INTERLEAVE, Line 9: PREFERRED; javadoc Line 10: Line 11: Line 12: private String value; Line 13: private static Map<String, NumaTuneMode> mappings; 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(); I'm for numa in VdsStatic. Line 1319: } Line 1320: Line 1321: public List<NumaNode> getNumaNodeList() { Line 1322: return mVdsDynamic.getNumaNodeList(); Line 1322: return mVdsDynamic.getNumaNodeList(); Line 1323: } Line 1324: Line 1325: public List<NumaNodeStatistics> getNumaNodeStatisticsList() { Line 1326: return mVdsStatistics.getNumaNodeStatisticsList(); why stats is separated from NumaNode ? Line 1327: } Line 1328: Line 1329: public List<CpuStatistics> getCpuStatisticsList() { Line 1330: return mVdsStatistics.getCpuStatisticsList(); Line 1330: return mVdsStatistics.getCpuStatisticsList(); Line 1331: } Line 1332: Line 1333: public String getNumaNodeDistance() { Line 1334: return mVdsDynamic.getNumaNodeDistance(); javadoc. Line 1335: } Line 1336: Line 1337: public Boolean getAutoNumaBalancing() { Line 1338: return mVdsDynamic.getAutoNumaBalancing(); Line 1333: public String getNumaNodeDistance() { Line 1334: return mVdsDynamic.getNumaNodeDistance(); Line 1335: } Line 1336: Line 1337: public Boolean getAutoNumaBalancing() { native (boolean). javadoc. Line 1338: return mVdsDynamic.getAutoNumaBalancing(); Line 1339: } Line 1340: http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java: Line 1703: vmStatic.setCustomSerialNumber(customSerialNumber); Line 1704: } Line 1705: Line 1706: public NumaTuneMode getNumaTuneMode() { Line 1707: return this.vmStatic.getNumaTuneMode(); 'this' is needed? Line 1708: } Line 1709: Line 1710: public void setNumaTuneMode(NumaTuneMode numaTuneMode) { Line 1711: this.vmStatic.setNumaTuneMode(numaTuneMode); Line 1715: return this.vmStatic.getvNumaNodeCount(); Line 1716: } Line 1717: Line 1718: public List<NumaNode> getNumaTuneNodeList() { Line 1719: return this.vmStatic.getNumaTuneNodeList(); I don't follow, why the vm need to hold Host numa nodes? Line 1720: } Line 1721: Line 1722: public List<GuestNumaNode> getvNumaNodeList() { Line 1723: return this.vmStatic.getvNumaNodeList(); Line 1723: return this.vmStatic.getvNumaNodeList(); Line 1724: } Line 1725: Line 1726: public boolean isNumaManualBinding() { Line 1727: return this.vmStatic.isNumaManualBinding(); javadoc. Line 1728: } Line 1729: Line 1730: public void setNumaManualBinding(boolean numaManualBinding) { Line 1731: this.vmStatic.setNumaManualBinding(numaManualBinding); http://gerrit.ovirt.org/#/c/23702/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsDynamic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsDynamic.java: Line 150: private int numaNodeCount; Line 151: Line 152: private List<NumaNode> numaNodeList; Line 153: Line 154: private String numaNodeDistance; javadoc. Line 155: Line 156: private Boolean autoNumaBalancing; Line 157: Line 158: public void setVersion(RpmVersion value) { Line 152: private List<NumaNode> numaNodeList; Line 153: Line 154: private String numaNodeDistance; Line 155: Line 156: private Boolean autoNumaBalancing; javadoc. boolean Line 157: Line 158: public void setVersion(RpmVersion value) { Line 159: rpmVersion = value; Line 160: } -- 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: 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
