Merge "libvirt: Delegate OVS plug to os-vif" into stable/wallaby

16 files changed, 419 insertions, 132 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 8d45e70cd7..440dbcf354 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -7997,11 +7997,12 @@ class ComputeManager(manager.Manager):
                          'but source is either too old, or is set to an '
                          'older upgrade level.', instance=instance)
             if self.network_api.supports_port_binding_extension(ctxt):
-                # Create migrate_data vifs
-                migrate_data.vifs = \
-                    migrate_data_obj.\
-                    VIFMigrateData.create_skeleton_migrate_vifs(
-                        instance.get_network_info())
+                # Create migrate_data vifs if not provided by driver.
+                if 'vifs' not in migrate_data:
+                    migrate_data.vifs = (
+                        migrate_data_obj.
+                        VIFMigrateData.create_skeleton_migrate_vifs(
+                            instance.get_network_info()))
                 # Claim PCI devices for VIFs on destination (if needed)
                 port_id_to_pci = self._claim_pci_for_instance_vifs(
                     ctxt, instance)
diff --git a/nova/network/model.py b/nova/network/model.py
index be29a11663..f46514e13e 100644
--- a/nova/network/model.py
+++ b/nova/network/model.py
@@ -384,7 +384,8 @@ class VIF(Model):
                  details=None, devname=None, ovs_interfaceid=None,
                  qbh_params=None, qbg_params=None, active=False,
                  vnic_type=VNIC_TYPE_NORMAL, profile=None,
-                 preserve_on_delete=False, **kwargs):
+                 preserve_on_delete=False, delegate_create=False,
+                 **kwargs):
         super(VIF, self).__init__()
 
         self['id'] = id
@@ -401,6 +402,7 @@ class VIF(Model):
         self['vnic_type'] = vnic_type
         self['profile'] = profile
         self['preserve_on_delete'] = preserve_on_delete
+        self['delegate_create'] = delegate_create
 
         self._set_meta(kwargs)
 
@@ -408,7 +410,7 @@ class VIF(Model):
         keys = ['id', 'address', 'network', 'vnic_type',
                 'type', 'profile', 'details', 'devname',
                 'ovs_interfaceid', 'qbh_params', 'qbg_params',
-                'active', 'preserve_on_delete']
+                'active', 'preserve_on_delete', 'delegate_create']
         return all(self[k] == other[k] for k in keys)
 
     def __ne__(self, other):
diff --git a/nova/network/neutron.py b/nova/network/neutron.py
index db6b431eb0..6adf637396 100644
--- a/nova/network/neutron.py
+++ b/nova/network/neutron.py
@@ -3066,7 +3066,8 @@ class API(base.Base):
             ovs_interfaceid=ovs_interfaceid,
             devname=devname,
             active=vif_active,
-            preserve_on_delete=preserve_on_delete
+            preserve_on_delete=preserve_on_delete,
+            delegate_create=True,
         )
 
     def _build_network_info_model(self, context, instance, networks=None,
diff --git a/nova/network/os_vif_util.py b/nova/network/os_vif_util.py
index f933887219..e4ce610d56 100644
--- a/nova/network/os_vif_util.py
+++ b/nova/network/os_vif_util.py
@@ -21,6 +21,7 @@ versioned object model os_vif.objects.*
 from os_vif import objects
 from oslo_config import cfg
 from oslo_log import log as logging
+import pbr.version
 
 from nova import exception
 from nova.i18n import _
@@ -347,6 +348,27 @@ def _nova_to_osvif_vif_ovs(vif):
             vif_name=vif_name,
             bridge_name=_get_hybrid_bridge_name(vif))
     else:
+        # NOTE(stephenfin): The 'create_port' attribute was added in os-vif
+        # 1.15.0 and isn't available with the lowest version supported here
+        if (
+            pbr.version.VersionInfo('os-vif').semantic_version() >=
+            pbr.version.SemanticVersion.from_pip_string('1.15.0')
+        ):
+            profile.create_port = vif.get('delegate_create', False)
+        elif vif.get('delegate_create', False):
+            # NOTE(stephenfin): This should never happen, since if the
+            # 'delegate_create' attribute is true then it was set by the
+            # destination compute node as part of the live migration operation:
+            # the same destination compute node that we are currently running
+            # on. We include it for sanity-preservation
+            LOG.warning(
+                'os-vif 1.15.0 or later is required to support '
+                'delegated port creation but you have %s; consider '
+                'updating this package to resolve bugs #1734320 and '
+                '#1815989',
+                pbr.version.VersionInfo('os-vif').version_string(),
+            )
+
         obj = _get_vif_instance(
             vif,
             objects.vif.VIFOpenVSwitch,
diff --git a/nova/objects/migrate_data.py b/nova/objects/migrate_data.py
index a145dfb723..cf0e4bf9a3 100644
--- a/nova/objects/migrate_data.py
+++ b/nova/objects/migrate_data.py
@@ -22,8 +22,8 @@ from nova import exception
 from nova.objects import base as obj_base
 from nova.objects import fields
 
-
 LOG = log.getLogger(__name__)
+OS_VIF_DELEGATION = 'os_vif_delegation'
 
 
 @obj_base.NovaObjectRegistry.register
@@ -60,6 +60,8 @@ class VIFMigrateData(obj_base.NovaObject):
 
     @property
     def vif_details(self):
+        if 'vif_details_json' not in self:
+            return {}
         return jsonutils.loads(self.vif_details_json)
 
     @vif_details.setter
@@ -68,12 +70,27 @@ class VIFMigrateData(obj_base.NovaObject):
 
     @property
     def profile(self):
+        if 'profile_json' not in self:
+            return {}
         return jsonutils.loads(self.profile_json)
 
     @profile.setter
     def profile(self, profile_dict):
         self.profile_json = jsonutils.dumps(profile_dict)
 
+    @property
+    def supports_os_vif_delegation(self):
+        return self.profile.get(OS_VIF_DELEGATION, False)
+
+    # TODO(stephenfin): add a proper delegation field instead of storing this
+    # info in the profile catch-all blob
+    @supports_os_vif_delegation.setter
+    def supports_os_vif_delegation(self, supported):
+        # we can't simply set the attribute using dict notation since the
+        # getter returns a copy of the data, not the data itself
+        self.profile = dict(
+            self.profile or {}, **{OS_VIF_DELEGATION: supported})
+
     def get_dest_vif(self):
         """Get a destination VIF representation of this object.
 
@@ -91,6 +108,7 @@ class VIFMigrateData(obj_base.NovaObject):
         vif['vnic_type'] = self.vnic_type
         vif['profile'] = self.profile
         vif['details'] = self.vif_details
+        vif['delegate_create'] = self.supports_os_vif_delegation
         return vif
 
     @classmethod
diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py
index 66fe059400..94b6fb228c 100644
--- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py
+++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py
@@ -577,16 +577,15 @@ class SRIOVServersTest(_PCIServersTestBase):
         self.start_compute(pci_info=pci_info)
 
         # create the SR-IOV port
-        self.neutron.create_port({'port': self.neutron.network_4_port_1})
+        port = self.neutron.create_port(
+            {'port': self.neutron.network_4_port_1})
 
-        # create a server using the VF and multiple networks
-        extra_spec = {'pci_passthrough:alias': f'{self.VFS_ALIAS_NAME}:1'}
-        flavor_id = self._create_flavor(extra_spec=extra_spec)
+        flavor_id = self._create_flavor()
         server = self._create_server(
             flavor_id=flavor_id,
             networks=[
                 {'uuid': base.LibvirtNeutronFixture.network_1['id']},
-                {'port': base.LibvirtNeutronFixture.network_4_port_1['id']},
+                {'port': port['port']['id']},
             ],
         )
 
@@ -600,13 +599,16 @@ class SRIOVServersTest(_PCIServersTestBase):
             base.LibvirtNeutronFixture.network_1_port_2['mac_address'],
             diagnostics['nic_details'][0]['mac_address'],
         )
-        self.assertIsNotNone(diagnostics['nic_details'][0]['tx_packets'])
+
+        for key in ('rx_packets', 'tx_packets'):
+            self.assertIn(key, diagnostics['nic_details'][0])
 
         self.assertEqual(
             base.LibvirtNeutronFixture.network_4_port_1['mac_address'],
             diagnostics['nic_details'][1]['mac_address'],
         )
-        self.assertIsNone(diagnostics['nic_details'][1]['tx_packets'])
+        for key in ('rx_packets', 'tx_packets'):
+            self.assertIn(key, diagnostics['nic_details'][1])
 
     def test_create_server_after_change_in_nonsriov_pf_to_sriov_pf(self):
         # Starts a compute with PF not configured with SRIOV capabilities
diff --git a/nova/tests/unit/fake_network.py b/nova/tests/unit/fake_network.py
index 9725b52cac..63d24f9e6f 100644
--- a/nova/tests/unit/fake_network.py
+++ b/nova/tests/unit/fake_network.py
@@ -120,7 +120,7 @@ def fake_get_instance_nw_info(test, num_networks=1):
             qbg_params=None,
             active=False,
             vnic_type='normal',
-            profile=None,
+            profile={},
             preserve_on_delete=False,
             meta={'rxtx_cap': 30},
         )
diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py
index a85a19e285..a4e6f23ccf 100644
--- a/nova/tests/unit/network/test_neutron.py
+++ b/nova/tests/unit/network/test_neutron.py
@@ -3271,6 +3271,7 @@ class TestAPI(TestAPIBase):
                 requested_ports[index].get(
                     constants.BINDING_PROFILE) or {},
                 nw_info.get('profile'))
+            self.assertTrue(nw_info.get('delegate_create'))
             index += 1
 
         self.assertFalse(nw_infos[0]['active'])
diff --git a/nova/tests/unit/network/test_os_vif_util.py b/nova/tests/unit/network/test_os_vif_util.py
index 7fb2573ce3..e15e4eb92a 100644
--- a/nova/tests/unit/network/test_os_vif_util.py
+++ b/nova/tests/unit/network/test_os_vif_util.py
@@ -458,7 +458,8 @@ class OSVIFUtilTestCase(test.NoDBTestCase):
                 subnets=[]),
             details={
                 model.VIF_DETAILS_PORT_FILTER: True,
-            }
+            },
+            delegate_create=False,
         )
 
         actual = os_vif_util.nova_to_osvif_vif(vif)
@@ -471,7 +472,8 @@ class OSVIFUtilTestCase(test.NoDBTestCase):
             plugin="ovs",
             port_profile=osv_objects.vif.VIFPortProfileOpenVSwitch(
                 interface_id="dc065497-3c8d-4f44-8fb4-e1d33c16a536",
-                datapath_type=None),
+                datapath_type=None,
+                create_port=False),
             preserve_on_delete=False,
             vif_name="nicdc065497-3c",
             network=osv_objects.network.Network(
@@ -588,6 +590,7 @@ class OSVIFUtilTestCase(test.NoDBTestCase):
                 model.VIF_DETAILS_OVS_DATAPATH_TYPE:
                     model.VIF_DETAILS_OVS_DATAPATH_SYSTEM
             },
+            delegate_create=True,
         )
 
         actual = os_vif_util.nova_to_osvif_vif(vif)
@@ -600,7 +603,8 @@ class OSVIFUtilTestCase(test.NoDBTestCase):
             plugin="ovs",
             port_profile=osv_objects.vif.VIFPortProfileOpenVSwitch(
                 interface_id="dc065497-3c8d-4f44-8fb4-e1d33c16a536",
-                datapath_type=model.VIF_DETAILS_OVS_DATAPATH_SYSTEM),
+                datapath_type=model.VIF_DETAILS_OVS_DATAPATH_SYSTEM,
+                create_port=True),
             preserve_on_delete=False,
             vif_name="nicdc065497-3c",
             network=osv_objects.network.Network(
diff --git a/nova/tests/unit/objects/test_migrate_data.py b/nova/tests/unit/objects/test_migrate_data.py
index 5c84b2c39a..7f587c6906 100644
--- a/nova/tests/unit/objects/test_migrate_data.py
+++ b/nova/tests/unit/objects/test_migrate_data.py
@@ -316,3 +316,24 @@ class TestVIFMigrateData(test.NoDBTestCase):
         self.assertEqual(len(vifs), len(mig_vifs))
         self.assertEqual([vif['id'] for vif in vifs],
                          [mig_vif.port_id for mig_vif in mig_vifs])
+
+    def test_supports_os_vif_delegation(self):
+        # first try setting on a object without 'profile' defined
+        migrate_vif = objects.VIFMigrateData(
+            port_id=uuids.port_id, vnic_type=network_model.VNIC_TYPE_NORMAL,
+            vif_type=network_model.VIF_TYPE_OVS, vif_details={},
+            host='fake-dest-host')
+        migrate_vif.supports_os_vif_delegation = True
+        self.assertTrue(migrate_vif.supports_os_vif_delegation)
+        self.assertEqual(migrate_vif.profile, {'os_vif_delegation': True})
+
+        # now do the same but with profile defined
+        migrate_vif = objects.VIFMigrateData(
+            port_id=uuids.port_id, vnic_type=network_model.VNIC_TYPE_NORMAL,
+            vif_type=network_model.VIF_TYPE_OVS, vif_details={},
+            host='fake-dest-host', profile={'interface_name': 'eth0'})
+        migrate_vif.supports_os_vif_delegation = True
+        self.assertTrue(migrate_vif.supports_os_vif_delegation)
+        self.assertEqual(
+            migrate_vif.profile,
+            {'os_vif_delegation': True, 'interface_name': 'eth0'})
diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py
index 5164192f3d..cb19fe82e3 100644
--- a/nova/tests/unit/virt/libvirt/fakelibvirt.py
+++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py
@@ -1264,6 +1264,16 @@ class Domain(object):
             nic_info['_attached'] = True
             self._def['devices']['nics'] += [nic_info]
             result = True
+        else:
+            # FIXME(sean-k-mooney): We don't currently handle attaching
+            # or detaching hostdevs but we have tests that assume we do so
+            # this is an error not an exception. This affects PCI passthough,
+            # vGPUs and PF neutron ports.
+            LOG.error(
+                "Trying to attach an unsupported device type."
+                "The fakelibvirt implementation is incomplete "
+                "and should be extended to support %s: %s",
+                xml, self._def['devices'])
 
         return result
 
@@ -1278,17 +1288,45 @@ class Domain(object):
         self.attachDevice(xml)
 
     def detachDevice(self, xml):
-        # TODO(gibi): this should handle nics similarly to attachDevice()
-        disk_info = _parse_disk_info(etree.fromstring(xml))
-        attached_disk_info = None
-        for attached_disk in self._def['devices']['disks']:
-            if attached_disk['target_dev'] == disk_info.get('target_dev'):
-                attached_disk_info = attached_disk
-                break
-
-        if attached_disk_info:
-            self._def['devices']['disks'].remove(attached_disk_info)
-        return attached_disk_info is not None
+        # detachDevice is a common function used for all devices types
+        # so we need to handle each separately
+        if xml.startswith("<disk"):
+            disk_info = _parse_disk_info(etree.fromstring(xml))
+            attached_disk_info = None
+            for attached_disk in self._def['devices']['disks']:
+                if attached_disk['target_dev'] == disk_info.get('target_dev'):
+                    attached_disk_info = attached_disk
+                    break
+
+            if attached_disk_info:
+                self._def['devices']['disks'].remove(attached_disk_info)
+
+            return attached_disk_info is not None
+
+        if xml.startswith("<interface"):
+            nic_info = _parse_nic_info(etree.fromstring(xml))
+            attached_nic_info = None
+            for attached_nic in self._def['devices']['nics']:
+                if attached_nic['mac'] == nic_info['mac']:
+                    attached_nic_info = attached_nic
+                    break
+
+            if attached_nic_info:
+                self._def['devices']['nics'].remove(attached_nic_info)
+
+            return attached_nic_info is not None
+
+        # FIXME(sean-k-mooney): We don't currently handle attaching or
+        # detaching hostdevs but we have tests that assume we do so this is
+        # an error not an exception. This affects PCI passthough, vGPUs and
+        # PF neutron ports
+        LOG.error(
+            "Trying to detach an unsupported device type."
+            "The fakelibvirt implementation is incomplete "
+            "and should be extended to support %s: %s",
+            xml, self._def['devices'])
+
+        return False
 
     def detachDeviceFlags(self, xml, flags):
         self.detachDevice(xml)
@@ -1310,34 +1348,48 @@ class Domain(object):
       <target dev='%(target_dev)s' bus='%(target_bus)s'/>
       <address type='drive' controller='0' bus='0' unit='0'/>
     </disk>''' % dict(source_attr=source_attr, **disk)
-
         nics = ''
-        for nic in self._def['devices']['nics']:
+        for func, nic in enumerate(self._def['devices']['nics']):
+            if func > 7:
+                # this should never be raised but is just present to highlight
+                # the limitations of the current fake when writing new tests.
+                # if you see this raised when add a new test you will need
+                # to extend this fake to use both functions and slots.
+                # the pci function is limited to 3 bits or 0-7.
+                raise RuntimeError(
+                    'Test attempts to add more than 8 PCI devices. This is '
+                    'not supported by the fake libvirt implementation.')
+            nic['func'] = func
+            # this branch covers most interface types with a source
+            # such as linux bridge interfaces.
             if 'source' in nic:
-                if nic['type'] == 'hostdev':
-                    nics += '''<interface type='%(type)s'>
-                      <mac address='%(mac)s'/>
-                      <source>
-                        <address type='pci' domain='0x0000' bus='0x81' slot='0x00' function='0x01'/>
-                      </source>
-                      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
-                    </interface>''' % nic  # noqa: E501
-                elif nic['type'] == 'vdpa':
-                    # TODO(stephenfin): In real life, this would actually have
-                    # an '<address>' element, but that requires information
-                    # about the host that we're not passing through yet
-                    nics += '''<interface type='%(type)s'>
-                      <mac address='%(mac)s'/>
-                      <source dev='%(source)s'/>
-                      <model type='virtio'/>
-                    </interface>'''
-                else:
-                    nics += '''<interface type='%(type)s'>
-                      <mac address='%(mac)s'/>
-                      <source %(type)s='%(source)s'/>
-                      <target dev='tap274487d1-60'/>
-                      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
-                    </interface>''' % nic  # noqa: E501
+                nics += '''<interface type='%(type)s'>
+          <mac address='%(mac)s'/>
+          <source %(type)s='%(source)s'/>
+          <target dev='tap274487d1-6%(func)s'/>
+          <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
+                   function='0x%(func)s'/>
+        </interface>''' % nic
+            elif nic['type'] in ('ethernet',):
+                # this branch covers kernel ovs interfaces
+                nics += '''<interface type='%(type)s'>
+          <mac address='%(mac)s'/>
+          <target dev='tap274487d1-6%(func)s'/>
+          <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
+                   function='0x%(func)s'/>
+        </interface>''' % nic
+            else:
+                # This branch covers the macvtap vnic-type.
+                # This is incomplete as the source dev should be unique
+                # and map to the VF netdev name but due to the mocking in
+                # the fixture we hard code it.
+                nics += '''<interface type='%(type)s'>
+          <mac address='%(mac)s'/>
+          <source dev='fake_pf_interface_name' mode='passthrough'>
+              <address type='pci' domain='0x0000' bus='0x81' slot='0x00'
+                   function='0x%(func)s'/>
+          </source>
+        </interface>''' % nic
 
         hostdevs = ''
         for hostdev in self._def['devices']['hostdevs']:
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 172f24f1ec..a4c61d6b1b 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -57,6 +57,7 @@ from oslo_utils import strutils
 from oslo_utils import units
 from oslo_utils import uuidutils
 from oslo_utils import versionutils
+import pbr.version
 
 from nova.api.metadata import base as instance_metadata
 from nova.compute import manager
@@ -711,6 +712,7 @@ def _create_test_instance():
         'trusted_certs': None,
         'resources': None,
         'migration_context': None,
+        'info_cache': None,
     }
 
 
@@ -754,6 +756,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.addCleanup(_p.stop)
 
         self.test_instance = _create_test_instance()
+        network_info = objects.InstanceInfoCache(
+            network_info=_fake_network_info(self))
+        self.test_instance['info_cache'] = network_info
         self.test_image_meta = {
             "disk_format": "raw",
         }
@@ -10640,11 +10645,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
             self.assertEqual(drvr._uri(), testuri)
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
         '_create_shared_storage_test_file')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU')
     def test_check_can_live_migrate_dest_all_pass_with_block_migration(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         instance_ref = objects.Instance(**self.test_instance)
         instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -10675,11 +10684,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           'serial_listen_addr': None},
                          return_value.obj_to_primitive()['nova_object.data'])
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
         '_create_shared_storage_test_file')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU')
     def test_check_can_live_migrate_dest_all_pass_with_over_commit(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         instance_ref = objects.Instance(**self.test_instance)
         instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -10711,11 +10724,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           'serial_listen_addr': None},
                          return_value.obj_to_primitive()['nova_object.data'])
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
         '_create_shared_storage_test_file')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU')
     def test_check_can_live_migrate_dest_all_pass_no_block_migration(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         instance_ref = objects.Instance(**self.test_instance)
         instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -10744,12 +10761,16 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           'serial_listen_addr': None},
                          return_value.obj_to_primitive()['nova_object.data'])
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_create_shared_storage_test_file',
                        return_value='fake')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU')
     def test_check_can_live_migrate_dest_fills_listen_addrs(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         # Tests that check_can_live_migrate_destination returns the listen
         # addresses required by check_can_live_migrate_source.
         self.flags(server_listen='192.0.2.12', group='vnc')
@@ -10772,13 +10793,17 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.assertEqual('203.0.113.56',
                          str(result.serial_listen_addr))
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_create_shared_storage_test_file',
                        return_value='fake')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU',
                        return_value=1)
     def test_check_can_live_migrate_dest_ensure_serial_adds_not_set(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         self.flags(proxyclient_address='127.0.0.1', group='serial_console')
         self.flags(enabled=False, group='serial_console')
         instance_ref = objects.Instance(**self.test_instance)
@@ -10789,12 +10814,16 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             self.context, instance_ref, compute_info, compute_info)
         self.assertIsNone(result.serial_listen_addr)
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_create_shared_storage_test_file',
                        return_value='fake')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_compare_cpu')
     def test_check_can_live_migrate_guest_cpu_none_model(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         # Tests that when instance.vcpu_model.model is None, the host cpu
         # model is used for live migration.
         instance_ref = objects.Instance(**self.test_instance)
@@ -10818,12 +10847,16 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           'serial_listen_addr': None},
                          result.obj_to_primitive()['nova_object.data'])
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_create_shared_storage_test_file',
                        return_value='fake')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_compare_cpu')
     def test_check_can_live_migrate_dest_numa_lm(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         instance_ref = objects.Instance(**self.test_instance)
         instance_ref.numa_topology = objects.InstanceNUMATopology(
             cells=[objects.InstanceNUMACell()])
@@ -10833,12 +10866,16 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             self.context, instance_ref, compute_info, compute_info)
         self.assertTrue(result.dst_supports_numa_live_migration)
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_create_shared_storage_test_file',
                        return_value='fake')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_compare_cpu')
     def test_check_can_live_migrate_dest_numa_lm_no_instance_numa(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         instance_ref = objects.Instance(**self.test_instance)
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
         compute_info = {'cpu_info': 'asdf', 'disk_available_least': 1}
@@ -10846,11 +10883,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             self.context, instance_ref, compute_info, compute_info)
         self.assertNotIn('dst_supports_numa_live_migration', result)
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
         '_create_shared_storage_test_file')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU')
     def test_check_can_live_migrate_dest_no_instance_cpu_info(
-            self, mock_cpu, mock_test_file):
+        self, mock_cpu, mock_test_file,
+    ):
         instance_ref = objects.Instance(**self.test_instance)
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
         compute_info = {'cpu_info': jsonutils.dumps({
@@ -10883,12 +10924,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           'serial_listen_addr': None},
                          return_value.obj_to_primitive()['nova_object.data'])
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=False))
     @mock.patch.object(libvirt_driver.LibvirtDriver,
         '_create_shared_storage_test_file')
     @mock.patch.object(fakelibvirt.Connection, 'compareCPU')
     def test_check_can_live_migrate_dest_file_backed(
-            self, mock_cpu, mock_test_file):
-
+        self, mock_cpu, mock_test_file,
+    ):
         self.flags(file_backed_memory=1024, group='libvirt')
 
         instance_ref = objects.Instance(**self.test_instance)
@@ -10925,6 +10969,34 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           self.context, instance_ref,
                           compute_info, compute_info, False)
 
+    @mock.patch(
+        'nova.network.neutron.API.supports_port_binding_extension',
+        new=mock.Mock(return_value=True))
+    @mock.patch.object(
+        libvirt_driver.LibvirtDriver,
+        '_create_shared_storage_test_file',
+        return_value='fake')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_compare_cpu')
+    def test_check_can_live_migrate_dest_create_dummy_vif(
+        self, mock_cpu, mock_test_file,
+    ):
+        instance_ref = objects.Instance(**self.test_instance)
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        compute_info = {'cpu_info': 'asdf', 'disk_available_least': 1}
+        result = drvr.check_can_live_migrate_destination(
+            self.context, instance_ref, compute_info, compute_info)
+        self.assertIn('vifs', result)
+        self.assertIsInstance(result.vifs, list)
+
+        supports_os_vif_delegation = (
+            pbr.version.VersionInfo('os-vif').semantic_version() >=
+            pbr.version.SemanticVersion.from_pip_string('1.15.0')
+        )
+        for vif in result.vifs:
+            self.assertEqual(
+                supports_os_vif_delegation,
+                vif.supports_os_vif_delegation)
+
     @mock.patch.object(host.Host, 'compare_cpu')
     @mock.patch.object(nova.virt.libvirt, 'config')
     def test_compare_cpu_compatible_host_cpu(self, mock_vconfig, mock_compare):
diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py
index 5e638bdd76..0fe8792a13 100644
--- a/nova/tests/unit/virt/libvirt/test_vif.py
+++ b/nova/tests/unit/virt/libvirt/test_vif.py
@@ -22,6 +22,7 @@ from os_vif.objects import fields as osv_fields
 from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_utils.fixture import uuidsentinel as uuids
+import pbr.version
 
 from nova import exception
 from nova.network import model as network_model
@@ -149,7 +150,8 @@ class LibvirtVifTestCase(test.NoDBTestCase):
         type=network_model.VIF_TYPE_OVS,
         details={'port_filter': True},
         devname='tap-xxx-yyy-zzz',
-        ovs_interfaceid=uuids.ovs)
+        ovs_interfaceid=uuids.ovs,
+        delegate_create=True)
 
     vif_ovs_legacy = network_model.VIF(id=uuids.vif,
         address='ca:fe:de:ad:be:ef',
@@ -410,7 +412,8 @@ class LibvirtVifTestCase(test.NoDBTestCase):
 
         self.os_vif_ovs_prof = osv_objects.vif.VIFPortProfileOpenVSwitch(
             interface_id="07bd6cea-fb37-4594-b769-90fc51854ee9",
-            profile_id="fishfood")
+            profile_id="fishfood",
+            create_port=True)
 
         self.os_vif_repr_prof = osv_objects.vif.VIFPortProfileOVSRepresentor(
             interface_id="07bd6cea-fb37-4594-b769-90fc51854ee9",
@@ -1014,36 +1017,22 @@ class LibvirtVifTestCase(test.NoDBTestCase):
                        self.vif_iovisor['network']['id'],
                        self.instance.project_id)])
 
-    def _check_ovs_virtualport_driver(self, d, vif, want_iface_id):
-        xml = self._get_instance_xml(d, vif)
-        node = self._get_node(xml)
-        self._assertTypeAndMacEquals(node, "bridge", "source", "bridge",
-                                     vif, "br0")
-        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, want_iface_id)
-                iface_id_found = True
-
-        self.assertTrue(iface_id_found)
-
-    def test_generic_ovs_virtualport_driver(self):
-        d = vif.LibvirtGenericVIFDriver()
-        want_iface_id = self.vif_ovs['ovs_interfaceid']
-        self._check_ovs_virtualport_driver(d,
-                                           self.vif_ovs,
-                                           want_iface_id)
-
     def test_direct_plug_with_port_filter_cap_no_nova_firewall(self):
         d = vif.LibvirtGenericVIFDriver()
         br_want = self.vif_midonet['devname']
         xml = self._get_instance_xml(d, self.vif_ovs_filter_cap)
         node = self._get_node(xml)
-        self._assertTypeAndMacEquals(node, "bridge", "target", "dev",
-                                     self.vif_ovs_filter_cap, br_want)
+        # NOTE(stephenfin): We delegate creation of the port to os-vif if, and
+        # only if, the version present is suitably new
+        if (
+            pbr.version.VersionInfo('os-vif').semantic_version() >=
+            pbr.version.SemanticVersion.from_pip_string('1.15.0')
+        ):
+            self._assertTypeAndMacEquals(node, "ethernet", "target", "dev",
+                                         self.vif_ovs_filter_cap, br_want)
+        else:
+            self._assertTypeAndMacEquals(node, "bridge", "target", "dev",
+                                         self.vif_ovs_filter_cap, br_want)
 
     def test_ib_hostdev_driver(self):
         d = vif.LibvirtGenericVIFDriver()
@@ -1522,8 +1511,8 @@ class LibvirtVifTestCase(test.NoDBTestCase):
         d = vif.LibvirtGenericVIFDriver()
         xml = self._get_instance_xml(d, vif_model)
         node = self._get_node(xml)
-
-        self._assertXmlEqual(expected_xml, node)
+        node_xml = etree.tostring(node).decode()
+        self._assertXmlEqual(expected_xml, node_xml)
 
     def test_config_os_vif_bridge(self):
         os_vif_type = self.os_vif_bridge
@@ -1582,22 +1571,40 @@ class LibvirtVifTestCase(test.NoDBTestCase):
         os_vif_type = self.os_vif_agilio_ovs
         vif_type = self.vif_agilio_ovs
 
-        expected_xml = """
-            <interface type="bridge">
-             <mac address="22:52:25:62:e2:aa"/>
-             <model type="virtio"/>
-             <source bridge="br0"/>
-             <mtu size="9000"/>
-             <target dev="nicdc065497-3c"/>
-             <virtualport type="openvswitch">
-              <parameters
-              interfaceid="07bd6cea-fb37-4594-b769-90fc51854ee9"/>
-             </virtualport>
-              <bandwidth>
-               <inbound average="100" peak="200" burst="300"/>
-               <outbound average="10" peak="20" burst="30"/>
-              </bandwidth>
-            </interface>"""
+        # NOTE(stephenfin): We delegate creation of the port to os-vif if, and
+        # only if, the version present is suitably new
+        if (
+            pbr.version.VersionInfo('os-vif').semantic_version() >=
+            pbr.version.SemanticVersion.from_pip_string('1.15.0')
+        ):
+            expected_xml = """
+                <interface type="ethernet">
+                 <mac address="22:52:25:62:e2:aa"/>
+                 <model type="virtio"/>
+                 <mtu size="9000"/>
+                 <target dev="nicdc065497-3c"/>
+                 <bandwidth>
+                  <inbound average="100" peak="200" burst="300"/>
+                  <outbound average="10" peak="20" burst="30"/>
+                 </bandwidth>
+                </interface>"""
+        else:
+            expected_xml = """
+                <interface type="bridge">
+                 <mac address="22:52:25:62:e2:aa"/>
+                 <model type="virtio"/>
+                 <source bridge="br0"/>
+                 <mtu size="9000"/>
+                 <target dev="nicdc065497-3c"/>
+                 <virtualport type="openvswitch">
+                  <parameters
+                  interfaceid="07bd6cea-fb37-4594-b769-90fc51854ee9"/>
+                 </virtualport>
+                  <bandwidth>
+                   <inbound average="100" peak="200" burst="300"/>
+                   <outbound average="10" peak="20" burst="30"/>
+                  </bandwidth>
+                </interface>"""
 
         self._test_config_os_vif(os_vif_type, vif_type, expected_xml)
 
@@ -1635,21 +1642,39 @@ class LibvirtVifTestCase(test.NoDBTestCase):
         os_vif_type = self.os_vif_ovs
         vif_type = self.vif_ovs
 
-        expected_xml = """
-            <interface type="bridge">
-             <mac address="22:52:25:62:e2:aa"/>
-             <model type="virtio"/>
-             <source bridge="br0"/>
-             <mtu size="9000"/>
-             <target dev="nicdc065497-3c"/>
-             <virtualport type="openvswitch">
-              <parameters interfaceid="07bd6cea-fb37-4594-b769-90fc51854ee9"/>
-             </virtualport>
-             <bandwidth>
-              <inbound average="100" peak="200" burst="300"/>
-              <outbound average="10" peak="20" burst="30"/>
-             </bandwidth>
-            </interface>"""
+        # NOTE(stephenfin): We delegate creation of the port to os-vif if, and
+        # only if, the version present is suitably new
+        if (
+            pbr.version.VersionInfo('os-vif').semantic_version() >=
+            pbr.version.SemanticVersion.from_pip_string('1.15.0')
+        ):
+            expected_xml = """
+                <interface type="ethernet">
+                 <mac address="22:52:25:62:e2:aa"/>
+                 <model type="virtio"/>
+                 <mtu size="9000"/>
+                 <target dev="nicdc065497-3c"/>
+                 <bandwidth>
+                  <inbound average="100" peak="200" burst="300"/>
+                  <outbound average="10" peak="20" burst="30"/>
+                 </bandwidth>
+                </interface>"""
+        else:
+            expected_xml = """
+                <interface type="bridge">
+                 <mac address="22:52:25:62:e2:aa"/>
+                 <model type="virtio"/>
+                 <source bridge="br0"/>
+                 <mtu size="9000"/>
+                 <target dev="nicdc065497-3c"/>
+                 <virtualport type="openvswitch">
+                  <parameters interfaceid="07bd6cea-fb37-4594-b769-90fc51854ee9"/>
+                 </virtualport>
+                 <bandwidth>
+                  <inbound average="100" peak="200" burst="300"/>
+                  <outbound average="10" peak="20" burst="30"/>
+                 </bandwidth>
+                </interface>"""  # noqa: E501
 
         self._test_config_os_vif(os_vif_type, vif_type, expected_xml)
 
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index d027661108..30c2fff135 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -72,6 +72,7 @@ from oslo_utils import strutils
 from oslo_utils import timeutils
 from oslo_utils import units
 from oslo_utils import uuidutils
+import pbr.version
 
 from nova.api.metadata import base as instance_metadata
 from nova.api.metadata import password
@@ -91,9 +92,11 @@ from nova import exception
 from nova.i18n import _
 from nova.image import glance
 from nova.network import model as network_model
+from nova.network import neutron
 from nova import objects
 from nova.objects import diagnostics as diagnostics_obj
 from nova.objects import fields
+from nova.objects import migrate_data as migrate_data_obj
 from nova.pci import manager as pci_manager
 from nova.pci import utils as pci_utils
 import nova.privsep.libvirt
@@ -456,6 +459,7 @@ class LibvirtDriver(driver.ComputeDriver):
 
         self._volume_api = cinder.API()
         self._image_api = glance.API()
+        self._network_api = neutron.API()
 
         # The default choice for the sysinfo_serial config option is "unique"
         # which does not have a special function since the value is just the
@@ -9093,6 +9097,35 @@ class LibvirtDriver(driver.ComputeDriver):
         if instance.numa_topology:
             data.dst_supports_numa_live_migration = True
 
+        # NOTE(sean-k-mooney): The migrate_data vifs field is used to signal
+        # that we are using the multiple port binding workflow so we can only
+        # populate it if we are using multiple port bindings.
+        # TODO(stephenfin): Remove once we can do this unconditionally in X or
+        # later
+        if self._network_api.supports_port_binding_extension(context):
+
+            # NOTE(stephenfin): This functionality was only added in os-vif
+            # 1.15.0, but our lower constraint is 1.14.0. Only enable this if
+            # os-vif is new enough
+            supports_os_vif_delegation = (
+                pbr.version.VersionInfo('os-vif').semantic_version() >=
+                pbr.version.SemanticVersion.from_pip_string('1.15.0')
+            )
+            if not supports_os_vif_delegation:
+                LOG.warning(
+                    'os-vif 1.15.0 or later is required to support '
+                    'delegated port creation but you have %s; consider '
+                    'updating this package to resolve bugs #1734320 and '
+                    '#1815989',
+                    pbr.version.VersionInfo('os-vif').version_string(),
+                )
+
+            data.vifs = (
+                migrate_data_obj.VIFMigrateData.create_skeleton_migrate_vifs(
+                    instance.get_network_info()))
+            for vif in data.vifs:
+                vif.supports_os_vif_delegation = supports_os_vif_delegation
+
         return data
 
     def post_claim_migrate_data(self, context, instance, migrate_data, claim):
diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py
index a18a0e5c23..dab788d662 100644
--- a/nova/virt/libvirt/vif.py
+++ b/nova/virt/libvirt/vif.py
@@ -450,10 +450,15 @@ class LibvirtGenericVIFDriver(object):
         conf.target_dev = vif.vif_name
 
     def _set_config_VIFOpenVSwitch(self, instance, vif, conf):
-        conf.net_type = "bridge"
-        conf.source_dev = vif.bridge_name
-        conf.target_dev = vif.vif_name
-        self._set_config_VIFPortProfile(instance, vif, conf)
+        # if delegating creation to os-vif, create an ethernet-type VIF and let
+        # os-vif do the actual wiring up
+        if 'create_port' in vif.port_profile and vif.port_profile.create_port:
+            self._set_config_VIFGeneric(instance, vif, conf)
+        else:
+            conf.net_type = "bridge"
+            conf.source_dev = vif.bridge_name
+            conf.target_dev = vif.vif_name
+            self._set_config_VIFPortProfile(instance, vif, conf)
 
     def _set_config_VIFVHostUser(self, instance, vif, conf):
         # TODO(sahid): We should never configure a driver backend for
diff --git a/releasenotes/notes/libvirt-delegate-ovs-plugging-to-os-vif-6adc0398a0e0df58.yaml b/releasenotes/notes/libvirt-delegate-ovs-plugging-to-os-vif-6adc0398a0e0df58.yaml
new file mode 100644
index 0000000000..56cb3e1987
--- /dev/null
+++ b/releasenotes/notes/libvirt-delegate-ovs-plugging-to-os-vif-6adc0398a0e0df58.yaml
@@ -0,0 +1,28 @@
+---
+security:
+  - |
+    In this release OVS port creation has been delegated to os-vif when the
+    ``noop`` or ``openvswitch`` security group firewall drivers are
+    enabled in Neutron. Those options, and others that disable the
+    ``hybrid_plug`` mechanism, will now use os-vif instead of libvirt to plug
+    VIFs into the bridge.  By delegating port plugging to os-vif we can use the
+    ``isolate_vif`` config option to ensure VIFs are plugged securely preventing
+    guests from accessing other tenants' networks before the neutron ovs agent
+    can wire up the port. See `bug #1734320`_ for details.
+    Note that OVN, ODL and other SDN solutions also use
+    ``hybrid_plug=false`` but they are not known to be affected by the security
+    issue caused by the previous behavior. As such the ``isolate_vif``
+    os-vif config option is only used when deploying with ml2/ovs.
+fixes:
+  - |
+    In this release we delegate port plugging to os-vif for all OVS interface
+    types. This allows os-vif to create the OVS port before libvirt creates
+    a tap device during a live migration therefore preventing the loss of
+    the MAC learning frames generated by QEMU. This resolves a long-standing
+    race condition between Libvirt creating the OVS port, Neutron wiring up
+    the OVS port and QEMU generating RARP packets to populate the vswitch
+    MAC learning table. As a result this reduces the interval during a live
+    migration where packets can be lost. See `bug #1815989`_ for details.
+
+    .. _`bug #1734320`: https://bugs.launchpad.net/neutron/+bug/1734320
+    .. _`bug #1815989`: https://bugs.launchpad.net/neutron/+bug/1815989