Skip to content

Commit b134854

Browse files
Toshiaki Hatanospark404
Toshiaki Hatano
authored and
spark404
committed
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>
1 parent b81f824 commit b134854

5 files changed

Lines changed: 70 additions & 93 deletions

File tree

‎api/src/com/cloud/vm/VirtualMachineName.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,6 @@ public static long getSystemVmId(String vmName) {
9393
return Long.parseLong(vmName.substring(begin + 1, end));
9494
}
9595

96-
public static String getVnet(String vmName) {
97-
return vmName.substring(vmName.lastIndexOf(SEPARATOR) + SEPARATOR.length());
98-
}
99-
10096
public static String getRouterName(long routerId, String instance) {
10197
StringBuilder builder = new StringBuilder("r");
10298
builder.append(SEPARATOR).append(routerId).append(SEPARATOR).append(instance);

‎core/src/com/cloud/agent/api/StopCommand.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.cloud.vm.VirtualMachine;
2020

2121
public class StopCommand extends RebootCommand {
22-
String vnet;
2322
private boolean isProxy=false;
2423
private String urlPort=null;
2524
private String publicConsoleProxyIpAddress=null;
@@ -35,12 +34,6 @@ public StopCommand(VirtualMachine vm, boolean isProxy, String urlPort, String pu
3534
this.publicConsoleProxyIpAddress = publicConsoleProxyIpAddress;
3635
this.executeInSequence = executeInSequence;
3736
}
38-
39-
public StopCommand(VirtualMachine vm, String vnet, boolean executeInSequence) {
40-
super(vm);
41-
this.vnet = vnet;
42-
this.executeInSequence = executeInSequence;
43-
}
4437

4538
public StopCommand(VirtualMachine vm, boolean executeInSequence) {
4639
super(vm);
@@ -52,10 +45,6 @@ public StopCommand(String vmName, boolean executeInSequence) {
5245
this.executeInSequence = executeInSequence;
5346
}
5447

55-
public String getVnet() {
56-
return vnet;
57-
}
58-
5948
@Override
6049
public boolean executeInSequence() {
6150
return executeInSequence;

‎plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java‎

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,17 @@
3333
import javax.naming.ConfigurationException;
3434
import java.net.URI;
3535
import java.util.Map;
36+
import java.util.regex.Matcher;
37+
import java.util.regex.Pattern;
3638
import java.io.File;
3739

3840
public class BridgeVifDriver extends VifDriverBase {
3941

4042
private static final Logger s_logger = Logger
4143
.getLogger(BridgeVifDriver.class);
4244
private int _timeout;
45+
46+
private static final Object _vnetBridgeMonitor = new Object();
4347
private String _modifyVlanPath;
4448

4549
@Override
@@ -136,7 +140,7 @@ else if (nic.getBroadcastType() == Networks.BroadcastDomainType.Lswitch) {
136140

137141
@Override
138142
public void unplug(LibvirtVMDef.InterfaceDef iface) {
139-
// Nothing needed as libvirt cleans up tap interface from bridge.
143+
deleteVnetBr(iface.getBrName());
140144
}
141145

142146
private String setVnetBrName(String pifName, String vnetId) {
@@ -161,16 +165,64 @@ private String createVlanBr(String vlanId, String nic)
161165

162166
private void createVnet(String vnetId, String pif, String brName)
163167
throws InternalErrorException {
164-
final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
165-
command.add("-v", vnetId);
166-
command.add("-p", pif);
167-
command.add("-b", brName);
168-
command.add("-o", "add");
169-
170-
final String result = command.execute();
171-
if (result != null) {
172-
throw new InternalErrorException("Failed to create vnet " + vnetId
173-
+ ": " + result);
168+
synchronized (_vnetBridgeMonitor) {
169+
final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
170+
command.add("-v", vnetId);
171+
command.add("-p", pif);
172+
command.add("-b", brName);
173+
command.add("-o", "add");
174+
175+
final String result = command.execute();
176+
if (result != null) {
177+
throw new InternalErrorException("Failed to create vnet " + vnetId
178+
+ ": " + result);
179+
}
180+
}
181+
}
182+
183+
private void deleteVnetBr(String brName){
184+
synchronized (_vnetBridgeMonitor) {
185+
String cmdout = Script.runSimpleBashScript("ls /sys/class/net/" + brName + "/brif | grep vnet");
186+
if (cmdout != null && cmdout.contains("vnet")) {
187+
// Active VM remains on that bridge
188+
return;
189+
}
190+
191+
Pattern oldStyleBrNameRegex = Pattern.compile("^cloudVirBr(\\d+)$");
192+
Pattern brNameRegex = Pattern.compile("^br(\\S+)-(\\d+)$");
193+
Matcher oldStyleBrNameMatcher = oldStyleBrNameRegex.matcher(brName);
194+
Matcher brNameMatcher = brNameRegex.matcher(brName);
195+
196+
String pName = null;
197+
String vNetId = null;
198+
if (oldStyleBrNameMatcher.find()) {
199+
// Actually modifyvlan.sh doesn't require pif name when deleting its bridge so far.
200+
pName = "undefined";
201+
vNetId = oldStyleBrNameMatcher.group(1);
202+
} else if (brNameMatcher.find()) {
203+
if(brNameMatcher.group(1) != null || !brNameMatcher.group(1).isEmpty()) {
204+
pName = brNameMatcher.group(1);
205+
} else {
206+
pName = "undefined";
207+
}
208+
vNetId = brNameMatcher.group(2);
209+
}
210+
211+
if (vNetId == null || vNetId.isEmpty()) {
212+
s_logger.debug("unable to get a vNet ID from name "+ brName);
213+
return;
214+
}
215+
216+
final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
217+
command.add("-o", "delete");
218+
command.add("-v", vNetId);
219+
command.add("-p", pName);
220+
command.add("-b", brName);
221+
222+
final String result = command.execute();
223+
if (result != null) {
224+
s_logger.debug("Delete bridge " + brName + " failed: " + result);
225+
}
174226
}
175227
}
176228

‎plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java‎

Lines changed: 7 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,10 +1065,6 @@ private boolean checkOvsNetwork(String networkName) {
10651065
}
10661066
}
10671067

1068-
private String getVnetId(String vnetId) {
1069-
return vnetId;
1070-
}
1071-
10721068
private void passCmdLine(String vmName, String cmdLine)
10731069
throws InternalErrorException {
10741070
final Script command = new Script(_patchViaSocketPath, _timeout, s_logger);
@@ -1694,6 +1690,11 @@ private UnPlugNicAnswer execute(UnPlugNicCommand cmd) {
16941690
for (InterfaceDef pluggedNic : pluggedNics) {
16951691
if (pluggedNic.getMacAddress().equalsIgnoreCase(nic.getMac())) {
16961692
vm.detachDevice(pluggedNic.toString());
1693+
// We don't know which "traffic type" is associated with
1694+
// each interface at this point, so inform all vif drivers
1695+
for(VifDriver vifDriver : getAllVifDrivers()){
1696+
vifDriver.unplug(pluggedNic);
1697+
}
16971698
return new UnPlugNicAnswer(cmd, true, "success");
16981699
}
16991700
}
@@ -2726,8 +2727,6 @@ private synchronized Answer execute(MigrateCommand cmd) {
27262727
vifDriver.unplug(iface);
27272728
}
27282729
}
2729-
cleanupVM(conn, vmName,
2730-
getVnetId(VirtualMachineName.getVnet(vmName)));
27312730
}
27322731

27332732
return new MigrateAnswer(cmd, result == null, result, null);
@@ -3043,11 +3042,6 @@ protected Answer execute(StopCommand cmd) {
30433042
}
30443043
}
30453044

3046-
final String result2 = cleanupVnet(conn, cmd.getVnet());
3047-
3048-
if (result != null && result2 != null) {
3049-
result = result2 + result;
3050-
}
30513045
state = State.Stopped;
30523046
return new StopAnswer(cmd, result, 0, true);
30533047
} catch (LibvirtException e) {
@@ -4123,16 +4117,6 @@ protected List<Object> getHostInfo() {
41234117
return info;
41244118
}
41254119

4126-
protected void cleanupVM(Connect conn, final String vmName,
4127-
final String vnet) {
4128-
s_logger.debug("Trying to cleanup the vnet: " + vnet);
4129-
if (vnet != null) {
4130-
cleanupVnet(conn, vnet);
4131-
}
4132-
4133-
_vmStats.remove(vmName);
4134-
}
4135-
41364120
protected String rebootVM(Connect conn, String vmName) {
41374121
Domain dm = null;
41384122
String msg = null;
@@ -4274,29 +4258,6 @@ protected String stopVM(Connect conn, String vmName, boolean force) {
42744258
return null;
42754259
}
42764260

4277-
public synchronized String cleanupVnet(Connect conn, final String vnetId) {
4278-
// VNC proxy VMs do not have vnet
4279-
if (vnetId == null || vnetId.isEmpty()
4280-
|| isDirectAttachedNetwork(vnetId)) {
4281-
return null;
4282-
}
4283-
4284-
final List<String> names = getAllVmNames(conn);
4285-
4286-
if (!names.isEmpty()) {
4287-
for (final String name : names) {
4288-
if (VirtualMachineName.getVnet(name).equals(vnetId)) {
4289-
return null; // Can't remove the vnet yet.
4290-
}
4291-
}
4292-
}
4293-
4294-
final Script command = new Script(_modifyVlanPath, _timeout, s_logger);
4295-
command.add("-o", "delete");
4296-
command.add("-v", vnetId);
4297-
return command.execute();
4298-
}
4299-
43004261
protected Integer getVncPort(Connect conn, String vmName)
43014262
throws LibvirtException {
43024263
LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
@@ -4410,26 +4371,11 @@ private DiskDef.diskBus getGuestDiskModel(String guestOSType) {
44104371
}
44114372
}
44124373

4413-
private String getVnetIdFromBrName(String vnetBrName) {
4414-
if (vnetBrName.contains("cloudVirBr")) {
4415-
return vnetBrName.replaceAll("cloudVirBr", "");
4416-
} else {
4417-
Pattern r = Pattern.compile("-(\\d+)$");
4418-
Matcher m = r.matcher(vnetBrName);
4419-
if(m.group(1) != null || !m.group(1).isEmpty()) {
4420-
return m.group(1);
4421-
} else {
4422-
s_logger.debug("unable to get a vlan ID from name " + vnetBrName);
4423-
return "";
4424-
}
4425-
}
4426-
}
4427-
44284374
private void cleanupVMNetworks(Connect conn, List<InterfaceDef> nics) {
44294375
if (nics != null) {
44304376
for (InterfaceDef nic : nics) {
4431-
if (nic.getHostNetType() == hostNicType.VNET) {
4432-
cleanupVnet(conn, getVnetIdFromBrName(nic.getBrName()));
4377+
for(VifDriver vifDriver : getAllVifDrivers()){
4378+
vifDriver.unplug(nic);
44334379
}
44344380
}
44354381
}

‎plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
public class OvsVifDriver extends VifDriverBase {
3939
private static final Logger s_logger = Logger.getLogger(OvsVifDriver.class);
4040
private int _timeout;
41-
private String _modifyVlanPath;
4241

4342
@Override
4443
public void configure(Map<String, Object> params) throws ConfigurationException {
@@ -52,11 +51,6 @@ public void configure(Map<String, Object> params) throws ConfigurationException
5251
String value = (String) params.get("scripts.timeout");
5352
_timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
5453

55-
_modifyVlanPath = Script.findScript(networkScriptsDir, "modifyvlan.sh");
56-
if (_modifyVlanPath == null) {
57-
throw new ConfigurationException("Unable to find modifyvlan.sh");
58-
}
59-
6054
createControlNetwork(_bridges.get("linklocal"));
6155
}
6256

0 commit comments

Comments
 (0)