libvirt: fix overly strict CPU model comparison in live migration
The current libvirt driver migration code checks whether the dest host can support the guest, by comparing the source host CPU model. This is overly strict, as the guest may well be running with a more restrictive CPU model that hides many host features. The correct approach is to compare against the guest's configured CPU model. To achieve the compatibility when migrate instance from old compute node which does not provide vcpu_model in instance object, the source host's compute info is still provided. Co-authored: Daniel P. Berrange <berrange@redhat.com> Co-authored: Yunhong Jiang <yunhong.jiang@intel.com> Closes-Bug: 1082414 Change-Id: I65f505fec64c65d2641d2bfd940cde44bcd83a78
This commit is contained in:
yunhong jiang
parent
9758f7f0ef
commit
79a0755597
|
|
@ -66,6 +66,7 @@ from nova.tests.unit import fake_network
|
|||
import nova.tests.unit.image.fake
|
||||
from nova.tests.unit import matchers
|
||||
from nova.tests.unit.objects import test_pci_device
|
||||
from nova.tests.unit.objects import test_vcpu_model
|
||||
from nova.tests.unit.virt.libvirt import fake_imagebackend
|
||||
from nova.tests.unit.virt.libvirt import fake_libvirt_utils
|
||||
from nova.tests.unit.virt.libvirt import fakelibvirt
|
||||
|
|
@ -6079,26 +6080,28 @@ class LibvirtConnTestCase(test.TestCase):
|
|||
self.assertEqual(29, fake_timer.counter, "Didn't wait the expected "
|
||||
"amount of time")
|
||||
|
||||
def test_check_can_live_migrate_dest_all_pass_with_block_migration(self):
|
||||
@mock.patch.object(libvirt_driver.LibvirtDriver,
|
||||
'_create_shared_storage_test_file')
|
||||
@mock.patch.object(fakelibvirt.Connection, 'compareCPU')
|
||||
def test_check_can_live_migrate_dest_all_pass_with_block_migration(
|
||||
self, mock_cpu, mock_test_file):
|
||||
instance_ref = objects.Instance(**self.test_instance)
|
||||
instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
compute_info = {'disk_available_least': 400,
|
||||
'cpu_info': 'asdf',
|
||||
}
|
||||
filename = "file"
|
||||
|
||||
self.mox.StubOutWithMock(drvr, '_create_shared_storage_test_file')
|
||||
self.mox.StubOutWithMock(drvr, '_compare_cpu')
|
||||
|
||||
# _check_cpu_match
|
||||
drvr._compare_cpu("asdf")
|
||||
mock_cpu.return_value = 1
|
||||
|
||||
# mounted_on_same_shared_storage
|
||||
drvr._create_shared_storage_test_file().AndReturn(filename)
|
||||
mock_test_file.return_value = filename
|
||||
|
||||
self.mox.ReplayAll()
|
||||
# No need for the src_compute_info
|
||||
return_value = drvr.check_can_live_migrate_destination(self.context,
|
||||
instance_ref, compute_info, compute_info, True)
|
||||
instance_ref, None, compute_info, True)
|
||||
self.assertThat({"filename": "file",
|
||||
'image_type': 'default',
|
||||
'disk_available_mb': 409600,
|
||||
|
|
@ -6106,22 +6109,54 @@ class LibvirtConnTestCase(test.TestCase):
|
|||
"block_migration": True},
|
||||
matchers.DictMatches(return_value))
|
||||
|
||||
def test_check_can_live_migrate_dest_all_pass_no_block_migration(self):
|
||||
@mock.patch.object(libvirt_driver.LibvirtDriver,
|
||||
'_create_shared_storage_test_file')
|
||||
@mock.patch.object(fakelibvirt.Connection, 'compareCPU')
|
||||
def test_check_can_live_migrate_dest_all_pass_no_block_migration(
|
||||
self, mock_cpu, mock_test_file):
|
||||
instance_ref = objects.Instance(**self.test_instance)
|
||||
instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
compute_info = {'cpu_info': 'asdf'}
|
||||
compute_info = {'disk_available_least': 400,
|
||||
'cpu_info': 'asdf',
|
||||
}
|
||||
filename = "file"
|
||||
|
||||
self.mox.StubOutWithMock(drvr, '_create_shared_storage_test_file')
|
||||
self.mox.StubOutWithMock(drvr, '_compare_cpu')
|
||||
# _check_cpu_match
|
||||
mock_cpu.return_value = 1
|
||||
# mounted_on_same_shared_storage
|
||||
mock_test_file.return_value = filename
|
||||
# No need for the src_compute_info
|
||||
return_value = drvr.check_can_live_migrate_destination(self.context,
|
||||
instance_ref, None, compute_info, False)
|
||||
self.assertThat({"filename": "file",
|
||||
"image_type": 'default',
|
||||
"block_migration": False,
|
||||
"disk_over_commit": False,
|
||||
"disk_available_mb": None},
|
||||
matchers.DictMatches(return_value))
|
||||
|
||||
@mock.patch.object(libvirt_driver.LibvirtDriver,
|
||||
'_create_shared_storage_test_file')
|
||||
@mock.patch.object(fakelibvirt.Connection, 'compareCPU')
|
||||
def test_check_can_live_migrate_dest_no_instance_cpu_info(
|
||||
self, mock_cpu, mock_test_file):
|
||||
instance_ref = objects.Instance(**self.test_instance)
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
compute_info = {'cpu_info': jsonutils.dumps({
|
||||
"vendor": "AMD",
|
||||
"arch": arch.I686,
|
||||
"features": ["sse3"],
|
||||
"model": "Opteron_G3",
|
||||
"topology": {"cores": 2, "threads": 1, "sockets": 4}
|
||||
})}
|
||||
filename = "file"
|
||||
|
||||
# _check_cpu_match
|
||||
drvr._compare_cpu("asdf")
|
||||
|
||||
mock_cpu.return_value = 1
|
||||
# mounted_on_same_shared_storage
|
||||
drvr._create_shared_storage_test_file().AndReturn(filename)
|
||||
mock_test_file.return_value = filename
|
||||
|
||||
self.mox.ReplayAll()
|
||||
return_value = drvr.check_can_live_migrate_destination(self.context,
|
||||
instance_ref, compute_info, compute_info, False)
|
||||
self.assertThat({"filename": "file",
|
||||
|
|
@ -6131,18 +6166,15 @@ class LibvirtConnTestCase(test.TestCase):
|
|||
"disk_available_mb": None},
|
||||
matchers.DictMatches(return_value))
|
||||
|
||||
def test_check_can_live_migrate_dest_incompatible_cpu_raises(self):
|
||||
@mock.patch.object(fakelibvirt.Connection, 'compareCPU')
|
||||
def test_check_can_live_migrate_dest_incompatible_cpu_raises(
|
||||
self, mock_cpu):
|
||||
instance_ref = objects.Instance(**self.test_instance)
|
||||
instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
compute_info = {'cpu_info': 'asdf'}
|
||||
|
||||
self.mox.StubOutWithMock(drvr, '_compare_cpu')
|
||||
|
||||
drvr._compare_cpu("asdf").AndRaise(exception.InvalidCPUInfo(
|
||||
reason='foo')
|
||||
)
|
||||
|
||||
self.mox.ReplayAll()
|
||||
mock_cpu.side_effect = exception.InvalidCPUInfo(reason='foo')
|
||||
self.assertRaises(exception.InvalidCPUInfo,
|
||||
drvr.check_can_live_migrate_destination,
|
||||
self.context, instance_ref,
|
||||
|
|
@ -13248,6 +13280,29 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
|
|||
self.assertEqual(cpumodel.MODE_HOST_MODEL, vcpu_model.mode)
|
||||
self.assertEqual(vcpu_model, vcpu_model_1)
|
||||
|
||||
def test_vcpu_model_to_config(self):
|
||||
drv = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
|
||||
|
||||
feature = objects.VirtCPUFeature(policy=cpumodel.POLICY_REQUIRE,
|
||||
name='sse')
|
||||
feature_1 = objects.VirtCPUFeature(policy=cpumodel.POLICY_FORBID,
|
||||
name='aes')
|
||||
topo = objects.VirtCPUTopology(sockets=1, cores=2, threads=4)
|
||||
vcpu_model = objects.VirtCPUModel(mode=cpumodel.MODE_HOST_MODEL,
|
||||
features=[feature, feature_1],
|
||||
topology=topo)
|
||||
|
||||
cpu = drv._vcpu_model_to_cpu_config(vcpu_model)
|
||||
self.assertEqual(cpumodel.MODE_HOST_MODEL, cpu.mode)
|
||||
self.assertEqual(1, cpu.sockets)
|
||||
self.assertEqual(4, cpu.threads)
|
||||
self.assertEqual(2, len(cpu.features))
|
||||
self.assertEqual(set(['sse', 'aes']),
|
||||
set([f.name for f in cpu.features]))
|
||||
self.assertEqual(set([cpumodel.POLICY_REQUIRE,
|
||||
cpumodel.POLICY_FORBID]),
|
||||
set([f.policy for f in cpu.features]))
|
||||
|
||||
|
||||
class LibvirtVolumeUsageTestCase(test.NoDBTestCase):
|
||||
"""Test for LibvirtDriver.get_all_volume_usage."""
|
||||
|
|
|
|||
|
|
@ -3904,6 +3904,33 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
|
||||
return vcpu_model
|
||||
|
||||
def _vcpu_model_to_cpu_config(self, vcpu_model):
|
||||
"""Create libvirt CPU config according to VirtCPUModel object.
|
||||
|
||||
:param:vcpu_model: VirtCPUModel object.
|
||||
|
||||
:return: vconfig.LibvirtConfigGuestCPU.
|
||||
|
||||
"""
|
||||
|
||||
cpu_config = vconfig.LibvirtConfigGuestCPU()
|
||||
cpu_config.arch = vcpu_model.arch
|
||||
cpu_config.model = vcpu_model.model
|
||||
cpu_config.mode = vcpu_model.mode
|
||||
cpu_config.match = vcpu_model.match
|
||||
cpu_config.vendor = vcpu_model.vendor
|
||||
if vcpu_model.topology:
|
||||
cpu_config.sockets = vcpu_model.topology.sockets
|
||||
cpu_config.cores = vcpu_model.topology.cores
|
||||
cpu_config.threads = vcpu_model.topology.threads
|
||||
if vcpu_model.features:
|
||||
for f in vcpu_model.features:
|
||||
xf = vconfig.LibvirtConfigGuestCPUFeature()
|
||||
xf.name = f.name
|
||||
xf.policy = f.policy
|
||||
cpu_config.features.add(xf)
|
||||
return cpu_config
|
||||
|
||||
def _get_guest_config(self, instance, network_info, image_meta,
|
||||
disk_info, rescue=None, block_device_info=None,
|
||||
context=None, flavor=None):
|
||||
|
|
@ -4919,8 +4946,11 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
(disk_available_gb * units.Ki) - CONF.reserved_host_disk_mb
|
||||
|
||||
# Compare CPU
|
||||
source_cpu_info = src_compute_info['cpu_info']
|
||||
self._compare_cpu(source_cpu_info)
|
||||
if not instance.vcpu_model:
|
||||
source_cpu_info = src_compute_info['cpu_info']
|
||||
self._compare_cpu(None, source_cpu_info)
|
||||
else:
|
||||
self._compare_cpu(instance.vcpu_model, None)
|
||||
|
||||
# Create file on storage, to be checked on source host
|
||||
filename = self._create_shared_storage_test_file()
|
||||
|
|
@ -5071,34 +5101,40 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
'necessary': necessary})
|
||||
raise exception.MigrationPreCheckError(reason=reason)
|
||||
|
||||
def _compare_cpu(self, cpu_info):
|
||||
"""Checks the host cpu is compatible to a cpu given by xml.
|
||||
"xml" must be a part of libvirt.openAuth(...).getCapabilities().
|
||||
return values follows by virCPUCompareResult.
|
||||
if 0 > return value, do live migration.
|
||||
'http://libvirt.org/html/libvirt-libvirt.html#virCPUCompareResult'
|
||||
def _compare_cpu(self, guest_cpu, host_cpu_str):
|
||||
"""Check the host is compatible with the requested CPU
|
||||
|
||||
:param guest_cpu: nova.objects.VirtCPUModel or None
|
||||
:param host_cpu_str: JSON from _get_cpu_info() method
|
||||
|
||||
If the 'guest_cpu' parameter is not None, this will be
|
||||
validated for migration compatibility with the host.
|
||||
Otherwise the 'host_cpu_str' JSON string will be used for
|
||||
validation.
|
||||
|
||||
:param cpu_info: json string of cpu feature from _get_cpu_info()
|
||||
:returns:
|
||||
None. if given cpu info is not compatible to this server,
|
||||
raise exception.
|
||||
"""
|
||||
|
||||
# NOTE(berendt): virConnectCompareCPU not working for Xen
|
||||
if CONF.libvirt.virt_type == 'xen':
|
||||
return 1
|
||||
if CONF.libvirt.virt_type not in ['qemu', 'kvm']:
|
||||
return
|
||||
|
||||
info = jsonutils.loads(cpu_info)
|
||||
LOG.info(_LI('Instance launched has CPU info: %s'), cpu_info)
|
||||
cpu = vconfig.LibvirtConfigCPU()
|
||||
cpu.arch = info['arch']
|
||||
cpu.model = info['model']
|
||||
cpu.vendor = info['vendor']
|
||||
cpu.sockets = info['topology']['sockets']
|
||||
cpu.cores = info['topology']['cores']
|
||||
cpu.threads = info['topology']['threads']
|
||||
for f in info['features']:
|
||||
cpu.add_feature(vconfig.LibvirtConfigCPUFeature(f))
|
||||
if guest_cpu is None:
|
||||
info = jsonutils.loads(host_cpu_str)
|
||||
LOG.info(_LI('Instance launched has CPU info: %s'), host_cpu_str)
|
||||
cpu = vconfig.LibvirtConfigCPU()
|
||||
cpu.arch = info['arch']
|
||||
cpu.model = info['model']
|
||||
cpu.vendor = info['vendor']
|
||||
cpu.sockets = info['topology']['sockets']
|
||||
cpu.cores = info['topology']['cores']
|
||||
cpu.threads = info['topology']['threads']
|
||||
for f in info['features']:
|
||||
cpu.add_feature(vconfig.LibvirtConfigCPUFeature(f))
|
||||
else:
|
||||
cpu = self._vcpu_model_to_cpu_config(guest_cpu)
|
||||
|
||||
u = "http://libvirt.org/html/libvirt-libvirt.html#virCPUCompareResult"
|
||||
m = _("CPU doesn't have compatibility.\n\n%(ret)s\n\nRefer to %(u)s")
|
||||
|
|
|
|||
Loading…
Reference in New Issue