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 <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/a65b584d Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/a65b584d Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/a65b584d Branch: refs/heads/cloud-agent-with-openvswitch Commit: a65b584d188002caaf9c60914c02fba8ef1bda7c Parents: 98a2890 Author: Marcus Sorensen <[email protected]> Authored: Fri Jan 18 18:28:08 2013 -0700 Committer: Marcus Sorensen <[email protected]> Committed: Fri Jan 18 18:28:08 2013 -0700 ---------------------------------------------------------------------- .../hypervisor/kvm/resource/BridgeVifDriver.java | 13 +-- .../kvm/resource/LibvirtComputingResource.java | 82 ++++++++++----- 2 files changed, 62 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/a65b584d/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index e6f2f7f..51849f1 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/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); - command.add("-c"); - 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 { + File f = new File("/sys/devices/virtual/net/" + bridgeName); + if (f.exists()) { return true; + } else { + return false; } } } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/a65b584d/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index eac60f4..a269e8a 100755 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/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 */ - String cmdout = Script.runSimpleBashScript("brctl show | tail -n +2 | grep -v \"^\\s\"|awk '{print $1}'|sed '{:q;N;s/\\n/%/g;t q}'"); - s_logger.debug("cmdout was " + cmdout); - List<String> bridges = Arrays.asList(cmdout.split("%")); + File dir = new File("/sys/devices/virtual/net"); + File[] netdevs = dir.listFiles(); + List<String> bridges = new ArrayList<String>(); + 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 vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + pif); + String pif = matchPifFileInDirectory(bridge); + File vlanfile = new File("/proc/net/vlan" + pif); - if (vlan != null && !vlan.isEmpty()) { - pif = Script.runSimpleBashScript("grep ^Device\\: /proc/net/vlan/" + pif + " | awk {'print $2'}"); + if (vlanfile.isFile()) { + 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 " - + networkName + " | awk '{print $4}'"); - if (name == null) { + String name = matchPifFileInDirectory(networkName); + + 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(); - final Script cmd = new Script("/bin/bash", s_logger); - cmd.add("-c"); - cmd.add("vlanid=$(brctl show |grep " + brName - + " |awk '{print $4}' | cut -s -d. -f 2);echo $vlanid"); - String result = cmd.execute(vlanIdParser); - if (result != null) { - return null; - } - String vlanId = vlanIdParser.getLine(); - if (vlanId.equalsIgnoreCase("")) { - return null; + String pif= matchPifFileInDirectory(brName); + String[] pifparts = pif.split("\\."); + + if(pifparts.length == 2) { + return pifparts[1]; } 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());
