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>
This commit is contained in:
Toshiaki Hatano 2013-07-12 03:49:35 +00:00 committed by Hugo Trippaers
parent b81f824140
commit b134854937
5 changed files with 70 additions and 93 deletions

View File

@ -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);

View File

@ -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;

View File

@ -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);
}
}
}

View File

@ -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);
}
}
}

View File

@ -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"));
}