Liran Zelkha has posted comments on this change. Change subject: core: Make VDS a JPA entity ......................................................................
Patch Set 43: (11 comments) https://gerrit.ovirt.org/#/c/36600/43/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNodeCPUMap.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNodeCPUMap.java: Line 18: */ Line 19: @Entity Line 20: @Table(name = "numa_node_cpu_map") Line 21: @Cacheable(true) Line 22: public class NumaNodeCPUMap implements Serializable, BusinessEntity<Guid> { > s/NumaNodeCPUMap/NumaNodeCpuMap Done Line 23: Line 24: private static final long serialVersionUID = -683066053231559224L; Line 25: Line 26: @Id Line 20: @Table(name = "numa_node_cpu_map") Line 21: @Cacheable(true) Line 22: public class NumaNodeCPUMap implements Serializable, BusinessEntity<Guid> { Line 23: Line 24: private static final long serialVersionUID = -683066053231559224L; > this id is already in use by VdsNumaNode. Please generate a new one. Done Line 25: Line 26: @Id Line 27: @Column(name = "id") Line 28: @Type(type = "org.ovirt.engine.core.dao.jpa.GuidUserType") Line 35: @Column(name = "cpu_core_id") Line 36: private int cpuCoreId; Line 37: Line 38: public NumaNodeCPUMap() { Line 39: super(); > not needed Done Line 40: } Line 41: Line 42: public NumaNodeCPUMap(Guid id, Guid numaNodeId, int cpuCoreId) { Line 43: super(); Line 39: super(); Line 40: } Line 41: Line 42: public NumaNodeCPUMap(Guid id, Guid numaNodeId, int cpuCoreId) { Line 43: super(); > not needed Done Line 44: this.id = id; Line 45: this.numaNodeId = numaNodeId; Line 46: this.cpuCoreId = cpuCoreId; Line 47: } https://gerrit.ovirt.org/#/c/36600/43/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 89: this.cpuUsagePercent = cpuUsagePercent; Line 90: } Line 91: Line 92: @Override Line 93: public int hashCode() { > shouldn't the hashCode() and equals() be chagned as well according to the g Done Line 94: final int prime = 31; Line 95: int result = 1; Line 96: long temp; Line 97: temp = Double.doubleToLongBits(cpuIdle); https://gerrit.ovirt.org/#/c/36600/43/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNodeVmVds.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NumaNodeVmVds.java: Line 46: @Column(name = "is_pinned") Line 47: private boolean pinned; Line 48: Line 49: public NumaNodeVmVds() { Line 50: super(); > not needed. Done Line 51: } Line 52: Line 53: public NumaNodeVmVds(Guid id, VmNumaNode vmNumaNode, boolean pinned, Integer nodeIndex) { Line 54: this.id = id; https://gerrit.ovirt.org/#/c/36600/43/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 245: @Column(name = "is_numa_supported") Line 246: private boolean numaSupport; Line 247: Line 248: @Column(name = "supported_rng_sources") Line 249: @Type( > please merge lines Done Line 250: type = "org.ovirt.engine.core.dao.jpa.VmRngDeviceSourceUserType") Line 251: private Set<VmRngDevice.Source> supportedRngSources; Line 252: Line 253: @Column(name = "maintenance_reason") Line 258: // Set of additional features supported by the VDSM. Line 259: private transient Set<String> additionalFeatures; Line 260: Line 261: @PostLoad Line 262: protected void afterLoad() { > isn't it possible to annotate the method both with @PostLoad and @PrePersis Both methods are not the same. The PostLoad makes the DB String a Set, and the PrePersist makes the Set a DB String. They are inverted. Line 263: supportedClusterVersionsSet = parseSupportedVersions(getSupportedClusterLevels()); Line 264: supportedEngineVersionsSet = parseSupportedVersions(getSupportedEngines()); Line 265: } Line 266: https://gerrit.ovirt.org/#/c/36600/43/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsNumaNode.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsNumaNode.java: Line 52: private Guid id; Line 53: Line 54: @Column(name = "vds_id") Line 55: @Type(type = "org.ovirt.engine.core.dao.jpa.GuidUserType") Line 56: private Guid vdsId; > s/vdsId/hostId ? Not sure. It's VDS in the DB... Line 57: Line 58: @Column(name = "vm_id") Line 59: @Type(type = "org.ovirt.engine.core.dao.jpa.GuidUserType") Line 60: private Guid vmId; Line 64: Line 65: private transient List<Integer> cpuIds; Line 66: Line 67: @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER, mappedBy = "numaNodeId") Line 68: @LazyCollection(LazyCollectionOption.FALSE) > isn't it redundant if the fetch type is EAGER ? Done Line 69: private List<NumaNodeCPUMap> cpus; Line 70: Line 71: @Column(name = "mem_total") Line 72: private long memTotal; https://gerrit.ovirt.org/#/c/36600/43/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsNumaNodeDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsNumaNodeDAODbFacadeImpl.java: Line 32: fillCpuList(vdsNumaNodes); Line 33: return vdsNumaNodes; Line 34: } Line 35: Line 36: protected void fillCpuList(List<? extends VdsNumaNode> vdsNumaNodes) { > wouldn't having that code in a method annotated with @PostLoad inside the b Done Line 37: for (VdsNumaNode node : vdsNumaNodes) { Line 38: if (node.getCpus() != null) { Line 39: List<Integer> cpuIds = new ArrayList<>(); Line 40: for (NumaNodeCPUMap cpu : node.getCpus()) { -- To view, visit https://gerrit.ovirt.org/36600 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d7aa140e20dcd9468ae1ed00af1df0cf3e8b9e6 Gerrit-PatchSet: 43 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
