Adding support for multiple VIF drivers in KVM
- Building map of {trafficType, vifDriver} at configure time
- Use the relevant VIF driver for the given traffic type when call plug()
- Inform all vif drivers when call unplug(), as we no longer know traffic type
- Refactor VIF driver choosing code and add unit tests
- Basic unit tests, just test default case
- Also slight refactor of unit test code, and use jUnit 4 instead of 3, to
match rest of codebase
Signed-off-by: Hugo Trippaers <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
Commit:
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/ff099fa6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/ff099fa6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/ff099fa6
Branch: refs/heads/ui-multiple-nics
Commit: ff099fa651e8476220332d7e1cc4b24746802cfe
Parents: b12aebe
Author: Dave Cahill <[email protected]>
Authored: Fri Feb 22 17:04:42 2013 +0900
Committer: Hugo Trippaers <[email protected]>
Committed: Mon Mar 4 20:56:34 2013 +0100
----------------------------------------------------------------------
.../kvm/resource/LibvirtComputingResource.java | 116 ++++++--
.../kvm/resource/LibvirtVifDriverTest.java | 226 +++++++++++++++
2 files changed, 322 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ff099fa6/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 805de40..5a96c36 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
@@ -41,6 +41,8 @@ import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
+import java.util.HashSet;
import java.util.Properties;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
@@ -276,7 +278,11 @@ ServerResource {
private String _mountPoint = "/mnt";
StorageLayer _storage;
private KVMStoragePoolManager _storagePoolMgr;
- private VifDriver _vifDriver;
+
+ private VifDriver _defaultVifDriver;
+ private Map<TrafficType, VifDriver> _trafficTypeVifDrivers;
+ protected static final String DEFAULT_OVS_VIF_DRIVER_CLASS_NAME =
"com.cloud.hypervisor.kvm.resource.OvsVifDriver";
+ protected static final String DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME =
"com.cloud.hypervisor.kvm.resource.BridgeVifDriver";
private static final class KeyValueInterpreter extends OutputInterpreter {
private final Map<String, String> map = new HashMap<String, String>();
@@ -775,24 +781,66 @@ ServerResource {
params.put("libvirt.host.bridges", bridges);
params.put("libvirt.host.pifs", _pifs);
- // Load the vif driver
- String vifDriverName = (String) params.get("libvirt.vif.driver");
- if (vifDriverName == null) {
+ params.put("libvirt.computing.resource", this);
+
+ configureVifDrivers(params);
+
+ return true;
+ }
+
+ protected void configureVifDrivers(Map<String, Object> params)
+ throws ConfigurationException {
+ final String LIBVIRT_VIF_DRIVER = "libvirt.vif.driver";
+
+ _trafficTypeVifDrivers = new HashMap<TrafficType, VifDriver>();
+
+ // Load the default vif driver
+ String defaultVifDriverName = (String) params.get(LIBVIRT_VIF_DRIVER);
+ if (defaultVifDriverName == null) {
if (_bridgeType == BridgeType.OPENVSWITCH) {
- s_logger.info("No libvirt.vif.driver specififed. Defaults to
OvsVifDriver.");
- vifDriverName =
"com.cloud.hypervisor.kvm.resource.OvsVifDriver";
+ s_logger.info("No libvirt.vif.driver specified. Defaults to
OvsVifDriver.");
+ defaultVifDriverName = DEFAULT_OVS_VIF_DRIVER_CLASS_NAME;
} else {
- s_logger.info("No libvirt.vif.driver specififed. Defaults to
BridgeVifDriver.");
- vifDriverName =
"com.cloud.hypervisor.kvm.resource.BridgeVifDriver";
- }
+ s_logger.info("No libvirt.vif.driver specified. Defaults to
BridgeVifDriver.");
+ defaultVifDriverName = DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME;
+ }
+ }
+ _defaultVifDriver = getVifDriverClass(defaultVifDriverName, params);
+
+ // Load any per-traffic-type vif drivers
+ for (Map.Entry<String, Object> entry : params.entrySet())
+ {
+ String k = entry.getKey();
+ String vifDriverPrefix = LIBVIRT_VIF_DRIVER + ".";
+
+ if(k.startsWith(vifDriverPrefix)){
+ // Get trafficType
+ String trafficTypeSuffix =
k.substring(vifDriverPrefix.length());
+
+ // Does this suffix match a real traffic type?
+ TrafficType trafficType =
TrafficType.getTrafficType(trafficTypeSuffix);
+ if(!trafficType.equals(TrafficType.None)){
+ // Get vif driver class name
+ String vifDriverClassName = (String) entry.getValue();
+ // if value is null, ignore
+ if(vifDriverClassName != null){
+ // add traffic type to vif driver mapping to Map
+ _trafficTypeVifDrivers.put(trafficType,
+ getVifDriverClass(vifDriverClassName, params));
+ }
+ }
+ }
}
+ }
- params.put("libvirt.computing.resource", this);
+ protected VifDriver getVifDriverClass(String vifDriverClassName,
Map<String, Object> params)
+ throws ConfigurationException {
+ VifDriver vifDriver;
try {
- Class<?> clazz = Class.forName(vifDriverName);
- _vifDriver = (VifDriver) clazz.newInstance();
- _vifDriver.configure(params);
+ Class<?> clazz = Class.forName(vifDriverClassName);
+ vifDriver = (VifDriver) clazz.newInstance();
+ vifDriver.configure(params);
} catch (ClassNotFoundException e) {
throw new ConfigurationException("Unable to find class for
libvirt.vif.driver " + e);
} catch (InstantiationException e) {
@@ -800,8 +848,28 @@ ServerResource {
} catch (Exception e) {
throw new ConfigurationException("Failed to initialize
libvirt.vif.driver " + e);
}
+ return vifDriver;
+ }
- return true;
+ protected VifDriver getVifDriver(TrafficType trafficType){
+ VifDriver vifDriver = _trafficTypeVifDrivers.get(trafficType);
+
+ if(vifDriver == null){
+ vifDriver = _defaultVifDriver;
+ }
+
+ return vifDriver;
+ }
+
+ protected List<VifDriver> getAllVifDrivers(){
+ Set<VifDriver> vifDrivers = new HashSet<VifDriver>();
+
+ vifDrivers.add(_defaultVifDriver);
+ vifDrivers.addAll(_trafficTypeVifDrivers.values());
+
+ ArrayList<VifDriver> vifDriverList = new
ArrayList<VifDriver>(vifDrivers);
+
+ return vifDriverList;
}
private void getPifs() {
@@ -1443,7 +1511,7 @@ ServerResource {
}
Domain vm = getDomain(conn, vmName);
- vm.attachDevice(_vifDriver.plug(nicTO, "Other PV
(32-bit)").toString());
+ vm.attachDevice(getVifDriver(nicTO.getType()).plug(nicTO, "Other PV
(32-bit)").toString());
}
private PlugNicAnswer execute(PlugNicCommand cmd) {
@@ -1462,7 +1530,7 @@ ServerResource {
}
nicnum++;
}
- vm.attachDevice(_vifDriver.plug(nic, "Other PV
(32-bit)").toString());
+ vm.attachDevice(getVifDriver(nic.getType()).plug(nic, "Other PV
(32-bit)").toString());
return new PlugNicAnswer(cmd, true, "success");
} catch (Exception e) {
String msg = " Plug Nic failed due to " + e.toString();
@@ -2560,7 +2628,11 @@ ServerResource {
} else {
destroy_network_rules_for_vm(conn, vmName);
for (InterfaceDef iface : ifaces) {
- _vifDriver.unplug(iface);
+ // 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(iface);
+ }
}
cleanupVM(conn, vmName,
getVnetId(VirtualMachineName.getVnet(vmName)));
@@ -2580,7 +2652,7 @@ ServerResource {
try {
Connect conn = LibvirtConnection.getConnection();
for (NicTO nic : nics) {
- _vifDriver.plug(nic, null);
+ getVifDriver(nic.getType()).plug(nic, null);
}
/* setup disks, e.g for iso */
@@ -2794,7 +2866,11 @@ ServerResource {
}
}
for (InterfaceDef iface: ifaces) {
- _vifDriver.unplug(iface);
+ // 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(iface);
+ }
}
}
@@ -3231,7 +3307,7 @@ ServerResource {
private void createVif(LibvirtVMDef vm, NicTO nic)
throws InternalErrorException, LibvirtException {
vm.getDevices().addDevice(
- _vifDriver.plug(nic, vm.getGuestOSType()).toString());
+ getVifDriver(nic.getType()).plug(nic,
vm.getGuestOSType()).toString());
}
protected CheckSshAnswer execute(CheckSshCommand cmd) {
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ff099fa6/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtVifDriverTest.java
----------------------------------------------------------------------
diff --git
a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtVifDriverTest.java
b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtVifDriverTest.java
new file mode 100644
index 0000000..7d47e6e
--- /dev/null
+++
b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtVifDriverTest.java
@@ -0,0 +1,226 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.cloud.hypervisor.kvm.resource;
+
+import com.cloud.network.Networks.TrafficType;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.BridgeType;
+
+import org.junit.Before;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.HashMap;
+import java.util.Map;
+import javax.naming.ConfigurationException;
+
+
+import static org.mockito.Mockito.*;
+
+public class LibvirtVifDriverTest {
+ private LibvirtComputingResource res;
+
+ private Map<TrafficType, VifDriver> assertions;
+
+ final String LIBVIRT_VIF_DRIVER = "libvirt.vif.driver";
+ final String FAKE_VIF_DRIVER_CLASS_NAME =
"com.cloud.hypervisor.kvm.resource.FakeVifDriver";
+ final String NONEXISTENT_VIF_DRIVER_CLASS_NAME =
"com.cloud.hypervisor.kvm.resource.NonExistentVifDriver";
+
+ private VifDriver fakeVifDriver, bridgeVifDriver, ovsVifDriver;
+
+ @Before
+ public void setUp() {
+ // Use a spy because we only want to override getVifDriverClass
+ LibvirtComputingResource resReal = new LibvirtComputingResource();
+ res = spy(resReal);
+
+ try{
+ bridgeVifDriver =
+ (VifDriver)
Class.forName(LibvirtComputingResource.DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME).newInstance();
+ ovsVifDriver =
+ (VifDriver)
Class.forName(LibvirtComputingResource.DEFAULT_OVS_VIF_DRIVER_CLASS_NAME).newInstance();
+
+ // Instantiating bridge vif driver again as the fake vif driver
+ // is good enough, as this is a separate instance
+ fakeVifDriver =
+ (VifDriver)
Class.forName(LibvirtComputingResource.DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME).newInstance();
+
+ doReturn(bridgeVifDriver).when(res)
+
.getVifDriverClass(eq(LibvirtComputingResource.DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME),
anyMap());
+ doReturn(ovsVifDriver).when(res)
+
.getVifDriverClass(eq(LibvirtComputingResource.DEFAULT_OVS_VIF_DRIVER_CLASS_NAME),
anyMap());
+ doReturn(fakeVifDriver).when(res)
+ .getVifDriverClass(eq(FAKE_VIF_DRIVER_CLASS_NAME),
anyMap());
+
+ } catch (final ConfigurationException ex){
+ fail("Unexpected ConfigurationException while configuring VIF
drivers: " + ex.getMessage());
+ } catch (final Exception ex){
+ fail("Unexpected Exception while configuring VIF drivers");
+ }
+
+ assertions = new HashMap<TrafficType, VifDriver>();
+ }
+
+
+ // Helper function
+ // Configure LibvirtComputingResource using params
+ private void configure (Map<String, Object> params)
+ throws ConfigurationException{
+ res.configureVifDrivers(params);
+ }
+
+ // Helper function
+ private void checkAssertions(){
+ // Check the defined assertions
+ for (Map.Entry<TrafficType, VifDriver> assertion :
assertions.entrySet()){
+ assertEquals(res.getVifDriver(assertion.getKey()),
+ assertion.getValue());
+ }
+ }
+
+ // Helper when all answers should be the same
+ private void checkAllSame(VifDriver vifDriver)
+ throws ConfigurationException {
+
+ for(TrafficType trafficType : TrafficType.values()){
+ assertions.put(trafficType, vifDriver);
+ }
+
+ checkAssertions();
+ }
+
+ @Test
+ public void testDefaults()
+ throws ConfigurationException {
+ // If no special vif driver settings, all traffic types should
+ // map to the default vif driver for the bridge type
+ Map<String, Object> params = new HashMap<String, Object>();
+
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+ checkAllSame(bridgeVifDriver);
+
+ res._bridgeType = BridgeType.OPENVSWITCH;
+ configure(params);
+ checkAllSame(ovsVifDriver);
+ }
+
+ @Test
+ public void testDefaultsWhenExplicitlySet()
+ throws ConfigurationException {
+
+ Map<String, Object> params = new HashMap<String, Object>();
+
+ // Switch res' bridge type for test purposes
+ params.put(LIBVIRT_VIF_DRIVER,
LibvirtComputingResource.DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+ checkAllSame(bridgeVifDriver);
+
+ params.clear();
+ params.put(LIBVIRT_VIF_DRIVER,
LibvirtComputingResource.DEFAULT_OVS_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.OPENVSWITCH;
+ configure(params);
+ checkAllSame(ovsVifDriver);
+ }
+
+ @Test
+ public void testWhenExplicitlySetDifferentDefault()
+ throws ConfigurationException {
+
+ // Tests when explicitly set vif driver to OVS when using regular
bridges and vice versa
+ Map<String, Object> params = new HashMap<String, Object>();
+
+ // Switch res' bridge type for test purposes
+ params.put(LIBVIRT_VIF_DRIVER,
LibvirtComputingResource.DEFAULT_OVS_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+ checkAllSame(ovsVifDriver);
+
+ params.clear();
+ params.put(LIBVIRT_VIF_DRIVER,
LibvirtComputingResource.DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.OPENVSWITCH;
+ configure(params);
+ checkAllSame(bridgeVifDriver);
+ }
+
+ @Test
+ public void testOverrideSomeTrafficTypes()
+ throws ConfigurationException {
+
+ Map<String, Object> params = new HashMap<String, Object>();
+ params.put(LIBVIRT_VIF_DRIVER + "." + "Public",
FAKE_VIF_DRIVER_CLASS_NAME);
+ params.put(LIBVIRT_VIF_DRIVER + "." + "Guest",
+ LibvirtComputingResource.DEFAULT_OVS_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+
+ // Initially, set all traffic types to use default
+ for(TrafficType trafficType : TrafficType.values()){
+ assertions.put(trafficType, bridgeVifDriver);
+ }
+
+ assertions.put(TrafficType.Public, fakeVifDriver);
+ assertions.put(TrafficType.Guest, ovsVifDriver);
+
+ checkAssertions();
+ }
+
+ @Test
+ public void testBadTrafficType()
+ throws ConfigurationException {
+ Map<String, Object> params = new HashMap<String, Object>();
+ params.put(LIBVIRT_VIF_DRIVER + "." + "NonExistentTrafficType",
FAKE_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+
+ // Set all traffic types to use default, because bad traffic type
should be ignored
+ for(TrafficType trafficType : TrafficType.values()){
+ assertions.put(trafficType, bridgeVifDriver);
+ }
+
+ checkAssertions();
+ }
+
+ @Test
+ public void testEmptyTrafficType()
+ throws ConfigurationException {
+ Map<String, Object> params = new HashMap<String, Object>();
+ params.put(LIBVIRT_VIF_DRIVER + ".", FAKE_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+
+ // Set all traffic types to use default, because bad traffic type
should be ignored
+ for(TrafficType trafficType : TrafficType.values()){
+ assertions.put(trafficType, bridgeVifDriver);
+ }
+
+ checkAssertions();
+ }
+
+ @Test(expected=ConfigurationException.class)
+ public void testBadVifDriverClassName()
+ throws ConfigurationException {
+ Map<String, Object> params = new HashMap<String, Object>();
+ params.put(LIBVIRT_VIF_DRIVER + "." + "Public",
NONEXISTENT_VIF_DRIVER_CLASS_NAME);
+ res._bridgeType = BridgeType.NATIVE;
+ configure(params);
+ }
+}
