You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by hu...@apache.org on 2013/07/15 09:14:24 UTC
[2/2] git commit: updated refs/heads/master to b134854
CLOUDSTACK-3431: KVM: cloudstack-plugin-hypervisor-kvm with BridgeVifDriver doesn't cleanup vNet due to multiple reasons
- Move vnetBridge clean up function from LibvirtComputingResource to BridgeVifDriver
-- since only BridgeVifDriver have to handle this event
- LibvirtComputingResource now properly call VifDriver.unplug() when it receives UnPlugCommand
- Remove not working and no longer used method getVnet(String) from VirtualMachineName
- Remove not working and no longer used method getVnet() from StopCommand
- Remove unused constructer StopCommand(VirtualMachine, String, boolean) from StopCommand
- Remove unused member vnet from StopCommand
- Remove unused member _modifyVlanPath from OvsVifDriver
Tested with 2 KVM hosts and confirmed it correctly manipulate vnetBridge with start, stop, migrate, plug, and unplug event
Signed-off-by: Hugo Trippaers <ht...@schubergphilis.com>
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/b1348549
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/b1348549
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/b1348549
Branch: refs/heads/master
Commit: b13485493733585cce3044d209063bf37a774ece
Parents: b81f824
Author: Toshiaki Hatano <to...@verio.net>
Authored: Fri Jul 12 03:49:35 2013 +0000
Committer: Hugo Trippaers <ht...@schubergphilis.com>
Committed: Mon Jul 15 09:12:52 2013 +0200
----------------------------------------------------------------------
api/src/com/cloud/vm/VirtualMachineName.java | 4 --
core/src/com/cloud/agent/api/StopCommand.java | 11 ---
.../kvm/resource/BridgeVifDriver.java | 74 +++++++++++++++++---
.../kvm/resource/LibvirtComputingResource.java | 68 ++----------------
.../hypervisor/kvm/resource/OvsVifDriver.java | 6 --
5 files changed, 70 insertions(+), 93 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b1348549/api/src/com/cloud/vm/VirtualMachineName.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/VirtualMachineName.java b/api/src/com/cloud/vm/VirtualMachineName.java
index 81838b9..1279fd6 100755
--- a/api/src/com/cloud/vm/VirtualMachineName.java
+++ b/api/src/com/cloud/vm/VirtualMachineName.java
@@ -93,10 +93,6 @@ public class VirtualMachineName {
return Long.parseLong(vmName.substring(begin + 1, end));
}
- public static String getVnet(String vmName) {
- return vmName.substring(vmName.lastIndexOf(SEPARATOR) + SEPARATOR.length());
- }
-
public static String getRouterName(long routerId, String instance) {
StringBuilder builder = new StringBuilder("r");
builder.append(SEPARATOR).append(routerId).append(SEPARATOR).append(instance);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b1348549/core/src/com/cloud/agent/api/StopCommand.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/StopCommand.java b/core/src/com/cloud/agent/api/StopCommand.java
index a3ee3c9..65a6563 100755
--- a/core/src/com/cloud/agent/api/StopCommand.java
+++ b/core/src/com/cloud/agent/api/StopCommand.java
@@ -19,7 +19,6 @@ package com.cloud.agent.api;
import com.cloud.vm.VirtualMachine;
public class StopCommand extends RebootCommand {
- String vnet;
private boolean isProxy=false;
private String urlPort=null;
private String publicConsoleProxyIpAddress=null;
@@ -35,12 +34,6 @@ public class StopCommand extends RebootCommand {
this.publicConsoleProxyIpAddress = publicConsoleProxyIpAddress;
this.executeInSequence = executeInSequence;
}
-
- public StopCommand(VirtualMachine vm, String vnet, boolean executeInSequence) {
- super(vm);
- this.vnet = vnet;
- this.executeInSequence = executeInSequence;
- }
public StopCommand(VirtualMachine vm, boolean executeInSequence) {
super(vm);
@@ -52,10 +45,6 @@ public class StopCommand extends RebootCommand {
this.executeInSequence = executeInSequence;
}
- public String getVnet() {
- return vnet;
- }
-
@Override
public boolean executeInSequence() {
return executeInSequence;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b1348549/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 0db83cc..195cf40 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,8 @@ import org.libvirt.LibvirtException;
import javax.naming.ConfigurationException;
import java.net.URI;
import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.io.File;
public class BridgeVifDriver extends VifDriverBase {
@@ -40,6 +42,8 @@ public class BridgeVifDriver extends VifDriverBase {
private static final Logger s_logger = Logger
.getLogger(BridgeVifDriver.class);
private int _timeout;
+
+ private static final Object _vnetBridgeMonitor = new Object();
private String _modifyVlanPath;
@Override
@@ -136,7 +140,7 @@ public class BridgeVifDriver extends VifDriverBase {
@Override
public void unplug(LibvirtVMDef.InterfaceDef iface) {
- // Nothing needed as libvirt cleans up tap interface from bridge.
+ deleteVnetBr(iface.getBrName());
}
private String setVnetBrName(String pifName, String vnetId) {
@@ -161,16 +165,64 @@ public class BridgeVifDriver extends VifDriverBase {
private void createVnet(String vnetId, String pif, String brName)
throws InternalErrorException {
- final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
- command.add("-v", vnetId);
- command.add("-p", pif);
- command.add("-b", brName);
- command.add("-o", "add");
-
- final String result = command.execute();
- if (result != null) {
- throw new InternalErrorException("Failed to create vnet " + vnetId
- + ": " + result);
+ synchronized (_vnetBridgeMonitor) {
+ final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
+ command.add("-v", vnetId);
+ command.add("-p", pif);
+ command.add("-b", brName);
+ command.add("-o", "add");
+
+ final String result = command.execute();
+ if (result != null) {
+ throw new InternalErrorException("Failed to create vnet " + vnetId
+ + ": " + result);
+ }
+ }
+ }
+
+ private void deleteVnetBr(String brName){
+ synchronized (_vnetBridgeMonitor) {
+ String cmdout = Script.runSimpleBashScript("ls /sys/class/net/" + brName + "/brif | grep vnet");
+ if (cmdout != null && cmdout.contains("vnet")) {
+ // Active VM remains on that bridge
+ return;
+ }
+
+ Pattern oldStyleBrNameRegex = Pattern.compile("^cloudVirBr(\\d+)$");
+ Pattern brNameRegex = Pattern.compile("^br(\\S+)-(\\d+)$");
+ Matcher oldStyleBrNameMatcher = oldStyleBrNameRegex.matcher(brName);
+ Matcher brNameMatcher = brNameRegex.matcher(brName);
+
+ String pName = null;
+ String vNetId = null;
+ if (oldStyleBrNameMatcher.find()) {
+ // Actually modifyvlan.sh doesn't require pif name when deleting its bridge so far.
+ pName = "undefined";
+ vNetId = oldStyleBrNameMatcher.group(1);
+ } else if (brNameMatcher.find()) {
+ if(brNameMatcher.group(1) != null || !brNameMatcher.group(1).isEmpty()) {
+ pName = brNameMatcher.group(1);
+ } else {
+ pName = "undefined";
+ }
+ vNetId = brNameMatcher.group(2);
+ }
+
+ if (vNetId == null || vNetId.isEmpty()) {
+ s_logger.debug("unable to get a vNet ID from name "+ brName);
+ return;
+ }
+
+ final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
+ command.add("-o", "delete");
+ command.add("-v", vNetId);
+ command.add("-p", pName);
+ command.add("-b", brName);
+
+ final String result = command.execute();
+ if (result != null) {
+ s_logger.debug("Delete bridge " + brName + " failed: " + result);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b1348549/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 24f9ee0..a2e4044 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
@@ -1065,10 +1065,6 @@ ServerResource {
}
}
- private String getVnetId(String vnetId) {
- return vnetId;
- }
-
private void passCmdLine(String vmName, String cmdLine)
throws InternalErrorException {
final Script command = new Script(_patchViaSocketPath, _timeout, s_logger);
@@ -1694,6 +1690,11 @@ ServerResource {
for (InterfaceDef pluggedNic : pluggedNics) {
if (pluggedNic.getMacAddress().equalsIgnoreCase(nic.getMac())) {
vm.detachDevice(pluggedNic.toString());
+ // We don't know which "traffic type" is associated with
+ // each interface at this point, so inform all vif drivers
+ for(VifDriver vifDriver : getAllVifDrivers()){
+ vifDriver.unplug(pluggedNic);
+ }
return new UnPlugNicAnswer(cmd, true, "success");
}
}
@@ -2726,8 +2727,6 @@ ServerResource {
vifDriver.unplug(iface);
}
}
- cleanupVM(conn, vmName,
- getVnetId(VirtualMachineName.getVnet(vmName)));
}
return new MigrateAnswer(cmd, result == null, result, null);
@@ -3043,11 +3042,6 @@ ServerResource {
}
}
- final String result2 = cleanupVnet(conn, cmd.getVnet());
-
- if (result != null && result2 != null) {
- result = result2 + result;
- }
state = State.Stopped;
return new StopAnswer(cmd, result, 0, true);
} catch (LibvirtException e) {
@@ -4123,16 +4117,6 @@ ServerResource {
return info;
}
- protected void cleanupVM(Connect conn, final String vmName,
- final String vnet) {
- s_logger.debug("Trying to cleanup the vnet: " + vnet);
- if (vnet != null) {
- cleanupVnet(conn, vnet);
- }
-
- _vmStats.remove(vmName);
- }
-
protected String rebootVM(Connect conn, String vmName) {
Domain dm = null;
String msg = null;
@@ -4274,29 +4258,6 @@ ServerResource {
return null;
}
- public synchronized String cleanupVnet(Connect conn, final String vnetId) {
- // VNC proxy VMs do not have vnet
- if (vnetId == null || vnetId.isEmpty()
- || isDirectAttachedNetwork(vnetId)) {
- return null;
- }
-
- final List<String> names = getAllVmNames(conn);
-
- if (!names.isEmpty()) {
- for (final String name : names) {
- if (VirtualMachineName.getVnet(name).equals(vnetId)) {
- return null; // Can't remove the vnet yet.
- }
- }
- }
-
- final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
- command.add("-o", "delete");
- command.add("-v", vnetId);
- return command.execute();
- }
-
protected Integer getVncPort(Connect conn, String vmName)
throws LibvirtException {
LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
@@ -4410,26 +4371,11 @@ ServerResource {
}
}
- private String getVnetIdFromBrName(String vnetBrName) {
- if (vnetBrName.contains("cloudVirBr")) {
- return vnetBrName.replaceAll("cloudVirBr", "");
- } else {
- Pattern r = Pattern.compile("-(\\d+)$");
- Matcher m = r.matcher(vnetBrName);
- if(m.group(1) != null || !m.group(1).isEmpty()) {
- return m.group(1);
- } else {
- s_logger.debug("unable to get a vlan ID from name " + vnetBrName);
- return "";
- }
- }
- }
-
private void cleanupVMNetworks(Connect conn, List<InterfaceDef> nics) {
if (nics != null) {
for (InterfaceDef nic : nics) {
- if (nic.getHostNetType() == hostNicType.VNET) {
- cleanupVnet(conn, getVnetIdFromBrName(nic.getBrName()));
+ for(VifDriver vifDriver : getAllVifDrivers()){
+ vifDriver.unplug(nic);
}
}
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b1348549/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
index 951badd..7038d7e 100644
--- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
@@ -38,7 +38,6 @@ import com.cloud.utils.script.Script;
public class OvsVifDriver extends VifDriverBase {
private static final Logger s_logger = Logger.getLogger(OvsVifDriver.class);
private int _timeout;
- private String _modifyVlanPath;
@Override
public void configure(Map<String, Object> params) throws ConfigurationException {
@@ -52,11 +51,6 @@ public class OvsVifDriver extends VifDriverBase {
String value = (String) params.get("scripts.timeout");
_timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
- _modifyVlanPath = Script.findScript(networkScriptsDir, "modifyvlan.sh");
- if (_modifyVlanPath == null) {
- throw new ConfigurationException("Unable to find modifyvlan.sh");
- }
-
createControlNetwork(_bridges.get("linklocal"));
}