Alona Kaplan has posted comments on this change. Change subject: engine: move speed property from VdsNetworkInterface to VdsNetworkStatistics ......................................................................
Patch Set 7: (10 comments) http://gerrit.ovirt.org/#/c/28913/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/Nic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/Nic.java: Line 11: speed IMO speed shouldn't be part of this ctor and should be initialized in VdsNetworkInterface level (Nic, Vlan and Bond can have speed property so there is no reason to pass it separately to the Nic ctor). http://gerrit.ovirt.org/#/c/28913/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VdsNetworkStatistics.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VdsNetworkStatistics.java: Line 75: if Please use ObjectUtils.objectsEqual(speed, other.speed) Line 82: if Please use ObjectUtils.objectsEqual(vdsId, other.vdsId) http://gerrit.ovirt.org/#/c/28913/7/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java: Line 298: entity The speed mapping should be here. Line 346: iface.setBondType(bondType); Line 347: iface.setBondName(bondName); Line 348: iface.setBonded(isBond); Line 349: iface.setBondOptions(bondOptions); Line 350: iface.getStatistics().setSpeed(speed); not here (IIUC, this if will never be reached. Vlan device cannot be bond). Line 351: } else if (Boolean.TRUE.equals(isBond)) { Line 352: iface = new Bond(macAddress, bondOptions, bondType); Line 353: } else if (vlanId != null) { Line 354: iface = new Vlan(vlanId, baseInterface); Line 352: iface = new Bond(macAddress, bondOptions, bondType); Line 353: } else if (vlanId != null) { Line 354: iface = new Vlan(vlanId, baseInterface); Line 355: } else { Line 356: iface = new Nic(macAddress, speed, bondName); And not as part of the Nic ctor as I wrote in Nic.java. Line 357: } Line 358: Line 359: return iface; Line 360: } http://gerrit.ovirt.org/#/c/28913/7/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/network/InterfaceDaoTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/network/InterfaceDaoTest.java: Line 48: newQos.setInboundAverage(30); Line 49: newQos.setInboundPeak(30); Line 50: newQos.setInboundBurst(30); Line 51: Line 52: newVdsStatistics = new VdsNetworkStatistics(); There are specific tests for statistics- testMasshUpdateStatisticsForVds() testRemoveStatisticsForVds() testUpdateStatisticsForVds() Testing the speed should be part of them. The current method should remain as is- just remove the setSpeed line. Line 53: newVdsStatistics.setSpeed(1000); Line 54: Line 55: newVdsInterface = new VdsNetworkInterface(); Line 56: newVdsInterface.setStatistics(newVdsStatistics); http://gerrit.ovirt.org/#/c/28913/7/backend/manager/modules/dal/src/test/resources/fixtures.xml File backend/manager/modules/dal/src/test/resources/fixtures.xml: Line 4878: <value>0</value> Line 4879: <value>0</value> Line 4880: <value>0</value> Line 4881: <value>0</value> Line 4882: <value>0</value> speed? Line 4883: <null /> Line 4884: </row> Line 4885: <row> Line 4886: <value>ba31682e-6ae7-4f9d-8c6f-04c93acca9dd</value> http://gerrit.ovirt.org/#/c/28913/7/packaging/dbscripts/network_sp.sql File packaging/dbscripts/network_sp.sql: Line 348: Please do the formatting in a separate patch. Line 893: same -- To view, visit http://gerrit.ovirt.org/28913 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If4be28a786bee1eae6367c4ef19c9ee3b4afec61 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Yaniv Dary <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
