diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 5d20e7d54fba..4f4846311890 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -2044,6 +2044,12 @@ class Connection(object): return VIR_CPU_COMPARE_IDENTICAL + def compareHypervisorCPU( + self, emulator, arch, machine, virttype, + xml, flags + ): + return self.compareCPU(xml, flags) + def getCPUStats(self, cpuNum, flag): if cpuNum < 2: return {'kernel': 5664160000000, diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 9d45ba017c45..44d64978a035 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1320,7 +1320,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) self.assertRaises(exception.Invalid, drvr.init_host, "dummyhost") - @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') + @mock.patch( + 'nova.virt.libvirt.host.libvirt.Connection.compareHypervisorCPU') def test__check_cpu_compatibility_advance_model(self, mocked_compare): mocked_compare.side_effect = (2, 0) self.flags(cpu_mode="custom", @@ -1357,7 +1358,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) drvr.init_host("dummyhost") - @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') + @mock.patch( + 'nova.virt.libvirt.host.libvirt.Connection.compareHypervisorCPU') def test__check_cpu_compatibility_advance_flag(self, mocked_compare): mocked_compare.side_effect = (-1, 0) self.flags(cpu_mode="custom", @@ -1368,7 +1370,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertRaises(exception.InvalidCPUInfo, drvr.init_host, "dummyhost") - @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') + @mock.patch( + 'nova.virt.libvirt.host.libvirt.Connection.compareHypervisorCPU') def test__check_cpu_compatibility_wrong_flag(self, mocked_compare): # here, and in the surrounding similar tests, the non-zero error # code in the compareCPU() side effect indicates error @@ -1381,7 +1384,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertRaises(exception.InvalidCPUInfo, drvr.init_host, "dummyhost") - @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') + @mock.patch( + 'nova.virt.libvirt.host.libvirt.Connection.compareHypervisorCPU') def test__check_cpu_compatibility_enabled_and_disabled_flags( self, mocked_compare ): @@ -11162,7 +11166,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, new=mock.Mock(return_value=False)) @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_all_pass_with_block_migration( self, mock_cpu, mock_test_file, ): @@ -11201,7 +11205,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, new=mock.Mock(return_value=False)) @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_all_pass_with_over_commit( self, mock_cpu, mock_test_file, ): @@ -11241,7 +11245,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, new=mock.Mock(return_value=False)) @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_all_pass_no_block_migration( self, mock_cpu, mock_test_file, ): @@ -11279,7 +11283,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file', return_value='fake') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_fills_listen_addrs( self, mock_cpu, mock_test_file, ): @@ -11311,7 +11315,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file', return_value='fake') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU', + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU', return_value=1) def test_check_can_live_migrate_dest_ensure_serial_adds_not_set( self, mock_cpu, mock_test_file, @@ -11419,7 +11423,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, new=mock.Mock(return_value=False)) @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_no_instance_cpu_info( self, mock_cpu, mock_test_file, ): @@ -11460,7 +11464,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, new=mock.Mock(return_value=False)) @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_shared_storage_test_file') - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_file_backed( self, mock_cpu, mock_test_file, ): @@ -11486,7 +11490,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertTrue(return_value.dst_wants_file_backed_memory) - @mock.patch.object(fakelibvirt.Connection, 'compareCPU') + @mock.patch.object(fakelibvirt.Connection, 'compareHypervisorCPU') def test_check_can_live_migrate_dest_incompatible_cpu_raises( self, mock_cpu): instance_ref = objects.Instance(**self.test_instance) @@ -11522,7 +11526,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, for vif in result.vifs: self.assertTrue(vif.supports_os_vif_delegation) - @mock.patch.object(host.Host, 'compare_cpu') + @mock.patch.object(host.Host, 'compare_hypervisor_cpu') @mock.patch.object(nova.virt.libvirt, 'config') def test_compare_cpu_compatible_host_cpu(self, mock_vconfig, mock_compare): instance = objects.Instance(**self.test_instance) @@ -11532,7 +11536,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance) self.assertIsNone(ret) - @mock.patch.object(host.Host, 'compare_cpu') + @mock.patch.object(host.Host, 'compare_hypervisor_cpu') @mock.patch.object(nova.virt.libvirt, 'config') def test_compare_cpu_handles_not_supported_error_gracefully(self, mock_vconfig, @@ -11569,7 +11573,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(fakelibvirt.Connection, 'getLibVersion', return_value=versionutils.convert_version_to_int( libvirt_driver.MIN_LIBVIRT_AARCH64_CPU_COMPARE)) - @mock.patch.object(host.Host, 'compare_cpu') + @mock.patch.object(host.Host, 'compare_hypervisor_cpu') def test_compare_cpu_host_aarch64(self, mock_compare, mock_get_libversion, @@ -11592,7 +11596,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_compare.assert_called_once_with(caps.host.cpu.to_xml()) self.assertIsNone(ret) - @mock.patch.object(host.Host, 'compare_cpu') + @mock.patch.object(host.Host, 'compare_hypervisor_cpu') @mock.patch.object(nova.virt.libvirt.LibvirtDriver, '_vcpu_model_to_cpu_config') def test_compare_cpu_compatible_guest_cpu(self, mock_vcpu_to_cpu, @@ -11611,7 +11615,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, ret = conn._compare_cpu(None, None, instance) self.assertIsNone(ret) - @mock.patch.object(host.Host, 'compare_cpu') + @mock.patch.object(host.Host, 'compare_hypervisor_cpu') @mock.patch.object(nova.virt.libvirt, 'config') def test_compare_cpu_invalid_cpuinfo_raises(self, mock_vconfig, mock_compare): @@ -11623,7 +11627,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, jsonutils.dumps(_fake_cpu_info), instance) - @mock.patch.object(host.Host, 'compare_cpu') + @mock.patch.object(host.Host, 'compare_hypervisor_cpu') @mock.patch.object(nova.virt.libvirt, 'config') def test_compare_cpu_incompatible_cpu_raises(self, mock_vconfig, mock_compare): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index ff503de7e636..221d292a74b2 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -9963,7 +9963,7 @@ class LibvirtDriver(driver.ComputeDriver): try: cpu_xml = cpu.to_xml() LOG.debug("cpu compare xml: %s", cpu_xml, instance=instance) - ret = self._host.compare_cpu(cpu_xml) + ret = self._host.compare_hypervisor_cpu(cpu_xml) except libvirt.libvirtError as e: error_code = e.get_error_code() if error_code == libvirt.VIR_ERR_NO_SUPPORT: diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 46435a9a7fd8..9026da72172b 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -1605,6 +1605,22 @@ class Host(object): """Compares the given CPU description with the host CPU.""" return self.get_connection().compareCPU(xmlDesc, flags) + def compare_hypervisor_cpu(self, xmlDesc, flags=0): + """Compares the given CPU description with the CPU provided by + the host hypervisor. This is different from the older method, + compare_cpu(), which compares a given CPU definition with the + host CPU without considering the abilities of the host + hypervisor. Except @xmlDesc, rest of all the parameters to + compareHypervisorCPU API are optional (libvirt will choose + sensible defaults). + """ + emulator = None + arch = None + machine = None + virttype = None + return self.get_connection().compareHypervisorCPU( + emulator, arch, machine, virttype, xmlDesc, flags) + def is_cpu_control_policy_capable(self): """Returns whether kernel configuration CGROUP_SCHED is enabled diff --git a/releasenotes/notes/use-compareHypervisorCPU-b75c8f097cc73556.yaml b/releasenotes/notes/use-compareHypervisorCPU-b75c8f097cc73556.yaml new file mode 100644 index 000000000000..924e09a602b9 --- /dev/null +++ b/releasenotes/notes/use-compareHypervisorCPU-b75c8f097cc73556.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Nova's use of libvirt's compareCPU() API has become error-prone as + it doesn't take into account host hypervisor's capabilities. With + QEMU >=2.9 and libvirt >= 4.4.0, libvirt will do the right thing in + terms of CPU comparison checks via a new replacement API, + compareHypervisorCPU(). Nova satisfies the said minimum version + requirements of QEMU and libvirt by a good margin. + + This change replaces the usage of older API, compareCPU(), with the + new one, compareHypervisorCPU().