Mike Kolesnik has uploaded a new change for review.

Change subject: engine: Block non-exclusive network configurations (#851134)
......................................................................

engine: Block non-exclusive network configurations (#851134)

https://bugzilla.redhat.com/851134

As explained in previous commit:
On a Host's NIC only one of two configurations is possible:
  1. A single VM network.
  2. An optional single non-VM network + Any number of VLAN networks.

Blocked configurations that don't match this constraint from
Setup Networks.

Change-Id: I2712b6e6948de0881c56d519bbf82f7ecc7f6157
Signed-off-by: Mike Kolesnik <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetupNetworksHelper.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SetupNetworksHelperTest.java
2 files changed, 126 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/64/7664/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetupNetworksHelper.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetupNetworksHelper.java
index fb67d7c..da11164 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetupNetworksHelper.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetupNetworksHelper.java
@@ -43,6 +43,8 @@
     /** All network`s names that are attached to some sort of interface. */
     private Set<String> attachedNetworksNames = new HashSet<String>();
 
+    private Map<String, NetworkType> ifacesWithExclusiveNetwork = new 
HashMap<String, NetworkType>();
+
     public SetupNetworksHelper(SetupNetworksParameters parameters, Guid 
vdsGroupId) {
         params = parameters;
         this.vdsGroupId = vdsGroupId;
@@ -218,6 +220,10 @@
 
             // check if network exists on cluster
             if (getExistingClusterNetworks().containsKey(networkName)) {
+                Network network = 
getExistingClusterNetworks().get(networkName);
+                validateNetworkExclusiveOnIface(iface,
+                        determineNetworkType(network.getvlan_id(), 
network.isVmNetwork()));
+
                 VdsNetworkInterface existingIface = 
getExistingIfaces().get(iface.getName());
                 if (existingIface != null && 
!networkName.equals(existingIface.getNetworkName())) {
                     existingIface = getExistingIfaceByNetwork(networkName);
@@ -226,16 +232,50 @@
                 if (existingIface != null && 
existingIface.getNetworkImplementationDetails() != null
                         && 
!existingIface.getNetworkImplementationDetails().isInSync()) {
                     if (networkShouldBeSynced(networkName)) {
-                        
modifiedNetworks.add(getExistingClusterNetworks().get(networkName));
+                        modifiedNetworks.add(network);
                     } else if (networkWasModified(iface)) {
                         addViolation(VdcBllMessages.NETWORKS_NOT_IN_SYNC, 
networkName);
                     }
                 } else if (networkWasModified(iface)) {
-                    
modifiedNetworks.add(getExistingClusterNetworks().get(networkName));
+                    modifiedNetworks.add(network);
                 }
-            } else if (unmanagedNetworkChanged(iface)) {
-                addViolation(VdcBllMessages.NETWORKS_DONT_EXIST_IN_CLUSTER, 
networkName);
+            } else {
+                VdsNetworkInterface existingIface = 
getExistingIfaces().get(iface.getName());
+                existingIface = (existingIface == null ? iface : 
existingIface);
+                validateNetworkExclusiveOnIface(iface,
+                        determineNetworkType(existingIface.getVlanId(), 
existingIface.isBridged()));
+
+                if (unmanagedNetworkChanged(iface)) {
+                    
addViolation(VdcBllMessages.NETWORKS_DONT_EXIST_IN_CLUSTER, networkName);
+                }
             }
+        }
+    }
+
+    private NetworkType determineNetworkType(Integer vlanId, boolean 
vmNetwork) {
+        return vlanId != null ? NetworkType.VLAN : vmNetwork ? NetworkType.VM 
: NetworkType.NON_VM;
+    }
+
+    /**
+     * Make sure that the given interface has only a single VM network on it, 
or only at most one non-VM network and/or
+     * one or more logically different (i.e. VLAN) networks on it.
+     *
+     * @param iface
+     *            The interface to check.
+     * @param networkType
+     *            The type of the network.
+     */
+    private void validateNetworkExclusiveOnIface(VdsNetworkInterface iface, 
NetworkType networkType) {
+        String ifaceName = NetworkUtils.StripVlan(iface.getName());
+        if (ifacesWithExclusiveNetwork.containsKey(ifaceName)) {
+            if ((networkType == NetworkType.VLAN && 
ifacesWithExclusiveNetwork.get(ifaceName) == NetworkType.VM)
+                    || networkType != NetworkType.VLAN) {
+                
addViolation(VdcBllMessages.NETWORK_INTERFACES_NOT_EXCLUSIVELY_USED_BY_NETWORK, 
ifaceName);
+            }
+        }
+
+        if (networkType != NetworkType.VLAN) {
+            ifacesWithExclusiveNetwork.put(ifaceName, networkType);
         }
     }
 
@@ -426,4 +466,10 @@
     public VmInterfaceManager getVmInterfaceManager() {
         return new VmInterfaceManager();
     }
+
+    private enum NetworkType {
+        VM,
+        NON_VM,
+        VLAN
+    }
 }
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SetupNetworksHelperTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SetupNetworksHelperTest.java
index 66e9ce8..7f47e6d 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SetupNetworksHelperTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/SetupNetworksHelperTest.java
@@ -362,6 +362,82 @@
         validateAndAssertNetworkModified(helper, net);
     }
 
+    @Test
+    public void vlanNetworkWithVmNetworkDenied() {
+        Network net1 = createNetwork("net1");
+        Network net2 = createNetwork("net2");
+        net2.setvlan_id(100);
+        mockExistingNetworks(net1, net2);
+
+        VdsNetworkInterface nic = createNic("nic0", null);
+        VdsNetworkInterface vlanNic = createVlan(nic.getName(), 
net2.getvlan_id(), net2.getName());
+        mockExistingIfaces(nic, vlanNic);
+
+        nic.setNetworkName(net1.getName());
+
+        SetupNetworksHelper helper = createHelper(createParametersForNics(nic, 
vlanNic));
+
+        validateAndExpectViolation(helper,
+                
VdcBllMessages.NETWORK_INTERFACES_NOT_EXCLUSIVELY_USED_BY_NETWORK,
+                nic.getName());
+    }
+
+    @Test
+    public void unmanagedVlanNetworkWithVmNetworkDenied() {
+        Network net1 = createNetwork("net1");
+        Network net2 = createNetwork("net2");
+        net2.setvlan_id(100);
+        mockExistingNetworks(net1);
+
+        VdsNetworkInterface nic = createNicSyncedWithNetwork("nic0", net1);
+        mockExistingIfaces(nic);
+
+        VdsNetworkInterface vlanNic = createVlan(nic.getName(), 
net2.getvlan_id(), net2.getName());
+
+        SetupNetworksHelper helper = createHelper(createParametersForNics(nic, 
vlanNic));
+
+        validateAndExpectViolation(helper,
+                
VdcBllMessages.NETWORK_INTERFACES_NOT_EXCLUSIVELY_USED_BY_NETWORK,
+                nic.getName());
+    }
+
+    @Test
+    public void fakeVlanNicWithVmNetworkDenied() {
+        Network net1 = createNetwork("net1");
+        Network net2 = createNetwork("net2");
+        mockExistingNetworks(net1, net2);
+
+        VdsNetworkInterface nic = createNicSyncedWithNetwork("nic0", net1);
+        mockExistingIfaces(nic);
+
+        VdsNetworkInterface fakeVlanNic = createVlan(nic.getName(), 100, 
net2.getName());
+        fakeVlanNic.setVlanId(null);
+
+        SetupNetworksHelper helper = createHelper(createParametersForNics(nic, 
fakeVlanNic));
+
+        validateAndExpectViolation(helper,
+                
VdcBllMessages.NETWORK_INTERFACES_NOT_EXCLUSIVELY_USED_BY_NETWORK,
+                nic.getName());
+    }
+
+    @Test
+    public void twoVlanVmNetworks() {
+        Network net1 = createNetwork("net1");
+        net1.setvlan_id(100);
+        Network net2 = createNetwork("net2");
+        net2.setvlan_id(200);
+        VdsNetworkInterface nic = createNic("nic0", null);
+        VdsNetworkInterface vlan1 = createVlan(nic.getName(), 
net1.getvlan_id(), net1.getName());
+        VdsNetworkInterface vlan2 = createVlan(nic.getName(), 
net2.getvlan_id(), net2.getName());
+
+        mockExistingNetworks(net1, net2);
+        mockExistingIfaces(nic);
+
+        SetupNetworksHelper helper = createHelper(createParametersForNics(nic, 
vlan1, vlan2));
+
+        validateAndExpectNoViolations(helper);
+    }
+
     /* --- Tests for bonds functionality --- */
 
     @Test


--
To view, visit http://gerrit.ovirt.org/7664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2712b6e6948de0881c56d519bbf82f7ecc7f6157
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to