Lior Vernia has posted comments on this change.
Change subject: engine: Support interface QoS override in Setup Networks
......................................................................
Patch Set 4:
(4 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksCommand.java
Line 31: import org.ovirt.engine.core.utils.log.LogFactory;
Line 32: import org.ovirt.engine.core.utils.transaction.TransactionMethod;
Line 33: import org.ovirt.engine.core.utils.transaction.TransactionSupport;
Line 34:
Line 35: @NonTransactiveCommandAttribute
Not sure I do, because this QoS entity is related exclusively to the interface
and shouldn't appear in any other query. But I'll verify and see if this should
be fixed.
Line 36: public class SetupNetworksCommand<T extends SetupNetworksParameters>
extends VdsCommand<T> {
Line 37:
Line 38: /** Time between polling attempts, to prevent flooding the
host/network. */
Line 39: private static final long POLLING_BREAK = 500;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java
Line 329:
Line 330: if (existingIface != null &&
existingIface.getNetworkImplementationDetails() != null
Line 331: &&
!existingIface.getNetworkImplementationDetails().isInSync()) {
Line 332: if (networkShouldBeSynced(networkName)) {
Line 333: modifiedNetworks.add(network);
Yes, you are correct. I was under the wrong impression that we can't edit a
network when it's out of sync (probably got confused with unmanaged). Fixed.
Line 334: } else if (networkWasModified(iface)) {
Line 335:
addViolation(VdcBllMessages.NETWORKS_NOT_IN_SYNC, networkName);
Line 336: }
Line 337: } else {
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java
Line 1684: return createHelper(params, vds);
Line 1685: }
Line 1686:
Line 1687: private SetupNetworksHelper createHelper(SetupNetworksParameters
params, VDS vds) {
Line 1688:
when(vds.getVdsGroupCompatibilityVersion()).thenReturn(Version.v3_3);
Well, once I add the QoS validation to SetupNetworksHelper then in order to
run, all of the tests need to be be able to report a cluster compatibility
version when they run.
I think it always has to be mocked, somewhere. And the other overload of this
method already does some mocking on the VDS entity, so... Just tell me how you
want it to be done and I'll change it.
Line 1689:
Line 1690: SetupNetworksHelper helper = spy(new
SetupNetworksHelper(params, vds));
Line 1691:
Line 1692:
when(helper.getVmInterfaceManager()).thenReturn(vmInterfaceManager);
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java
Line 171: public static boolean isNetworkInSync(VdsNetworkInterface iface,
Network network, NetworkQoS qos) {
Line 172: return (network.getMtu() == 0 || iface.getMtu() ==
network.getMtu())
Line 173: && Objects.equals(iface.getVlanId(),
network.getVlanId())
Line 174: && iface.isBridged() == network.isVmNetwork()
Line 175: && (Objects.equals(iface.getQos(), qos) ||
iface.isQosOverridden());
As I stated, I think it's perfectly okay; if I'm not mistaken, so far the term
of "in sync" was only used when comparing with the DC-level network entity, not
when comparing NIC configuration on the host and NIC configuration in the DB.
The semantics are ours to define.
Line 176: }
Line 177:
Line 178: /**
Line 179: * Returns true if a given network is non-VM network with no Vlan
tagging, else false.
--
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: 4
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
