Enable Quantum linux bridge VIF driver to use "bridge" type

Fixes bug 1078210

This fix should be used in conjunction with the patch
https://review.openstack.org/#/c/14961/

Change-Id: Ifd79864db147fe3141e03bf1c931fd153d413305

5 files changed, 88 insertions, 36 deletions

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 994a3c0f4..193b49202 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -1146,11 +1146,14 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver):
 
     @classmethod
     @lockutils.synchronized('ensure_bridge', 'nova-', external=True)
-    def ensure_bridge(_self, bridge, interface, net_attrs=None, gateway=True):
+    def ensure_bridge(_self, bridge, interface, net_attrs=None, gateway=True,
+                      filtering=True):
         """Create a bridge unless it already exists.
 
         :param interface: the interface to create the bridge on.
         :param net_attrs: dictionary with  attributes used to create bridge.
+        :param gateway: whether or not the bridge is a gateway.
+        :param filtering: whether or not to create filters on the bridge.
 
         If net_attrs is set, it will add the net_attrs['gateway'] to the bridge
         using net_attrs['broadcast'] and net_attrs['cidr'].  It will also add
@@ -1161,7 +1164,7 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver):
 
         """
         if not _device_exists(bridge):
-            LOG.debug(_('Starting Bridge interface for %s'), interface)
+            LOG.debug(_('Starting Bridge %s'), bridge)
             _execute('brctl', 'addbr', bridge, run_as_root=True)
             _execute('brctl', 'setfd', bridge, 0, run_as_root=True)
             # _execute('brctl setageing %s 10' % bridge, run_as_root=True)
@@ -1173,6 +1176,8 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver):
             _execute('ip', 'link', 'set', bridge, 'up', run_as_root=True)
 
         if interface:
+            msg = _('Adding interface %(interface)s to bridge %(bridge)s')
+            LOG.debug(msg % locals())
             out, err = _execute('brctl', 'addif', bridge, interface,
                                 check_exit_code=False, run_as_root=True)
 
@@ -1207,18 +1212,19 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver):
                 msg = _('Failed to add interface: %s') % err
                 raise exception.NovaException(msg)
 
-        # Don't forward traffic unless we were told to be a gateway
-        ipv4_filter = iptables_manager.ipv4['filter']
-        if gateway:
-            ipv4_filter.add_rule('FORWARD',
-                                 '--in-interface %s -j ACCEPT' % bridge)
-            ipv4_filter.add_rule('FORWARD',
-                                 '--out-interface %s -j ACCEPT' % bridge)
-        else:
-            ipv4_filter.add_rule('FORWARD',
-                                 '--in-interface %s -j DROP' % bridge)
-            ipv4_filter.add_rule('FORWARD',
-                                 '--out-interface %s -j DROP' % bridge)
+        if filtering:
+            # Don't forward traffic unless we were told to be a gateway
+            ipv4_filter = iptables_manager.ipv4['filter']
+            if gateway:
+                ipv4_filter.add_rule('FORWARD',
+                                     '--in-interface %s -j ACCEPT' % bridge)
+                ipv4_filter.add_rule('FORWARD',
+                                     '--out-interface %s -j ACCEPT' % bridge)
+            else:
+                ipv4_filter.add_rule('FORWARD',
+                                     '--in-interface %s -j DROP' % bridge)
+                ipv4_filter.add_rule('FORWARD',
+                                     '--out-interface %s -j DROP' % bridge)
 
 
 # plugs interfaces using Open vSwitch
diff --git a/nova/tests/test_libvirt_config.py b/nova/tests/test_libvirt_config.py
index a00d5b572..c285d46c0 100644
--- a/nova/tests/test_libvirt_config.py
+++ b/nova/tests/test_libvirt_config.py
@@ -483,6 +483,7 @@ class LibvirtConfigGuestInterfaceTest(LibvirtConfigBaseTest):
         obj.source_dev = "br0"
         obj.mac_addr = "DE:AD:BE:EF:CA:FE"
         obj.model = "virtio"
+        obj.target_dev = "tap12345678"
         obj.filtername = "clean-traffic"
         obj.filterparams.append({"key": "IP", "value": "192.168.122.1"})
 
@@ -492,6 +493,7 @@ class LibvirtConfigGuestInterfaceTest(LibvirtConfigBaseTest):
               <mac address="DE:AD:BE:EF:CA:FE"/>
               <model type="virtio"/>
               <source bridge="br0"/>
+              <target dev="tap12345678"/>
               <filterref filter="clean-traffic">
                 <parameter name="IP" value="192.168.122.1"/>
               </filterref>
@@ -503,6 +505,7 @@ class LibvirtConfigGuestInterfaceTest(LibvirtConfigBaseTest):
         obj.source_dev = "br0"
         obj.mac_addr = "DE:AD:BE:EF:CA:FE"
         obj.model = "virtio"
+        obj.target_dev = "tap12345678"
         obj.vporttype = "openvswitch"
         obj.vportparams.append({"key": "instanceid", "value": "foobar"})
 
@@ -512,6 +515,7 @@ class LibvirtConfigGuestInterfaceTest(LibvirtConfigBaseTest):
               <mac address="DE:AD:BE:EF:CA:FE"/>
               <model type="virtio"/>
               <source bridge="br0"/>
+              <target dev="tap12345678"/>
               <virtualport type="openvswitch">
                 <parameters instanceid="foobar"/>
               </virtualport>
@@ -522,6 +526,7 @@ class LibvirtConfigGuestInterfaceTest(LibvirtConfigBaseTest):
         obj.net_type = "direct"
         obj.mac_addr = "DE:AD:BE:EF:CA:FE"
         obj.model = "virtio"
+        obj.target_dev = "tap12345678"
         obj.source_dev = "eth0"
         obj.vporttype = "802.1Qbh"
 
@@ -531,6 +536,7 @@ class LibvirtConfigGuestInterfaceTest(LibvirtConfigBaseTest):
               <mac address="DE:AD:BE:EF:CA:FE"/>
               <model type="virtio"/>
               <source mode="private" dev="eth0"/>
+              <target dev="tap12345678"/>
               <virtualport type="802.1Qbh"/>
             </interface>""")
 
diff --git a/nova/tests/test_libvirt_vif.py b/nova/tests/test_libvirt_vif.py
index bb4344512..d67f47f59 100644
--- a/nova/tests/test_libvirt_vif.py
+++ b/nova/tests/test_libvirt_vif.py
@@ -39,7 +39,8 @@ class LibvirtVifTestCase(test.TestCase):
              'vlan': 99,
              'gateway': '101.168.1.1',
              'broadcast': '101.168.1.255',
-             'dns1': '8.8.8.8'
+             'dns1': '8.8.8.8',
+             'id': 'network-id-xxx-yyy-zzz'
     }
 
     mapping = {
@@ -78,6 +79,48 @@ class LibvirtVifTestCase(test.TestCase):
         conf.add_device(nic)
         return conf.to_xml()
 
+    def test_multiple_nics(self):
+        conf = vconfig.LibvirtConfigGuest()
+        conf.virt_type = "qemu"
+        conf.name = "fake-name"
+        conf.uuid = "fake-uuid"
+        conf.memory = 100 * 1024
+        conf.vcpus = 4
+
+        # Tests multiple nic configuration and that target_dev is
+        # set for each
+        nics = [{'net_type': 'bridge',
+                 'mac_addr': '00:00:00:00:00:0b',
+                 'source_dev': 'b_source_dev',
+                 'target_dev': 'b_target_dev'},
+                {'net_type': 'ethernet',
+                 'mac_addr': '00:00:00:00:00:0e',
+                 'source_dev': 'e_source_dev',
+                 'target_dev': 'e_target_dev'},
+                {'net_type': 'direct',
+                 'mac_addr': '00:00:00:00:00:0d',
+                 'source_dev': 'd_source_dev',
+                 'target_dev': 'd_target_dev'}]
+
+        for nic in nics:
+            nic_conf = vconfig.LibvirtConfigGuestInterface()
+            nic_conf.net_type = nic['net_type']
+            nic_conf.target_dev = nic['target_dev']
+            nic_conf.mac_addr = nic['mac_addr']
+            nic_conf.source_dev = nic['source_dev']
+            conf.add_device(nic_conf)
+
+        xml = conf.to_xml()
+        doc = etree.fromstring(xml)
+        for nic in nics:
+            path = "./devices/interface/[@type='%s']" % nic['net_type']
+            node = doc.find(path)
+            self.assertEqual(nic['net_type'], node.get("type"))
+            self.assertEqual(nic['mac_addr'],
+                             node.find("mac").get("address"))
+            self.assertEqual(nic['target_dev'],
+                             node.find("target").get("dev"))
+
     def test_bridge_driver(self):
         d = vif.LibvirtBridgeDriver()
         xml = self._get_instance_xml(d)
@@ -146,13 +189,13 @@ class LibvirtVifTestCase(test.TestCase):
         ret = doc.findall('./devices/interface')
         self.assertEqual(len(ret), 1)
         node = ret[0]
-        self.assertEqual(node.get("type"), "ethernet")
+        self.assertEqual(node.get("type"), "bridge")
         dev_name = node.find("target").get("dev")
         self.assertTrue(dev_name.startswith("tap"))
         mac = node.find("mac").get("address")
         self.assertEqual(mac, self.mapping['mac'])
-        script = node.find("script").get("path")
-        self.assertEquals(script, "")
+        br_name = node.find("source").get("bridge")
+        self.assertTrue(br_name.startswith("brq"))
 
         d.unplug(None, (self.net, self.mapping))
 
diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py
index 4c3483cb9..58d065d21 100644
--- a/nova/virt/libvirt/config.py
+++ b/nova/virt/libvirt/config.py
@@ -460,13 +460,15 @@ class LibvirtConfigGuestInterface(LibvirtConfigGuestDevice):
         if self.net_type == "ethernet":
             if self.script is not None:
                 dev.append(etree.Element("script", path=self.script))
-            dev.append(etree.Element("target", dev=self.target_dev))
         elif self.net_type == "direct":
             dev.append(etree.Element("source", dev=self.source_dev,
                                      mode="private"))
         else:
             dev.append(etree.Element("source", bridge=self.source_dev))
 
+        if self.target_dev is not None:
+            dev.append(etree.Element("target", dev=self.target_dev))
+
         if self.vporttype is not None:
             vport = etree.Element("virtualport", type=self.vporttype)
             for p in self.vportparams:
diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py
index 91088c3d2..f0228de92 100644
--- a/nova/virt/libvirt/vif.py
+++ b/nova/virt/libvirt/vif.py
@@ -276,6 +276,9 @@ class LibvirtOpenVswitchVirtualPortDriver(vif.VIFDriver):
 class QuantumLinuxBridgeVIFDriver(vif.VIFDriver):
     """VIF driver for Linux Bridge when running Quantum."""
 
+    def get_bridge_name(self, network_id):
+        return ("brq" + network_id)[:LINUX_DEV_LEN]
+
     def get_dev_name(self, iface_id):
         return ("tap" + iface_id)[:LINUX_DEV_LEN]
 
@@ -284,28 +287,20 @@ class QuantumLinuxBridgeVIFDriver(vif.VIFDriver):
         iface_id = mapping['vif_uuid']
         dev = self.get_dev_name(iface_id)
 
-        if CONF.libvirt_type != 'xen':
-            linux_net.QuantumLinuxBridgeInterfaceDriver.create_tap_dev(dev)
+        bridge = self.get_bridge_name(network['id'])
+        linux_net.LinuxBridgeInterfaceDriver.ensure_bridge(bridge, None,
+                                                           filtering=False)
 
         conf = vconfig.LibvirtConfigGuestInterface()
-
-        if CONF.libvirt_use_virtio_for_bridges:
-            conf.model = 'virtio'
-        conf.net_type = "ethernet"
         conf.target_dev = dev
-        if CONF.libvirt_type != 'xen':
-            conf.script = ""
+        conf.net_type = "bridge"
         conf.mac_addr = mapping['mac']
+        conf.source_dev = bridge
+        if CONF.libvirt_use_virtio_for_bridges:
+            conf.model = "virtio"
 
         return conf
 
     def unplug(self, instance, vif):
-        """Unplug the VIF by deleting the port from the bridge."""
-        network, mapping = vif
-        dev = self.get_dev_name(mapping['vif_uuid'])
-        try:
-            if CONF.libvirt_type != 'xen':
-                utils.execute('ip', 'link', 'delete', dev, run_as_root=True)
-        except exception.ProcessExecutionError:
-            LOG.warning(_("Failed while unplugging vif"), instance=instance)
-            raise
+        """No action needed.  Libvirt takes care of cleanup"""
+        pass