Summary: Fix bridge parsing when bridge names are subsets of others

Detail: There are several places in the code that do a
"brctl show | grep bridgeName" or similar, which causes all sorts
of problems when you have for example a cloudVirBr50 and a
cloudVirBr5000. This patch attempts to stop relying on the output
of brctl, instead favoring sysfs and /sys/devices/virtual/net.
It cuts a lot of bash out altogether by using java File. It was
tested in my devcloud-kvm against current 4.0, as well as by the
customer reporting initial bug.

BUG-ID: CLOUDSTACK-938
Fix-For: 4.0.1
Signed-off-by: Marcus Sorensen <marcus@betterservers.com>

plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java

@@ -33,6 +33,7 @@ import org.libvirt.LibvirtException;
 import javax.naming.ConfigurationException;
 import java.net.URI;
 import java.util.Map;
+import java.io.File;
 
 public class BridgeVifDriver extends VifDriverBase {
 
@@ -206,15 +207,11 @@ public class BridgeVifDriver extends VifDriverBase {
     }
 
     private boolean isBridgeExists(String bridgeName) {
-        Script command = new Script("/bin/sh", _timeout);
+        File f = new File("/sys/devices/virtual/net/" + bridgeName);
-        command.add("-c");
+        if (f.exists()) {
-        command.add("brctl show|grep " + bridgeName);
-        final OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
-        String result = command.execute(parser);
-        if (result != null || parser.getLine() == null) {
-            return false;
-        } else {
             return true;
+        } else {
+            return false;
         }
     }
 }

plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

@@ -788,10 +788,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements
     }
 
     private void getPifs() {
-        /* gather all available bridges and find their pifs, so that we can match them against traffic labels later */
+        File dir = new File("/sys/devices/virtual/net");
-        String cmdout = Script.runSimpleBashScript("brctl show | tail -n +2 | grep -v \"^\\s\"|awk '{print $1}'|sed '{:q;N;s/\\n/%/g;t q}'");
+        File[] netdevs = dir.listFiles();
-        s_logger.debug("cmdout was " + cmdout);
+        List<String> bridges = new ArrayList<String>();
-        List<String> bridges = Arrays.asList(cmdout.split("%"));
+        for (int i = 0; i < netdevs.length; i++) {
+            File isbridge = new File(netdevs[i].getAbsolutePath() + "/bridge");
+            String netdevName = netdevs[i].getName();
+            s_logger.debug("looking in file " + netdevs[i].getAbsolutePath() + "/bridge");
+            if (isbridge.exists()) {
+                s_logger.debug("Found bridge " + netdevName);
+                bridges.add(netdevName);
+            }
+        }
+
         for (String bridge : bridges) {
             s_logger.debug("looking for pif for bridge " + bridge);
             String pif = getPif(bridge);
@@ -807,24 +816,53 @@ public class LibvirtComputingResource extends ServerResourceBase implements
     }
 
     private String getPif(String bridge) {
-        String pif = Script.runSimpleBashScript("brctl show | grep " + bridge + " | awk '{print $4}'");
+        String pif = matchPifFileInDirectory(bridge);
-        String vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + pif);
+        File vlanfile = new File("/proc/net/vlan" + pif);
 
-        if (vlan != null && !vlan.isEmpty()) {
+        if (vlanfile.isFile()) {
-                pif = Script.runSimpleBashScript("grep ^Device\\: /proc/net/vlan/" + pif + " | awk {'print $2'}");
+                pif = Script.runSimpleBashScript("grep ^Device\\: /proc/net/vlan/"
+                                                  + pif + " | awk {'print $2'}");
         }
 
         return pif;
     }
 
+    private String matchPifFileInDirectory(String bridgeName){
+        File f = new File("/sys/devices/virtual/net/" + bridgeName + "/brif");
+
+        if (! f.isDirectory()){
+            s_logger.debug("failing to get physical interface from bridge"
+                           + bridgeName + ", does " + f.getAbsolutePath() 
+                           + "exist?");
+            return "";
+        }
+
+        File[] interfaces = f.listFiles();
+
+        for (int i = 0; i < interfaces.length; i++) {
+            String fname = interfaces[i].getName();
+            s_logger.debug("matchPifFileInDirectory: file name '"+fname+"'");
+            if (fname.startsWith("eth") || fname.startsWith("bond")
+                || fname.startsWith("vlan")) {
+                return fname;
+            }
+        }
+
+        s_logger.debug("failing to get physical interface from bridge"
+                        + bridgeName + ", did not find an eth*, bond*, or vlan* in "
+                        + f.getAbsolutePath());
+        return "";
+    }
+
+
     private boolean checkNetwork(String networkName) {
         if (networkName == null) {
             return true;
         }
 
-        String name = Script.runSimpleBashScript("brctl show | grep "
+        String name = matchPifFileInDirectory(networkName);
-                + networkName + " | awk '{print $4}'");
+
-        if (name == null) {
+        if (name == null || name.isEmpty()) {
             return false;
         } else {
             return true;
@@ -1381,20 +1419,15 @@ public class LibvirtComputingResource extends ServerResourceBase implements
     }
 
     private String getVlanIdFromBridge(String brName) {
-        OutputInterpreter.OneLineParser vlanIdParser = new OutputInterpreter.OneLineParser();
+        String pif= matchPifFileInDirectory(brName);
-        final Script cmd = new Script("/bin/bash", s_logger);
+        String[] pifparts = pif.split("\\.");
-        cmd.add("-c");
+
-        cmd.add("vlanid=$(brctl show |grep " + brName
+        if(pifparts.length == 2) {
-                + " |awk '{print $4}' | cut -s -d. -f 2);echo $vlanid");
+            return pifparts[1];
-        String result = cmd.execute(vlanIdParser);
-        if (result != null) {
-            return null;
-        }
-        String vlanId = vlanIdParser.getLine();
-        if (vlanId.equalsIgnoreCase("")) {
-            return null;
         } else {
-            return vlanId;
+            s_logger.debug("failed to get vlan id from bridge " + brName 
+                           + "attached to physical interface" + pif);
+            return "";
         }
     }
 
@@ -1632,7 +1665,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements
             }
 
             for (IpAddressTO ip : ips) {
-                String ipVlan = ip.getVlanId();
                 String nicName = "eth" + vlanToNicNum.get(ip.getVlanId());
                 String netmask = Long.toString(NetUtils.getCidrSize(ip.getVlanNetmask()));
                 String subnet = NetUtils.getSubNet(ip.getPublicIp(), ip.getVlanNetmask());