Yevgeny Zaspitsky 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 VdsNe Done 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) Done Line 82: if > Please use ObjectUtils.objectsEqual(vdsId, other.vdsId) Done 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. Done 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). Done 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. Done 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- Done 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? Done 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. Done Line 893: > same Done -- 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
