cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bfede...@apache.org
Subject [30/50] [abbrv] git commit: refs/heads/ui-multiple-nics - Adding support for multiple VIF drivers in KVM
Date Tue, 05 Mar 2013 18:37:39 GMT
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>


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 <dcahill@midokura.com>
Authored: Fri Feb 22 17:04:42 2013 +0900
Committer: Hugo Trippaers <trippie@gmail.com>
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);
+    }
+}


Mime
View raw message