cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From h...@apache.org
Subject [2/2] git commit: updated refs/heads/master to b134854
Date Mon, 15 Jul 2013 07:14:24 GMT
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 <htrippaers@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 <toshiaki.hatano@verio.net>
Authored: Fri Jul 12 03:49:35 2013 +0000
Committer: Hugo Trippaers <htrippaers@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"));
     }
 


Mime
View raw message