diff --git a/nova/tests/functional/libvirt/test_live_migration.py b/nova/tests/functional/libvirt/test_live_migration.py index c339a79196f7..3b50f10f3fee 100644 --- a/nova/tests/functional/libvirt/test_live_migration.py +++ b/nova/tests/functional/libvirt/test_live_migration.py @@ -261,9 +261,9 @@ class LiveMigrationWithCpuSharedSet( conn = self.dest.driver._host.get_connection() dom = conn.lookupByUUIDString(self.server['id']) xml = dom.XMLDesc(0) - # The destination should be updated to "3-4" but it is not the case. - self.assertIn('1', xml) - self.assertNotIn('1', xml) + # The destination should be updated to "3-4". + self.assertNotIn('1', xml) + self.assertIn('1', xml) def test_live_migration_to_no_cpu_shared_set(self): """Reproducer for bug 1869804 #2. @@ -302,10 +302,9 @@ class LiveMigrationWithCpuSharedSet( dom = conn.lookupByUUIDString(self.server['id']) xml = dom.XMLDesc(0) # The destination cpuset should be removed because the - # host has no cpu_shared_set configured. Which is not the case due to - # the bug. - self.assertIn('1', xml) - self.assertNotIn('1', xml) + # host has no cpu_shared_set configured. + self.assertNotIn('1', xml) + self.assertIn('1', xml) def test_live_migration_from_no_cpu_shared_set_to_cpu_shared_set(self): """Reproducer for bug 1869804 #3. @@ -329,5 +328,5 @@ class LiveMigrationWithCpuSharedSet( dom = conn.lookupByUUIDString(self.server['id']) xml = dom.XMLDesc(0) # The destination should be updated to "0-1". - self.assertIn('1', xml) - self.assertNotIn('1', xml) + self.assertNotIn('1', xml) + self.assertIn('1', xml) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 163c1db2874b..5bd5ce4726a4 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -11400,6 +11400,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'dst_wants_file_backed_memory': False, 'graphics_listen_addr_spice': '127.0.0.1', 'graphics_listen_addr_vnc': '127.0.0.1', + 'dst_cpu_shared_set_info': (), 'serial_listen_addr': None}, return_value.obj_to_primitive()['nova_object.data']) @@ -11440,6 +11441,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'dst_wants_file_backed_memory': False, 'graphics_listen_addr_spice': '127.0.0.1', 'graphics_listen_addr_vnc': '127.0.0.1', + 'dst_cpu_shared_set_info': (), 'serial_listen_addr': None}, return_value.obj_to_primitive()['nova_object.data']) @@ -11477,6 +11479,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'dst_wants_file_backed_memory': False, 'graphics_listen_addr_spice': '127.0.0.1', 'graphics_listen_addr_vnc': '127.0.0.1', + 'dst_cpu_shared_set_info': (), 'serial_listen_addr': None}, return_value.obj_to_primitive()['nova_object.data']) @@ -11563,6 +11566,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'dst_wants_file_backed_memory': False, 'graphics_listen_addr_spice': '127.0.0.1', 'graphics_listen_addr_vnc': '127.0.0.1', + 'dst_cpu_shared_set_info': (), 'serial_listen_addr': None}, result.obj_to_primitive()['nova_object.data']) @@ -11703,6 +11707,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'dst_wants_file_backed_memory': False, 'graphics_listen_addr_spice': '127.0.0.1', 'graphics_listen_addr_vnc': '127.0.0.1', + 'dst_cpu_shared_set_info': (), 'serial_listen_addr': None}, return_value.obj_to_primitive()['nova_object.data']) diff --git a/nova/tests/unit/virt/libvirt/test_migration.py b/nova/tests/unit/virt/libvirt/test_migration.py index ca4fb02a1267..943b22ba2c6c 100644 --- a/nova/tests/unit/virt/libvirt/test_migration.py +++ b/nova/tests/unit/virt/libvirt/test_migration.py @@ -68,15 +68,18 @@ class UtilityMigrationTestCase(test.NoDBTestCase): 'spice': None}, addrs) @mock.patch('lxml.etree.tostring') + @mock.patch.object(migration, '_remove_cpu_shared_set_xml') + @mock.patch.object(migration, '_update_cpu_shared_set_xml') @mock.patch.object(migration, '_update_memory_backing_xml') @mock.patch.object(migration, '_update_perf_events_xml') @mock.patch.object(migration, '_update_graphics_xml') @mock.patch.object(migration, '_update_serial_xml') @mock.patch.object(migration, '_update_volume_xml') - def test_get_updated_guest_xml( - self, mock_volume, mock_serial, mock_graphics, - mock_perf_events_xml, mock_memory_backing, mock_tostring): - data = objects.LibvirtLiveMigrateData() + def _test_get_updated_guest_xml( + self, data, check_cpu_shared_set, mock_volume, mock_serial, + mock_graphics, mock_perf_events_xml, mock_memory_backing, + mock_update_cpu_shared_set, mock_remove_cpu_shared_set, mock_tostring + ): mock_guest = mock.Mock(spec=libvirt_guest.Guest) get_volume_config = mock.MagicMock() mock_guest.get_xml_desc.return_value = '' @@ -91,6 +94,57 @@ class UtilityMigrationTestCase(test.NoDBTestCase): mock_perf_events_xml.assert_called_once_with(mock.ANY, data) mock_memory_backing.assert_called_once_with(mock.ANY, data) self.assertEqual(1, mock_tostring.called) + if check_cpu_shared_set is not None: + check_cpu_shared_set( + mock_update_cpu_shared_set, mock_remove_cpu_shared_set) + + def test_get_updated_guest_xml(self): + data = objects.LibvirtLiveMigrateData() + self._test_get_updated_guest_xml(data, None) + + def test_get_updated_guest_xml_dst_cpu_shared_set_info_unset(self): + data = objects.LibvirtLiveMigrateData() + + def check_cpu_shared_set( + mock_update_cpu_shared_set, mock_remove_cpu_shared_set + ): + mock_update_cpu_shared_set.assert_not_called() + mock_remove_cpu_shared_set.assert_not_called() + + self._test_get_updated_guest_xml(data, check_cpu_shared_set) + + def test_get_updated_guest_xml_dst_cpu_shared_set_info_set_to_none(self): + """Ensure this test fails as dst_cpu_shared_set_info cannot be None + """ + def set_dst_cpu_shared_set_info(): + data.dst_cpu_shared_set_info = None + + data = objects.LibvirtLiveMigrateData() + self.assertRaises(ValueError, set_dst_cpu_shared_set_info) + + def test_get_updated_guest_xml_dst_cpu_shared_set_info_set_to_empty(self): + data = objects.LibvirtLiveMigrateData() + data.dst_cpu_shared_set_info = set() + + def check_cpu_shared_set( + mock_update_cpu_shared_set, mock_remove_cpu_shared_set + ): + mock_update_cpu_shared_set.assert_not_called() + mock_remove_cpu_shared_set.assert_called_once_with(mock.ANY, data) + + self._test_get_updated_guest_xml(data, check_cpu_shared_set) + + def test_get_updated_guest_xml_dst_cpu_shared_set_info_set(self): + data = objects.LibvirtLiveMigrateData() + data.dst_cpu_shared_set_info = set([2, 3]) + + def check_cpu_shared_set( + mock_update_cpu_shared_set, mock_remove_cpu_shared_set + ): + mock_update_cpu_shared_set.assert_called_once_with(mock.ANY, data) + mock_remove_cpu_shared_set.assert_not_called() + + self._test_get_updated_guest_xml(data, check_cpu_shared_set) def test_update_quota_xml(self): old_xml = """ @@ -224,6 +278,44 @@ class UtilityMigrationTestCase(test.NoDBTestCase): self.assertRaises(exception.NovaException, migration._update_mdev_xml, doc, data.target_mdevs) + def test_update_cpu_shared_set_xml(self): + doc = etree.fromstring(""" + + 1 + """) + data = objects.LibvirtLiveMigrateData( + dst_cpu_shared_set_info=set([3, 4])) + + result = etree.tostring( + migration._update_cpu_shared_set_xml(copy.deepcopy(doc), data), + encoding='unicode') + + expected = textwrap.dedent(""" + + 1 + """) + + self.assertXmlEqual(expected, result) + + def test_remove_cpu_shared_set_xml(self): + doc = etree.fromstring(""" + + 1 + """) + data = objects.LibvirtLiveMigrateData( + dst_cpu_shared_set_info=set()) + + result = etree.tostring( + migration._remove_cpu_shared_set_xml(copy.deepcopy(doc), data), + encoding='unicode') + + expected = textwrap.dedent(""" + + 1 + """) + + self.assertXmlEqual(expected, result) + def test_update_numa_xml(self): doc = etree.fromstring(""" diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index c01ef56a348b..d6c966b9a921 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -10007,6 +10007,12 @@ class LibvirtDriver(driver.ComputeDriver): if instance.numa_topology: data.dst_supports_numa_live_migration = True + data.dst_cpu_shared_set_info = ( + hardware.get_cpu_shared_set() or + hardware.get_vcpu_pin_set() or + set() + ) + # 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. diff --git a/nova/virt/libvirt/migration.py b/nova/virt/libvirt/migration.py index 25d271cac767..ac632a7e2e50 100644 --- a/nova/virt/libvirt/migration.py +++ b/nova/virt/libvirt/migration.py @@ -65,6 +65,25 @@ def get_updated_guest_xml(instance, guest, migrate_data, get_volume_config, xml_doc = _update_quota_xml(instance, xml_doc) if get_vif_config is not None: xml_doc = _update_vif_xml(xml_doc, migrate_data, get_vif_config) + + # If 'dst_cpu_shared_set_info' is set, we are migrating a VM to a + # destination host, patched to fix bug 1869804. + # Then, if dst_cpu_shared_set_info is empty (set()), it means that there + # is no cpu_shared_set configuration on the destination host. + if ( + 'dst_cpu_shared_set_info' in migrate_data and + not migrate_data.dst_cpu_shared_set_info + ): + # There is no cpu_shared_set configured on destination host. So we + # need to remove the VM cpuset if any. + xml_doc = _remove_cpu_shared_set_xml(xml_doc, migrate_data) + if ( + 'dst_cpu_shared_set_info' in migrate_data and + migrate_data.dst_cpu_shared_set_info + ): + # There is cpu_shared_set configured on destination host. So we need + # to update the VM cpuset. + xml_doc = _update_cpu_shared_set_xml(xml_doc, migrate_data) if 'dst_numa_info' in migrate_data: xml_doc = _update_numa_xml(xml_doc, migrate_data) if 'target_mdevs' in migrate_data: @@ -130,6 +149,34 @@ def _update_mdev_xml(xml_doc, target_mdevs): return xml_doc +def _update_cpu_shared_set_xml(xml_doc, migrate_data): + LOG.debug('_update_cpu_shared_set_xml input xml=%s', + etree.tostring(xml_doc, encoding='unicode', pretty_print=True)) + + vcpu = xml_doc.find('./vcpu') + vcpu.set('cpuset', hardware.format_cpu_spec( + migrate_data.dst_cpu_shared_set_info, True)) + + LOG.debug('_update_cpu_shared_set_xml output xml=%s', + etree.tostring(xml_doc, encoding='unicode', pretty_print=True)) + return xml_doc + + +def _remove_cpu_shared_set_xml(xml_doc, migrate_data): + LOG.debug('_remove_cpu_shared_set_xml input xml=%s', + etree.tostring(xml_doc, encoding='unicode', pretty_print=True)) + + vcpu = xml_doc.find('./vcpu') + if vcpu is not None: + cpuset = vcpu.get('cpuset') + if cpuset: + del vcpu.attrib['cpuset'] + + LOG.debug('_remove_cpu_shared_set_xml output xml=%s', + etree.tostring(xml_doc, encoding='unicode', pretty_print=True)) + return xml_doc + + def _update_numa_xml(xml_doc, migrate_data): LOG.debug('_update_numa_xml input xml=%s', etree.tostring(xml_doc, encoding='unicode', pretty_print=True))