Mike Kolesnik has posted comments on this change.
Change subject: engine: Support interface QoS override in Setup Networks
......................................................................
Patch Set 12:
(5 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java
Line 378: } else if (networkWasModified(iface)) {
Line 379: if
(networkIpAddressWasSameAsHostnameAndChanged(iface)) {
Line 380:
addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED,
networkName);
Line 381: }
Line 382: modifiedNetworks.add(network);
Indentation is a leftover?
Line 383: }
Line 384: } else {
Line 385: VdsNetworkInterface existingIface =
getExistingIfaces().get(iface.getName());
Line 386: existingIface = (existingIface == null ? iface :
existingIface);
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java
Line 209: VDS vds = mock(VDS.class);
Line 210: when(vds.getId()).thenReturn(Guid.Empty);
Line 211:
Line 212: SetupNetworksHelper helper =
createHelper(createParametersForNics(nic), vds);
Line 213:
when(vds.getVdsGroupCompatibilityVersion()).thenReturn(Version.v3_2);
Not sure why you moved this...
Line 214:
Line 215: validateAndExpectViolation(helper,
VdcBllMessages.NETWORK_ATTACH_ILLEGAL_GATEWAY, nic.getNetworkName());
Line 216: }
Line 217:
Line 1490:
Line 1491: private void assertNoInterfacesModified(SetupNetworksHelper
helper) {
Line 1492: assertEquals(MessageFormat.format("Expected no interfaces to
be modified, but the following interfaces were: {0}",
Line 1493: helper.getModifiedInterfaces()),
Line 1494: 0,
Expected value should be first..
Alternatively, you can check isEmpty() with assertTrue
Line 1495: helper.getModifiedInterfaces().size());
Line 1496: }
Line 1497:
Line 1498: /**
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommand.java
Line 58: if (network.isVmNetwork()) {
Line 59: opts.put(VdsProperties.STP, network.getStp() ? "yes" :
"no");
Line 60: }
Line 61:
Line 62: if ((iface.isQosOverridden() ? iface.getQos() != null :
network.getQosId() != null)
Can you simplify this logic? Its very hard to unnderstannd what's going on..
Line 63: &&
FeatureSupported.hostNetworkQos(getDbFacade().getVdsDao()
Line 64: .get(getParameters().getVdsId())
Line 65: .getVdsGroupCompatibilityVersion())) {
Line 66: NetworkQosMapper qosMapper =
....................................................
File
backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java
Line 207: Network network = createNetwork(null);
Line 208: VdsNetworkInterface iface = createNic("eth0", null, null,
network.getName());
Line 209:
Line 210: NetworkQoS qos = createQos();
Line 211: when(qosDao.get(any(Guid.class))).thenReturn(qos);
How do you expect this to be called if QoS is overridden?
Line 212: iface.setQosOverridden(true);
Line 213:
Line 214: qos(network, iface, null, true);
Line 215: }
--
To view, visit http://gerrit.ovirt.org/22766
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I22a4fefac1c5e80c98a72507623152ea4d30ef07
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
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
