Lior Vernia has posted comments on this change. Change subject: engine: Enforce maximal QoS commitment on interface ......................................................................
Patch Set 10: (4 comments) http://gerrit.ovirt.org/#/c/34133/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java: Line 274: .get(network.getQosId()); Line 275: Line 276: Float baseInterfaceCommitment = relativeCommitmentByBaseInterface.get(baseIfaceName); Line 277: if (baseInterfaceCommitment == null) { Line 278: baseInterfaceCommitment = new Float(0); > why not- 0f? Done Line 279: } Line 280: Line 281: Integer subInterfaceCommitment = qos.getOutAverageRealtime(); Line 282: if (subInterfaceCommitment != null) { Line 330: if (speed != null) { Line 331: return speed; Line 332: } Line 333: Line 334: // not cached, compute and cache for future reference > In case of a bond that was unchanged, you can use the speed value provided Done, also changed the general structure a bit. Line 335: boolean chooseMinimum = isBondModeFailover(iface.getBondType()); Line 336: speed = chooseMinimum ? Integer.MAX_VALUE : 0; Line 337: for (VdsNetworkInterface slave : slaves) { Line 338: speed = chooseMinimum ? Math.min(speed, slave.getSpeed()) : speed + slave.getSpeed(); Line 331: return speed; Line 332: } Line 333: Line 334: // not cached, compute and cache for future reference Line 335: boolean chooseMinimum = isBondModeFailover(iface.getBondType()); > 1. As I see the bond type is never used or initialized, so you need to chec 1. Done. 2. http://www.linuxhorizon.ro/bonding.html 3. It's identical to vdsm's calculation, and takes care of all existing bonding modes in Linux. Line 336: speed = chooseMinimum ? Integer.MAX_VALUE : 0; Line 337: for (VdsNetworkInterface slave : slaves) { Line 338: speed = chooseMinimum ? Math.min(speed, slave.getSpeed()) : speed + slave.getSpeed(); Line 339: } http://gerrit.ovirt.org/#/c/34133/10/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java: Line 471: return qosCommitmentSetup("nic0", totalCommitment); Line 472: } Line 473: Line 474: @Test Line 475: public void qosValidCommitment() { > Please add tests for different types of bonds. Done Line 476: SetupNetworksHelper helper = qosCommitmentSetup(DEFAULT_SPEED / 2); Line 477: validateAndExpectNoViolations(helper); Line 478: } Line 479: -- To view, visit http://gerrit.ovirt.org/34133 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icc89f4193d6b7b590c6e1e053af74ab23bc77a11 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[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
