libvirt: Replace usage of compareCPU() with compareHypervisorCPU()
The older libvirt API compareCPU() does not take into account the capabilities of the "host hypervisor" (KVM, QEMU and the details libvirt knows about the host) when comparing CPUs. This is causing unwanted failures during CPU compatibility checks. To fix this and other related problems, libvirt has introduced (in v4.4.0) a newer API, compareHypervisorCPU(), which _does_ take into account the host hypervisor's capabilities before comparing CPUs. This will help fix a range of problems, such as[1][2]. So let's switch to the newer API, which is largely a drop-in replacement. In this patch: - Introduce a wrapper method, compare_hypervisor_cpu() for libvirt's compareHypervisorCPU() API. - Update the _compare_cpu() method to use the wrapper method, compare_hypervisor_cpu(). - Update the unit tests to use the newer API, compareHypervisorCPU(). [1] https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1978064 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2138381 Change-Id: Ib523753f52993cfe72e35e0309e429ca879c125c Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
This commit is contained in:
Kashyap Chamarthy
parent
9caaaf1f22
commit
468b03e0ee
6
nova/tests/fixtures/libvirt.py
vendored
6
nova/tests/fixtures/libvirt.py
vendored
@ -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,
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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().
|
||||
Loading…
Reference in New Issue
Block a user