mirror of https://github.com/apache/cloudstack.git
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 <htrippaers@schubergphilis.com>
This commit is contained in:
Dave Cahill
Hugo Trippaers
parent
b12aebefee
commit
ff099fa651
|
|
@ -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) {
|
||||
if (_bridgeType == BridgeType.OPENVSWITCH) {
|
||||
s_logger.info("No libvirt.vif.driver specififed. Defaults to OvsVifDriver.");
|
||||
vifDriverName = "com.cloud.hypervisor.kvm.resource.OvsVifDriver";
|
||||
} else {
|
||||
s_logger.info("No libvirt.vif.driver specififed. Defaults to BridgeVifDriver.");
|
||||
vifDriverName = "com.cloud.hypervisor.kvm.resource.BridgeVifDriver";
|
||||
}
|
||||
}
|
||||
|
||||
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 specified. Defaults to OvsVifDriver.");
|
||||
defaultVifDriverName = DEFAULT_OVS_VIF_DRIVER_CLASS_NAME;
|
||||
} else {
|
||||
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));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue