From e60e859c9fd341bf341226a52cecbbc935b572a4 Mon Sep 17 00:00:00 2001
From: Keith Berger <keith.berger@suse.com>
Date: Mon, 24 Aug 2020 19:08:03 -0400
Subject: [PATCH] From 924c23996b42da27d785804c777ed843c0729983 Mon Sep 17
 00:00:00 2001 From: Lee Yarwood <lyarwood@redhat.com> Date: Wed, 5 Aug 2020
 23:00:06 +0100 Subject: [PATCH] libvirt: Provide
 VIR_MIGRATE_PARAM_PERSIST_XML during live  migration

The VIR_MIGRATE_PARAM_PERSIST_XML parameter was introduced in libvirt
v1.3.4 and is used to provide the new persistent configuration for the
destination during a live migration:

https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_MIGRATE_PARAM_PERSIST_XML

Without this parameter the persistent configuration on the destination
will be the same as the original persistent configuration on the source
when the VIR_MIGRATE_PERSIST_DEST flag is provided.

As Nova does not currently provide the VIR_MIGRATE_PARAM_PERSIST_XML
param but does provide the VIR_MIGRATE_PERSIST_DEST flag this means that
a soft reboot by Nova of the instance after a live migration can revert
the domain back to the original persistent configuration from the
source.

Note that this is only possible in Nova as a soft reboot actually
results in the virDomainShutdown and virDomainLaunch libvirt APIs being
called that recreate the domain using the persistent configuration.
virDomainReboot does not result in this but is not called at this time.

The impact of this on the instance after the soft reboot is pretty
severe, host devices referenced in the original persistent configuration
on the source may not exist or could even be used by other users on the
destination. CPU and NUMA affinity could also differ drastically between
the two hosts resulting in the instance being unable to start etc.

As MIN_LIBVIRT_VERSION is now > v1.3.4 this change simply includes the
VIR_MIGRATE_PARAM_PERSIST_XML param using the same updated XML for the
destination as is already provided to VIR_MIGRATE_PARAM_DEST_XML.

Conflicts:
    nova/tests/unit/virt/libvirt/test_driver.py
    nova/virt/libvirt/driver.py

Co-authored-by: Tadayoshi Hosoya <tad-hosoya@wr.jp.nec.com>
Closes-Bug: #1890501
Change-Id: Ia3f1d8e83cbc574ce5cb440032e12bbcb1e10e98
(cherry picked from commit 1bb8ee95d4c3ddc3f607ac57526b75af1b7fbcff)
(cherry picked from commit bbf9d1de06e9991acd968fceee899a8df3776d60)
(cherry picked from commit 6a07edb4b29d8bfb5c86ed14263f7cd7525958c1)
(cherry picked from commit b9ea91d17703f5b324a50727b6503ace0f4e95eb)
(cherry picked from commit c438fd9a0eb1903306a53ab44e3ae80660d8a429)
(cherry picked from commit a100c517ae4224a41d69346cace7a41cace7c165)
---
 nova/tests/unit/virt/libvirt/test_driver.py | 88 +++++++++++++++++++--
 nova/virt/libvirt/driver.py                 | 11 +++
 2 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index f5595e8eab..8ba4bc46b0 100755
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -8536,6 +8536,76 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 'instance', data, block_device_info=bdi))
         self.assertEqual(0, mock_get_instance_disk_info.call_count)
 
+    @mock.patch.object(fakelibvirt.virDomain, "migrateToURI3")
+    @mock.patch('nova.virt.libvirt.migration.get_updated_guest_xml')
+    @mock.patch.object(fakelibvirt.Connection, 'getLibVersion')
+    def test_live_migration_persistent_xml(
+        self, mock_get_version, mock_get_updated_xml, mock_migrateToURI3):
+        """Assert that persistent_xml only provided when libvirt is >= v1.3.4
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instance = self.test_instance
+        dest = '127.0.0.1'
+        block_migration = False
+        migrate_data = objects.LibvirtLiveMigrateData(
+            graphics_listen_addr_vnc='10.0.0.1',
+            graphics_listen_addr_spice='10.0.0.2',
+            serial_listen_addr='127.0.0.1',
+            target_connect_addr='127.0.0.1',
+            bdms=[],
+            block_migration=block_migration)
+        guest = libvirt_guest.Guest(fakelibvirt.virDomain)
+        device_names = ['vda']
+
+        mock_get_updated_xml.return_value = mock.sentinel.dest_xml
+
+        # persistent_xml was introduced in v1.3.4 so provide v1.3.3
+        v1_3_3 = versionutils.convert_version_to_int((1, 3, 3))
+        mock_get_version.return_value = v1_3_3
+
+        drvr._live_migration_operation(
+            self.context, instance, dest, block_migration, migrate_data,
+            guest, device_names)
+
+        expected_uri = drvr._live_migration_uri(dest)
+        expected_flags = 0
+        expected_params = {
+            'bandwidth': 0,
+            'destination_xml': mock.sentinel.dest_xml,
+            'migrate_disks': device_names,
+            'migrate_uri': 'tcp://127.0.0.1'
+        }
+
+        # Assert that migrateToURI3 is called without the persistent_xml param
+        mock_get_version.assert_called()
+        mock_migrateToURI3.assert_called_once_with(
+            expected_uri, params=expected_params, flags=expected_flags)
+
+        # reset mocks and try again with v1.3.4
+        mock_get_version.reset_mock()
+        mock_migrateToURI3.reset_mock()
+
+        # persistent_xml was introduced in v1.3.4 so provide it this time
+        v1_3_4 = versionutils.convert_version_to_int((1, 3, 4))
+        mock_get_version.return_value = v1_3_4
+
+        drvr._live_migration_operation(
+            self.context, instance, dest,
+            block_migration, migrate_data, guest, device_names)
+
+        expected_params = {
+            'bandwidth': 0,
+            'destination_xml': mock.sentinel.dest_xml,
+            'persistent_xml': mock.sentinel.dest_xml,
+            'migrate_disks': device_names,
+            'migrate_uri': 'tcp://127.0.0.1'
+        }
+
+        # Assert that migrateToURI3 is called with the persistent_xml param
+        mock_get_version.assert_called()
+        mock_migrateToURI3.assert_called_once_with(
+                expected_uri, params=expected_params, flags=expected_flags)
+
     def test_live_migration_update_graphics_xml(self):
         self.compute = manager.ComputeManager()
         instance_dict = dict(self.test_instance)
@@ -9059,19 +9129,21 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
     @mock.patch.object(host.Host, 'has_min_version', return_value=True)
     @mock.patch.object(fakelibvirt.virDomain, "migrateToURI3")
-    @mock.patch('nova.virt.libvirt.migration.get_updated_guest_xml',
-                return_value='')
+    @mock.patch('nova.virt.libvirt.migration.get_updated_guest_xml')
     @mock.patch('nova.virt.libvirt.guest.Guest.get_xml_desc',
                 return_value='<xml></xml>')
     def test_live_migration_uses_migrateToURI3(
             self, mock_old_xml, mock_new_xml, mock_migrateToURI3,
             mock_min_version):
+
+        mock_new_xml.return_value = mock.sentinel.new_xml
         # Preparing mocks
         disk_paths = ['vda', 'vdb']
         params = {
             'migrate_disks': ['vda', 'vdb'],
             'bandwidth': CONF.libvirt.live_migration_bandwidth,
-            'destination_xml': '',
+            'destination_xml': mock.sentinel.new_xml,
+            'persistent_xml': mock.sentinel.new_xml,
         }
         mock_migrateToURI3.side_effect = fakelibvirt.libvirtError("ERR")
 
@@ -9126,6 +9198,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             'migrate_disks': device_names,
             'bandwidth': CONF.libvirt.live_migration_bandwidth,
             'destination_xml': b'<xml/>',
+            'persistent_xml': b'<xml/>',
         }
         mock_migrateToURI3.assert_called_once_with(
             drvr._live_migration_uri('dest'), params=params,
@@ -9154,18 +9227,21 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
     @mock.patch.object(host.Host, 'has_min_version', return_value=True)
     @mock.patch.object(fakelibvirt.virDomain, "migrateToURI3")
-    @mock.patch('nova.virt.libvirt.migration.get_updated_guest_xml',
-                return_value='')
+    @mock.patch('nova.virt.libvirt.migration.get_updated_guest_xml')
     @mock.patch('nova.virt.libvirt.guest.Guest.get_xml_desc', return_value='')
     def test_block_live_migration_tunnelled_migrateToURI3(
             self, mock_old_xml, mock_new_xml,
             mock_migrateToURI3, mock_min_version):
         self.flags(live_migration_tunnelled=True, group='libvirt')
+
+        mock_new_xml.return_value = mock.sentinel.new_xml
+
         # Preparing mocks
         disk_paths = []
         params = {
             'bandwidth': CONF.libvirt.live_migration_bandwidth,
-            'destination_xml': '',
+            'destination_xml': mock.sentinel.new_xml,
+            'persistent_xml': mock.sentinel.new_xml,
         }
         # Start test
         migrate_data = objects.LibvirtLiveMigrateData(
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index c616406088..b063a6a557 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -302,6 +302,11 @@ PERF_EVENTS_CPU_FLAG_MAPPING = {'cmt': 'cmt',
                                 'mbmt': 'mbm_total',
                                }
 
+# libvirt >= v1.3.4 introduced VIR_MIGRATE_PARAM_PERSIST_XML that needs to be
+# provided when the VIR_MIGRATE_PERSIST_DEST flag is used to ensure the updated
+# domain XML is persisted on the destination.
+MIN_LIBVIRT_MIGRATE_PARAM_PERSIST_XML = (1, 3, 4)
+
 
 class LibvirtDriver(driver.ComputeDriver):
     capabilities = {
@@ -6519,6 +6524,12 @@ class LibvirtDriver(driver.ComputeDriver):
                     libvirt.VIR_MIGRATE_TUNNELLED != 0):
                     params.pop('migrate_disks')
 
+                # NOTE(lyarwood): Only available from v1.3.4
+                if self._host.has_min_version(
+                    MIN_LIBVIRT_MIGRATE_PARAM_PERSIST_XML
+                ):
+                    params['persistent_xml'] = new_xml_str
+
             # TODO(sahid): This should be in
             # post_live_migration_at_source but no way to retrieve
             # ports acquired on the host for the guest at this
-- 
2.28.0

