You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ml...@apache.org on 2013/01/19 01:29:24 UTC
git commit: Summary: Fix bridge parsing when bridge names are subsets
of others
Updated Branches:
refs/heads/4.0 854961821 -> 23d555397
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 <ma...@betterservers.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/23d55539
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/23d55539
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/23d55539
Branch: refs/heads/4.0
Commit: 23d55539763645d69fc8b35e393816108256d670
Parents: 8549618
Author: Marcus Sorensen <ma...@betterservers.com>
Authored: Fri Jan 18 17:25:20 2013 -0700
Committer: Marcus Sorensen <ma...@betterservers.com>
Committed: Fri Jan 18 17:25:20 2013 -0700
----------------------------------------------------------------------
.../hypervisor/kvm/resource/BridgeVifDriver.java | 13 +--
.../kvm/resource/LibvirtComputingResource.java | 65 ++++++++++-----
2 files changed, 48 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/23d55539/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 cf4de09..f406c86 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 {
@@ -183,15 +184,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/23d55539/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 a19022c..b8cce1e 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
@@ -782,8 +782,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements
String privPif = null;
String vlan = null;
if (_publicBridgeName != null) {
- pubPif = Script.runSimpleBashScript("brctl show | grep "
- + _publicBridgeName + " | awk '{print $4}'");
+ pubPif = matchPifFileInDirectory(_publicBridgeName);
vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + pubPif);
if (vlan != null && !vlan.isEmpty()) {
pubPif = Script
@@ -792,10 +791,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements
}
}
if (_guestBridgeName != null) {
- privPif = Script.runSimpleBashScript("brctl show | grep "
- + _guestBridgeName + " | awk '{print $4}'");
+ privPif = matchPifFileInDirectory(_guestBridgeName);
vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + privPif);
- if (vlan != null && !vlan.isEmpty()) {
+ File vlanfile = new File("/proc/net/vlan"+privPif);
+ if (vlanfile.isFile()) {
privPif = Script
.runSimpleBashScript("grep ^Device\\: /proc/net/vlan/"
+ privPif + " | awk {'print $2'}");
@@ -805,14 +804,41 @@ public class LibvirtComputingResource extends ServerResourceBase implements
_pifs.put("public", pubPif);
}
+ 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 = matchPifFileInDirectory(networkName);
- String name = Script.runSimpleBashScript("brctl show | grep "
- + networkName + " | awk '{print $4}'");
- if (name == null) {
+ if (name == null || name.isEmpty()) {
return false;
} else {
return true;
@@ -1226,20 +1252,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 "";
}
}
@@ -1478,7 +1499,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements
for (IpAddressTO ip : ips) {
String ipVlan = ip.getVlanId();
- String nicName = "eth" + vlanToNicNum.get(ip.getVlanId());
+ String nicName = "eth" + vlanToNicNum.get(ipVlan);
String netmask = Long.toString(NetUtils.getCidrSize(ip.getVlanNetmask()));
String subnet = NetUtils.getSubNet(ip.getPublicIp(), ip.getVlanNetmask());
_virtRouterResource.assignVpcIpToRouter(routerIP, ip.isAdd(), ip.getPublicIp(),