Alona Kaplan has uploaded a new change for review.

Change subject: engine: errors in audit log- VDS_SET_NONOPERATIONAL_IFACE_DOWN
......................................................................

engine: errors in audit log- VDS_SET_NONOPERATIONAL_IFACE_DOWN

The format of the log is-
"Host 'host-name' moved to Non-Operational state because interfaces
'iface-list are down but are needed by networks 'net-list' in the current
cluster"
There are some errors in it-
1. Displays vlan device names (should display just base interface names).
2. Doesn't display all the problamatics networks of a bond.
3. Doesn't display problematic bonds if there is another problematic
nic.

Since the bond status can be determined from the bond itself and shouldn't
be calculated from the slaves status and most of the bugs were part of this
logic, this patch removes it.

Change-Id: I9dfa5396c7622b5e689ee3e648fcccd0e75ed834
Bug-Url: https://bugzilla.redhat.com/1079964
Signed-off-by: Alona Kaplan <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
3 files changed, 23 insertions(+), 140 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/20/27720/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java
index 3d91e34..862b4e3 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java
@@ -3,6 +3,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.ovirt.engine.core.bll.interfaces.BackendInternal;
 import org.ovirt.engine.core.common.action.StorageDomainPoolParametersBase;
@@ -84,7 +85,7 @@
                                 nics = 
getDbFacade().getInterfaceDao().getAllInterfacesForVds(vds.getId());
                             }
 
-                            Pair<List<String>, List<String>> problematicNics =
+                            Pair<Set<String>, Set<String>> problematicNics =
                                     
NetworkMonitoringHelper.determineProblematicNics(nics, 
getDbFacade().getNetworkDao()
                                     .getAllForCluster(vds.getVdsGroupId()));
                             if (problematicNics.getFirst().isEmpty()) {
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java
index 40aac69..850ef12 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java
@@ -1,16 +1,16 @@
 package org.ovirt.engine.core.vdsbroker;
 
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 import org.ovirt.engine.core.common.businessentities.network.InterfaceStatus;
 import org.ovirt.engine.core.common.businessentities.network.Network;
 import org.ovirt.engine.core.common.businessentities.network.NetworkStatus;
 import 
org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface;
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.utils.NetworkUtils;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
 
 public class NetworkMonitoringHelper {
     /**
@@ -24,77 +24,21 @@
      * @return A pair of a list of the names of the NICs which are 
problematic, and a list of the names of the networks
      *         which are required by these NICs.
      */
-    public static Pair<List<String>, List<String>> 
determineProblematicNics(List<VdsNetworkInterface> interfaces,
+    public static Pair<Set<String>, Set<String>> 
determineProblematicNics(List<VdsNetworkInterface> interfaces,
             List<Network> clusterNetworks) {
-        Map<String, Boolean> bondsWithStatus = new HashMap<String, Boolean>();
-        List<String> networks = new ArrayList<String>();
-        List<String> brokenNics = new ArrayList<String>();
-        Map<String, List<String>> bondsWithListOfNics = new HashMap<String, 
List<String>>();
+        Set<String> networks = new HashSet<String>();
+        Set<String> brokenNics = new HashSet<String>();
 
         Map<String, Network> networksByName = 
NetworkUtils.networksByName(clusterNetworks);
 
         for (VdsNetworkInterface iface : interfaces) {
-
-            // Handle nics that are non bonded and not vlan over bond
             if (isRequiredInterfaceDown(networksByName, interfaces, iface)) {
-                brokenNics.add(iface.getName());
+                brokenNics.add(NetworkUtils.stripVlan(iface));
                 networks.add(iface.getNetworkName());
             }
-
-            // Handle bond nics
-            if (iface.getBondName() != null) {
-                populate(bondsWithStatus, interfaces, clusterNetworks, 
networks, bondsWithListOfNics, iface);
-            }
         }
 
-        // check the bond statuses, if one is down we mark it as broken
-        // only if we didn't already mark a NIC as broken
-        if (brokenNics.isEmpty()) {
-            for (Map.Entry<String, Boolean> entry : 
bondsWithStatus.entrySet()) {
-                if (!entry.getValue()) {
-                    // add the nics name for audit log
-                    for (String name : 
bondsWithListOfNics.get(entry.getKey())) {
-                        brokenNics.add(name);
-                    }
-                }
-            }
-        }
-
-        return new Pair<List<String>, List<String>>(brokenNics, networks);
-    }
-
-    private static void populate(Map<String, Boolean> bondsWithStatus,
-            List<VdsNetworkInterface> interfaces,
-            List<Network> clusterNetworks,
-            List<String> networks,
-            Map<String, List<String>> bondsWithListOfNics,
-            VdsNetworkInterface iface) {
-        Pair<Boolean, String> retVal =
-                isRequiredNetworkInCluster(iface.getBondName(), interfaces, 
clusterNetworks);
-        String networkName = retVal.getSecond();
-        if (retVal.getFirst()) {
-            if (!bondsWithStatus.containsKey(iface.getBondName())) {
-                bondsWithStatus.put(iface.getBondName(), false);
-            }
-            // It is enough for at least one of the interfaces of the bond to 
be up
-            bondsWithStatus.put(iface.getBondName(),
-                    bondsWithStatus.get(iface.getBondName())
-                            || (iface.getStatistics().getStatus() == 
InterfaceStatus.UP));
-
-            if (!networks.contains(networkName)
-                    && !bondsWithStatus.containsKey(iface.getName())) {
-                networks.add(networkName);
-            }
-            // we remove the network from the audit log if the bond
-            // is active
-            else if (networks.contains(networkName) && 
bondsWithStatus.get(iface.getBondName())) {
-                networks.remove(networkName);
-            }
-            if (!bondsWithListOfNics.containsKey(iface.getBondName())) {
-                bondsWithListOfNics.put(iface.getBondName(), new 
ArrayList<String>());
-            }
-            bondsWithListOfNics.get(iface.getBondName()).add(iface.getName());
-        }
+        return new Pair<Set<String>, Set<String>>(brokenNics, networks);
     }
 
     /**
@@ -106,89 +50,27 @@
     private static boolean isRequiredInterfaceDown(Map<String, Network> 
networksByName,
             List<VdsNetworkInterface> interfaces,
             VdsNetworkInterface iface) {
-        if (iface.getStatistics().getStatus() != InterfaceStatus.UP
-                && iface.getNetworkName() != null
-                && iface.getBonded() == null
-                && !isBondOrVlanOverBond(iface, interfaces)
+        if (iface.getNetworkName() != null &&
+                !isBaseInterfaceUp(iface, interfaces)
                 && networksByName.containsKey(iface.getNetworkName())) {
 
             Network net = networksByName.get(iface.getNetworkName());
-            if (net.getCluster().getStatus() == NetworkStatus.OPERATIONAL && 
net.getCluster().isRequired()
-                    && (iface.getVlanId() == null || !isVlanInterfaceUp(iface, 
interfaces))) {
+            if (net.getCluster().getStatus() == NetworkStatus.OPERATIONAL && 
net.getCluster().isRequired()) {
                 return true;
             }
         }
         return false;
     }
 
-    // method get bond name, list of cluster network - checks if the specified
-    // bonds network is in the clusterNetworks,
-    // if so return true and networkName of the bonds
-    private static Pair<Boolean, String> isRequiredNetworkInCluster(String 
bondName,
-            List<VdsNetworkInterface> interfaces,
-            List<Network> clusterNetworks) {
-        Pair<Boolean, String> retVal = new Pair<Boolean, String>();
-        for (VdsNetworkInterface iface : interfaces) {
-            if (iface.getName().equals(bondName)) {
-                for (Network net : clusterNetworks) {
-                    // If this is the network on the bond, or on a vlan over 
the bond, and the network is required
-                    // we want to check this network
-                    if ((net.getName().equals(iface.getNetworkName())
-                            || isVlanOverBondNetwork(bondName, net.getName(), 
interfaces))
-                            && net.getCluster().isRequired()) {
-                        retVal.setFirst(true);
-                        retVal.setSecond(net.getName());
-                        return retVal;
-                    }
-                }
-                retVal.setFirst(false);
-                return retVal;
-            }
-        }
-        retVal.setFirst(false);
-        return retVal;
-    }
-
-    // IsBond return true if the interface is bond,
-    // it also check if it's vlan over bond and return true in that case
-    // i.e. it return true in case of bond0 and bond0.5
-    private static boolean isBondOrVlanOverBond(VdsNetworkInterface iface, 
List<VdsNetworkInterface> interfaces) {
-        if (iface.getBonded() != null && iface.getBonded() == true) {
-            return true;
+    private static boolean isBaseInterfaceUp(VdsNetworkInterface iface, 
List<VdsNetworkInterface> interfaces) {
+        if (!NetworkUtils.isVlan(iface)) {
+            return iface.getStatistics().getStatus() == InterfaceStatus.UP;
         }
 
-        // check if vlan over bond i.e if we are in bond0.5 we look for bond0
-        return NetworkUtils.isBondVlan(interfaces, iface);
-     }
-
-    // function check if vlan over bond connected to network
-    // i.e. if we have bond0 that have vlan #5 like:
-    // bond0 and bond0.5
-    // bond0 is not connected to network just the bond0.5 is connected to 
network
-    // and this method check for that case
-    private static boolean isVlanOverBondNetwork(String bondName,
-            String networkName,
-            List<VdsNetworkInterface> interfaces) {
-        for (VdsNetworkInterface iface : interfaces) {
-            if (NetworkUtils.isVlan(iface) && 
NetworkUtils.interfaceBasedOn(iface, bondName)
-                    && networkName.equals(iface.getNetworkName())) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    // If vlan we search if the interface is up (i.e. not eth2.5 we look for 
eth2)
-    private static boolean isVlanInterfaceUp(VdsNetworkInterface vlan, 
List<VdsNetworkInterface> interfaces) {
-        if (!NetworkUtils.isVlan(vlan)) {
-            // not vlan
-            return true;
-        }
-
-        String ifaceName = vlan.getBaseInterface();
-        for (VdsNetworkInterface iface : interfaces) {
-            if (iface.getName().equals(ifaceName)) {
-                return iface.getStatistics().getStatus() == InterfaceStatus.UP;
+        String baseIfaceName = iface.getBaseInterface();
+        for (VdsNetworkInterface tmpIface : interfaces) {
+            if (tmpIface.getName().equals(baseIfaceName)) {
+                return tmpIface.getStatistics().getStatus() == 
InterfaceStatus.UP;
             }
         }
 
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
index 5a7f128..573d6ec 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
@@ -746,7 +746,7 @@
 
         try {
             reportNicStatusChanges();
-            Pair<List<String>, List<String>> problematicNics = 
NetworkMonitoringHelper.determineProblematicNics(_vds.getInterfaces(),
+            Pair<Set<String>, Set<String>> problematicNics = 
NetworkMonitoringHelper.determineProblematicNics(_vds.getInterfaces(),
                     
getDbFacade().getNetworkDao().getAllForCluster(_vds.getVdsGroupId()));
             brokenNics.addAll(problematicNics.getFirst());
             networks.addAll(problematicNics.getSecond());


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

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

Reply via email to