Merge "Set migrate_data.vifs only when using multiple port bindings"

Author: Zuul

nova/compute/manager.py

@@ -7720,15 +7720,18 @@ def check_can_live_migrate_destination(self, ctxt, instance,
                 LOG.info('Destination was ready for NUMA live migration, '
                          'but source is either too old, or is set to an '
                          'older upgrade level.', instance=instance)
-            # Create migrate_data vifs
-            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)
-            # Update migrate VIFs with the newly claimed PCI devices
-            self._update_migrate_vifs_profile_with_pci(migrate_data.vifs,
-                                                       port_id_to_pci)
+            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())
+                # Claim PCI devices for VIFs on destination (if needed)
+                port_id_to_pci = self._claim_pci_for_instance_vifs(
+                    ctxt, instance)
+                # Update migrate VIFs with the newly claimed PCI devices
+                self._update_migrate_vifs_profile_with_pci(
+                    migrate_data.vifs, port_id_to_pci)
         finally:
             self.driver.cleanup_live_migration_destination_check(ctxt,
                     dest_check_data)
@@ -7896,8 +7899,12 @@ def pre_live_migration(self, context, instance, block_migration, disk,
             # determine if it should wait for a 'network-vif-plugged' event
             # from neutron before starting the actual guest transfer in the
             # hypervisor
+            using_multiple_port_bindings = (
+                'vifs' in migrate_data and migrate_data.vifs)
             migrate_data.wait_for_vif_plugged = (
-                CONF.compute.live_migration_wait_for_vif_plug)
+                CONF.compute.live_migration_wait_for_vif_plug and
+                using_multiple_port_bindings
+            )
 
             # NOTE(tr3buchet): setup networks on destination host
             self.network_api.setup_networks_on_host(context, instance,

nova/tests/functional/regressions/test_bug_1888395.py

@@ -12,6 +12,9 @@
 
 import fixtures
 
+from lxml import etree
+from urllib import parse as urlparse
+
 from nova import context
 from nova.network import constants as neutron_constants
 from nova.network import neutron
@@ -67,6 +70,40 @@ def setUp(self):
                 kB_mem=10740000)})
 
         self.ctxt = context.get_admin_context()
+        # TODO(sean-k-mooney): remove this when it is part of ServersTestBase
+        self.useFixture(fixtures.MonkeyPatch(
+            'nova.tests.unit.virt.libvirt.fakelibvirt.Domain.migrateToURI3',
+            self._migrate_stub))
+
+    def _migrate_stub(self, domain, destination, params, flags):
+        """Stub out migrateToURI3."""
+
+        src_hostname = domain._connection.hostname
+        dst_hostname = urlparse.urlparse(destination).netloc
+
+        # In a real live migration, libvirt and QEMU on the source and
+        # destination talk it out, resulting in the instance starting to exist
+        # on the destination. Fakelibvirt cannot do that, so we have to
+        # manually create the "incoming" instance on the destination
+        # fakelibvirt.
+        dst = self.computes[dst_hostname]
+        dst.driver._host.get_connection().createXML(
+            params['destination_xml'],
+            'fake-createXML-doesnt-care-about-flags')
+
+        src = self.computes[src_hostname]
+        conn = src.driver._host.get_connection()
+
+        # because migrateToURI3 is spawned in a background thread, this method
+        # does not block the upper nova layers. Because we don't want nova to
+        # think the live migration has finished until this method is done, the
+        # last thing we do is make fakelibvirt's Domain.jobStats() return
+        # VIR_DOMAIN_JOB_COMPLETED.
+        server = etree.fromstring(
+            params['destination_xml']
+        ).find('./uuid').text
+        dom = conn.lookupByUUIDString(server)
+        dom.complete_job()
 
     def test_live_migrate(self):
         server = self._create_server(
@@ -86,14 +123,7 @@ def test_live_migrate(self):
             }
         )
 
-        # FIXME(sean-k-mooney): this should succeed but because of bug #188395
-        # it will fail.
-        # self._wait_for_server_parameter(
-        #     server, {'OS-EXT-SRV-ATTR:host': 'end_host', 'status': 'ACTIVE'})
-        # because of the bug the migration will fail in pre_live_migrate so
-        # the vm should still be active on the start_host
         self._wait_for_server_parameter(
-            server, {'OS-EXT-SRV-ATTR:host': 'start_host', 'status': 'ACTIVE'})
-
+            server, {'OS-EXT-SRV-ATTR:host': 'end_host', 'status': 'ACTIVE'})
         msg = "NotImplementedError: Cannot load 'vif_type' in the base class"
-        self.assertIn(msg, self.stdlog.logger.output)
+        self.assertNotIn(msg, self.stdlog.logger.output)

nova/tests/unit/compute/test_compute.py

@@ -18,6 +18,7 @@
 """Tests for compute service."""
 
 import datetime
+import fixtures as std_fixtures
 from itertools import chain
 import operator
 import sys
@@ -6108,13 +6109,19 @@ def stupid(*args, **kwargs):
             return fake_network.fake_get_instance_nw_info(self)
 
         self.stub_out('nova.network.neutron.API.get_instance_nw_info', stupid)
-
+        self.useFixture(
+            std_fixtures.MonkeyPatch(
+                'nova.network.neutron.API.supports_port_binding_extension',
+                lambda *args: True))
         # creating instance testdata
         instance = self._create_fake_instance_obj({'host': 'dummy'})
         c = context.get_admin_context()
         fake_notifier.NOTIFICATIONS = []
         migrate_data = objects.LibvirtLiveMigrateData(
             is_shared_instance_path=False)
+        vifs = migrate_data_obj.VIFMigrateData.create_skeleton_migrate_vifs(
+                stupid())
+        migrate_data.vifs = vifs
         mock_pre.return_value = migrate_data
 
         with mock.patch.object(self.compute.network_api,

nova/tests/unit/compute/test_compute_mgr.py

@@ -15,6 +15,7 @@
 import contextlib
 import copy
 import datetime
+import fixtures as std_fixtures
 import time
 
 from cinderclient import exceptions as cinder_exception
@@ -3372,20 +3373,48 @@ def _test_check_can_live_migrate_destination(self, do_raise=False,
             mock_event.assert_called_once_with(
                 self.context, 'compute_check_can_live_migrate_destination',
                 CONF.host, instance.uuid, graceful_exit=False)
+            return result
 
     def test_check_can_live_migrate_destination_success(self):
+        self.useFixture(std_fixtures.MonkeyPatch(
+            'nova.network.neutron.API.supports_port_binding_extension',
+            lambda *args: True))
         self._test_check_can_live_migrate_destination()
 
     def test_check_can_live_migrate_destination_fail(self):
+        self.useFixture(std_fixtures.MonkeyPatch(
+            'nova.network.neutron.API.supports_port_binding_extension',
+            lambda *args: True))
         self.assertRaises(
-                test.TestingException,
-                self._test_check_can_live_migrate_destination,
-                do_raise=True)
+            test.TestingException,
+            self._test_check_can_live_migrate_destination,
+            do_raise=True)
+
+    def test_check_can_live_migrate_destination_contins_vifs(self):
+        self.useFixture(std_fixtures.MonkeyPatch(
+            'nova.network.neutron.API.supports_port_binding_extension',
+            lambda *args: True))
+        migrate_data = self._test_check_can_live_migrate_destination()
+        self.assertIn('vifs', migrate_data)
+        self.assertIsNotNone(migrate_data.vifs)
+
+    def test_check_can_live_migrate_destination_no_binding_extended(self):
+        self.useFixture(std_fixtures.MonkeyPatch(
+            'nova.network.neutron.API.supports_port_binding_extension',
+            lambda *args: False))
+        migrate_data = self._test_check_can_live_migrate_destination()
+        self.assertNotIn('vifs', migrate_data)
 
     def test_check_can_live_migrate_destination_src_numa_lm_false(self):
+        self.useFixture(std_fixtures.MonkeyPatch(
+            'nova.network.neutron.API.supports_port_binding_extension',
+            lambda *args: True))
         self._test_check_can_live_migrate_destination(src_numa_lm=False)
 
     def test_check_can_live_migrate_destination_src_numa_lm_true(self):
+        self.useFixture(std_fixtures.MonkeyPatch(
+            'nova.network.neutron.API.supports_port_binding_extension',
+            lambda *args: True))
         self._test_check_can_live_migrate_destination(src_numa_lm=True)
 
     def test_dest_can_numa_live_migrate(self):

releasenotes/notes/restore-rocky-portbinding-semantics-48e9b1fa969cc5e9.yaml

@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    In the Rocky (18.0.0) release support was added to nova to use neutron's
+    multiple port binding feature when the binding-extended API extension
+    is available. In the Train (20.0.0) release the SR-IOV live migration
+    feature broke the semantics of the vifs field in the ``migration_data``
+    object that signals if the new multiple port binding workflow should
+    be used by always populating it even when the ``binding-extended`` API
+    extension is not present. This broke live migration for any deployment
+    that did not support the optional ``binding-extended`` API extension.
+    The Rocky behavior has now been restored enabling live migration
+    using the single port binding workflow when multiple port bindings
+    are not available.