Merge Libvirt....VirtualPortDriver class into LibvirtGenericVIFDriver

The LibvirtOpenVswitchVirtualPortDriver VIF driver and the
LibvirtOpenVswitchDriver VIF driver both support OpenVswitch
based guest configs. The only difference is that the former
uses libvirt's modern <network type='bridge'> with vport
profile specified, while the latter uses <network type='ethernet'>
and does all openvswitch setup itself. The reason for the latter
is to support running against libvirt versions older than 0.9.11
Rather than require the host admin to figure this out themselves,
the right behaviour can be choosen automatically based on the
libvirt connection's version number.

The LibvirtGenericVIFDriver class can use the 'vif_type' mapping
field to determine whether an OVS network configuration is
required. In combination with a check against the libvirt version
number, a dedicated driver for OVS bridge setup is no longer
required.

The LibvirtOpenVswitchVirtualPortDriver class functionality is
merged into LibvirtGenericVIFDriver. For backwards compatibility
with the Folsom release, the existing LibvirtOpenVswitchVirtualPortDriver
class is made to inherit from LibvirtGenericVIFDriver and directly
call the bridge specific setup APIs. This eases migration to
the new VIF impl during the Grizzly deployment lifecycle, with
an expectation that the LibvirtOpenVswitchVirtualPortDriver stub
will be deleted after the Havana release.

Blueprint: libvirt-vif-driver
Change-Id: I292c2fd7aa0c32b1c91d737152ed8e54ea2cf6ac
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

5 files changed, 149 insertions, 46 deletions

diff --git a/nova/tests/fake_network.py b/nova/tests/fake_network.py
index b97999e7d..883466cd6 100644
--- a/nova/tests/fake_network.py
+++ b/nova/tests/fake_network.py
@@ -47,7 +47,7 @@ class FakeIptablesFirewallDriver(object):
 
 class FakeVIFDriver(object):
 
-    def __init__(self, **kwargs):
+    def __init__(self, *args, **kwargs):
         pass
 
     def setattr(self, key, val):
@@ -65,6 +65,9 @@ class FakeVIFDriver(object):
     def plug(self, instance, vif):
         pass
 
+    def unplug(self, instance, vif):
+        pass
+
 
 class FakeModel(dict):
     """Represent a model from the db."""
diff --git a/nova/tests/fakelibvirt.py b/nova/tests/fakelibvirt.py
index 259d192cb..6abe7771c 100644
--- a/nova/tests/fakelibvirt.py
+++ b/nova/tests/fakelibvirt.py
@@ -481,7 +481,7 @@ class DomainSnapshot(object):
 
 
 class Connection(object):
-    def __init__(self, uri, readonly):
+    def __init__(self, uri, readonly, version=9007):
         if not uri or uri == '':
             if allow_default_uri_connection:
                 uri = 'qemu:///session'
@@ -506,6 +506,8 @@ class Connection(object):
         self._running_vms = {}
         self._id_counter = 1  # libvirt reserves 0 for the hypervisor.
         self._nwfilters = {}
+        self.fakeLibVersion = version
+        self.fakeVersion = version
 
     def _add_filter(self, nwfilter):
         self._nwfilters[nwfilter._name] = nwfilter
@@ -576,10 +578,10 @@ class Connection(object):
             return 'QEMU'
 
     def getLibVersion(self):
-        return 9007
+        return self.fakeLibVersion
 
     def getVersion(self):
-        return 14000
+        return self.fakeVersion
 
     def getHostname(self):
         return 'compute1'
diff --git a/nova/tests/test_libvirt_vif.py b/nova/tests/test_libvirt_vif.py
index 6a3530588..5c3d148b7 100644
--- a/nova/tests/test_libvirt_vif.py
+++ b/nova/tests/test_libvirt_vif.py
@@ -20,6 +20,7 @@ from nova import exception
 from nova.network import model as network_model
 from nova.openstack.common import cfg
 from nova import test
+from nova.tests import fakelibvirt
 from nova import utils
 from nova.virt.libvirt import config as vconfig
 from nova.virt.libvirt import vif
@@ -193,7 +194,10 @@ class LibvirtVifTestCase(test.TestCase):
         self.flags(libvirt_use_virtio_for_bridges=False,
                    libvirt_type='kvm')
 
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         xml = self._get_instance_xml(d,
                                      self.net_bridge,
                                      self.mapping_bridge)
@@ -212,7 +216,10 @@ class LibvirtVifTestCase(test.TestCase):
         self.flags(libvirt_use_virtio_for_bridges=True,
                    libvirt_type='kvm')
 
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         xml = self._get_instance_xml(d,
                                      self.net_bridge,
                                      self.mapping_bridge)
@@ -231,7 +238,10 @@ class LibvirtVifTestCase(test.TestCase):
         self.flags(libvirt_use_virtio_for_bridges=True,
                    libvirt_type='qemu')
 
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         xml = self._get_instance_xml(d,
                                      self.net_bridge,
                                      self.mapping_bridge)
@@ -250,7 +260,10 @@ class LibvirtVifTestCase(test.TestCase):
         self.flags(libvirt_use_virtio_for_bridges=True,
                    libvirt_type='xen')
 
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("xen:///system",
+                                          False)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         xml = self._get_instance_xml(d,
                                      self.net_bridge,
                                      self.mapping_bridge)
@@ -266,7 +279,10 @@ class LibvirtVifTestCase(test.TestCase):
         self.assertEqual(len(ret), 0)
 
     def test_generic_driver_none(self):
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         self.assertRaises(exception.NovaException,
                           self._get_instance_xml,
                           d,
@@ -287,23 +303,32 @@ class LibvirtVifTestCase(test.TestCase):
         self.assertEqual(mac, self.mapping_bridge['mac'])
 
     def test_bridge_driver(self):
-        d = vif.LibvirtBridgeDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtBridgeDriver(get_connection)
         self._check_bridge_driver(d,
                                   self.net_bridge,
                                   self.mapping_bridge,
                                   self.net_bridge['bridge'])
 
     def test_generic_driver_bridge(self):
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         self._check_bridge_driver(d,
                                   self.net_bridge,
                                   self.mapping_bridge,
                                   self.net_bridge['bridge'])
 
     def test_quantum_bridge_driver(self):
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.QuantumLinuxBridgeVIFDriver(get_connection)
         br_want = 'brq' + self.net_bridge_quantum['id']
         br_want = br_want[:network_model.NIC_NAME_LEN]
-        d = vif.QuantumLinuxBridgeVIFDriver()
         self._check_bridge_driver(d,
                                   self.net_bridge_quantum,
                                   self.mapping_bridge_quantum,
@@ -325,22 +350,28 @@ class LibvirtVifTestCase(test.TestCase):
         self.assertEquals(script, "")
 
     def test_ovs_ethernet_driver(self):
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False,
+                                          9010)
+        d = vif.LibvirtOpenVswitchDriver(get_connection)
         d = vif.LibvirtOpenVswitchDriver()
         self._check_ovs_ethernet_driver(d,
                                         self.net_ovs,
                                         self.mapping_ovs_legacy)
 
     def test_ovs_ethernet_driver(self):
-        d = vif.LibvirtGenericVIFDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False,
+                                          9010)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
         self._check_ovs_ethernet_driver(d,
                                         self.net_ovs,
                                         self.mapping_ovs)
 
-    def test_ovs_virtualport_driver(self):
-        d = vif.LibvirtOpenVswitchVirtualPortDriver()
-        xml = self._get_instance_xml(d,
-                                     self.net_ovs,
-                                     self.mapping_ovs)
+    def _check_ovs_virtualport_driver(self, d, net, mapping, want_iface_id):
+        xml = self._get_instance_xml(d, net, mapping)
 
         doc = etree.fromstring(xml)
         ret = doc.findall('./devices/interface')
@@ -351,21 +382,47 @@ class LibvirtVifTestCase(test.TestCase):
         br_name = node.find("source").get("bridge")
         self.assertEqual(br_name, "br0")
         mac = node.find("mac").get("address")
-        self.assertEqual(mac, self.mapping_ovs['mac'])
+        self.assertEqual(mac, mapping['mac'])
         vp = node.find("virtualport")
         self.assertEqual(vp.get("type"), "openvswitch")
         iface_id_found = False
         for p_elem in vp.findall("parameters"):
             iface_id = p_elem.get("interfaceid", None)
             if iface_id:
-                self.assertEqual(iface_id,
-                                 self.mapping_ovs['ovs_interfaceid'])
+                self.assertEqual(iface_id, want_iface_id)
                 iface_id_found = True
 
         self.assertTrue(iface_id_found)
 
+    def test_ovs_virtualport_driver(self):
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False,
+                                          9011)
+        d = vif.LibvirtOpenVswitchVirtualPortDriver(get_connection)
+        want_iface_id = 'vif-xxx-yyy-zzz'
+        self._check_ovs_virtualport_driver(d,
+                                           self.net_ovs,
+                                           self.mapping_ovs_legacy,
+                                           want_iface_id)
+
+    def test_generic_ovs_virtualport_driver(self):
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False,
+                                          9011)
+        d = vif.LibvirtGenericVIFDriver(get_connection)
+        want_iface_id = self.mapping_ovs['ovs_interfaceid']
+        self._check_ovs_virtualport_driver(d,
+                                           self.net_ovs,
+                                           self.mapping_ovs,
+                                           want_iface_id)
+
     def test_quantum_hybrid_driver(self):
-        d = vif.LibvirtHybridOVSBridgeDriver()
+        def get_connection():
+            return fakelibvirt.Connection("qemu:///session",
+                                          False)
+        d = vif.LibvirtHybridOVSBridgeDriver(get_connection)
         xml = self._get_instance_xml(d,
                                      self.net_ovs,
                                      self.mapping_ovs)
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index fdf37fe31..666eb66f3 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -285,7 +285,9 @@ class LibvirtDriver(driver.ComputeDriver):
             DEFAULT_FIREWALL_DRIVER,
             self.virtapi,
             get_connection=self._get_connection)
-        self.vif_driver = importutils.import_object(CONF.libvirt_vif_driver)
+
+        vif_class = importutils.import_class(CONF.libvirt_vif_driver)
+        self.vif_driver = vif_class(self._get_connection)
 
         self.volume_drivers = driver.driver_dict_from_config(
             CONF.libvirt_volume_drivers, self)
@@ -1880,7 +1882,7 @@ class LibvirtDriver(driver.ComputeDriver):
         for (network, mapping) in network_info:
             self.vif_driver.plug(instance, (network, mapping))
             cfg = self.vif_driver.get_config(instance,
-                                                     network, mapping)
+                                             network, mapping)
             guest.add_device(cfg)
 
         if CONF.libvirt_type == "qemu" or CONF.libvirt_type == "kvm":
diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py
index 131fb10e9..1110d356e 100644
--- a/nova/virt/libvirt/vif.py
+++ b/nova/virt/libvirt/vif.py
@@ -47,9 +47,26 @@ CONF.register_opts(libvirt_vif_opts)
 CONF.import_opt('libvirt_type', 'nova.virt.libvirt.driver')
 CONF.import_opt('use_ipv6', 'nova.netconf')
 
+# Since libvirt 0.9.11, <interface type='bridge'>
+# supports OpenVSwitch natively.
+LIBVIRT_OVS_VPORT_VERSION = 9011
+
 
 class LibvirtBaseVIFDriver(object):
 
+    def __init__(self, get_connection):
+        self.get_connection = get_connection
+        self.libvirt_version = None
+
+    def has_libvirt_version(self, want):
+        if self.libvirt_version is None:
+            conn = self.get_connection()
+            self.libvirt_version = conn.getLibVersion()
+
+        if self.libvirt_version >= want:
+            return True
+        return False
+
     def get_vif_devname(self, mapping):
         if 'vif_devname' in mapping:
             return mapping['vif_devname']
@@ -130,9 +147,26 @@ class LibvirtGenericVIFDriver(LibvirtBaseVIFDriver):
 
         return conf
 
+    def get_config_ovs_bridge(self, instance, network, mapping):
+        conf = super(LibvirtGenericVIFDriver,
+                     self).get_config(instance,
+                                      network,
+                                      mapping)
+
+        designer.set_vif_host_backend_ovs_config(
+            conf, self.get_bridge_name(network),
+            self.get_ovs_interfaceid(mapping),
+            self.get_vif_devname(mapping))
+
+        return conf
+
     def get_config_ovs(self, instance, network, mapping):
-        return self.get_config_ovs_ethernet(instance, network,
-                                            mapping)
+        if self.has_libvirt_version(LIBVIRT_OVS_VPORT_VERSION):
+            return self.get_config_ovs_bridge(instance, network,
+                                              mapping)
+        else:
+            return self.get_config_ovs_ethernet(instance, network,
+                                                mapping)
 
     def get_config(self, instance, network, mapping):
         vif_type = mapping.get('vif_type')
@@ -192,8 +226,16 @@ class LibvirtGenericVIFDriver(LibvirtBaseVIFDriver):
                                       dev, iface_id, mapping['mac'],
                                       instance['uuid'])
 
+    def plug_ovs_bridge(self, instance, vif):
+        """No manual plugging required."""
+        super(LibvirtGenericVIFDriver,
+              self).plug(instance, vif)
+
     def plug_ovs(self, instance, vif):
-        self.plug_ovs_ethernet(instance, vif)
+        if self.has_libvirt_version(LIBVIRT_OVS_VPORT_VERSION):
+            self.plug_ovs_bridge(instance, vif)
+        else:
+            self.plug_ovs_ethernet(instance, vif)
 
     def plug(self, instance, vif):
         network, mapping = vif
@@ -233,8 +275,16 @@ class LibvirtGenericVIFDriver(LibvirtBaseVIFDriver):
         except exception.ProcessExecutionError:
             LOG.exception(_("Failed while unplugging vif"), instance=instance)
 
+    def unplug_ovs_bridge(self, instance, vif):
+        """No manual unplugging required."""
+        super(LibvirtGenericVIFDriver,
+              self).unplug(instance, vif)
+
     def unplug_ovs(self, instance, vif):
-        return self.unplug_ovs_ethernet(instance, vif)
+        if self.has_libvirt_version(LIBVIRT_OVS_VPORT_VERSION):
+            self.unplug_ovs_bridge(instance, vif)
+        else:
+            self.unplug_ovs_ethernet(instance, vif)
 
     def unplug(self, instance, vif):
         network, mapping = vif
@@ -371,9 +421,10 @@ class LibvirtHybridOVSBridgeDriver(LibvirtBridgeDriver):
             LOG.exception(_("Failed while unplugging vif"), instance=instance)
 
 
-class LibvirtOpenVswitchVirtualPortDriver(LibvirtBaseVIFDriver):
-    """VIF driver for Open vSwitch that uses integrated libvirt
-       OVS virtual port XML (introduced in libvirt 0.9.11)."""
+class LibvirtOpenVswitchVirtualPortDriver(LibvirtGenericVIFDriver):
+    """Retained in Grizzly for compatibility with Quantum
+       drivers which do not yet report 'vif_type' port binding.
+       Will be deprecated in Havana, and removed in Ixxxx."""
 
     def get_bridge_name(self, network):
         return network.get('bridge') or CONF.libvirt_ovs_bridge
@@ -382,25 +433,13 @@ class LibvirtOpenVswitchVirtualPortDriver(LibvirtBaseVIFDriver):
         return mapping.get('ovs_interfaceid') or mapping['vif_uuid']
 
     def get_config(self, instance, network, mapping):
-        """Pass data required to create OVS virtual port element."""
-        conf = super(LibvirtOpenVswitchVirtualPortDriver,
-                     self).get_config(instance,
-                                      network,
-                                      mapping)
-
-        designer.set_vif_host_backend_ovs_config(
-            conf, self.get_bridge_name(network),
-            self.get_ovs_interfaceid(mapping),
-            self.get_vif_devname(mapping))
-
-        return conf
+        return self.get_config_ovs_bridge(instance, network, mapping)
 
     def plug(self, instance, vif):
-        pass
+        return self.plug_ovs_bridge(instance, vif)
 
     def unplug(self, instance, vif):
-        """No action needed.  Libvirt takes care of cleanup."""
-        pass
+        return self.unplug_ovs_bridge(instance, vif)
 
 
 class QuantumLinuxBridgeVIFDriver(LibvirtGenericVIFDriver):