diff --git a/doc/source/admin/adv-config.rst b/doc/source/admin/adv-config.rst index 82e6be6b32d1..225b13366372 100644 --- a/doc/source/admin/adv-config.rst +++ b/doc/source/admin/adv-config.rst @@ -20,6 +20,8 @@ vital. In these cases, instances can be expected to deliver near-native performance. The Compute service provides features to improve individual instance for these kind of workloads. +.. include:: /common/numa-live-migration-warning.txt + .. toctree:: :maxdepth: 2 diff --git a/doc/source/admin/configuring-migrations.rst b/doc/source/admin/configuring-migrations.rst index dc6a9a9832e5..d36e0f23bad3 100644 --- a/doc/source/admin/configuring-migrations.rst +++ b/doc/source/admin/configuring-migrations.rst @@ -19,7 +19,8 @@ This document covers live migrations using the .. note:: Not all Compute service hypervisor drivers support live-migration, or - support all live-migration features. + support all live-migration features. Similarly not all compute service + features are supported. Consult :doc:`/user/support-matrix` to determine which hypervisors support live-migration. diff --git a/doc/source/admin/cpu-topologies.rst b/doc/source/admin/cpu-topologies.rst index 5c9174e1c33e..8edb0f565fd4 100644 --- a/doc/source/admin/cpu-topologies.rst +++ b/doc/source/admin/cpu-topologies.rst @@ -7,6 +7,8 @@ control over how instances run on hypervisor CPUs and the topology of virtual CPUs available to instances. These features help minimize latency and maximize performance. +.. include:: /common/numa-live-migration-warning.txt + SMP, NUMA, and SMT ~~~~~~~~~~~~~~~~~~ diff --git a/doc/source/common/numa-live-migration-warning.txt b/doc/source/common/numa-live-migration-warning.txt new file mode 100644 index 000000000000..7b695f218830 --- /dev/null +++ b/doc/source/common/numa-live-migration-warning.txt @@ -0,0 +1,10 @@ +.. important:: + + Unless :oslo.config:option:`specifically enabled + `, live migration is not currently + possible for instances with a NUMA topology when using the libvirt driver. + A NUMA topology may be specified explicitly or can be added implicitly due + to the use of CPU pinning or huge pages. Refer to `bug #1289064`__ for more + information. + + __ https://bugs.launchpad.net/nova/+bug/1289064 diff --git a/doc/source/user/feature-classification.rst b/doc/source/user/feature-classification.rst index ceeafe572f5d..db0ce886d973 100644 --- a/doc/source/user/feature-classification.rst +++ b/doc/source/user/feature-classification.rst @@ -65,6 +65,8 @@ create a particular service. It is common for this workloads needing bare metal like performance, i.e. low latency and close to line speed performance. +.. include:: /common/numa-live-migration-warning.txt + .. feature_matrix:: feature-matrix-nfv.ini .. _matrix-hpc: diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index a3a82f0b6999..8ee66c67d519 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/nova/api/openstack/compute/migrate_server.py @@ -101,7 +101,11 @@ class MigrateServerController(wsgi.Controller): disk_over_commit = strutils.bool_from_string(disk_over_commit, strict=True) - instance = common.get_instance(self.compute_api, context, id) + # NOTE(stephenfin): we need 'numa_topology' because of the + # 'LiveMigrationTask._check_instance_has_no_numa' check in the + # conductor + instance = common.get_instance(self.compute_api, context, id, + expected_attrs=['numa_topology']) try: self.compute_api.live_migrate(context, instance, block_migration, disk_over_commit, host, force, diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index 3ed3f279f1fa..9836769d06e1 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -23,6 +23,7 @@ from nova import exception from nova.i18n import _ from nova import network from nova import objects +from nova.objects import fields as obj_fields from nova.scheduler import utils as scheduler_utils from nova import utils @@ -65,6 +66,7 @@ class LiveMigrationTask(base.TaskBase): def _execute(self): self._check_instance_is_active() + self._check_instance_has_no_numa() self._check_host_is_up(self.source) self._source_cn, self._held_allocations = ( @@ -151,6 +153,30 @@ class LiveMigrationTask(base.TaskBase): state=self.instance.power_state, method='live migrate') + def _check_instance_has_no_numa(self): + """Prevent live migrations of instances with NUMA topologies.""" + if not self.instance.numa_topology: + return + + # Only KVM (libvirt) supports NUMA topologies with CPU pinning; + # HyperV's vNUMA feature doesn't allow specific pinning + hypervisor_type = objects.ComputeNode.get_by_host_and_nodename( + self.context, self.source, self.instance.node).hypervisor_type + if hypervisor_type != obj_fields.HVType.KVM: + return + + msg = ('Instance has an associated NUMA topology. ' + 'Instance NUMA topologies, including related attributes ' + 'such as CPU pinning, huge page and emulator thread ' + 'pinning information, are not currently recalculated on ' + 'live migration. See bug #1289064 for more information.' + ) + + if CONF.workarounds.enable_numa_live_migration: + LOG.warning(msg, instance=self.instance) + else: + raise exception.MigrationPreCheckError(reason=msg) + def _check_host_is_up(self, host): service = objects.Service.get_by_compute_host(self.context, host) diff --git a/nova/conf/workarounds.py b/nova/conf/workarounds.py index e48f7d50ad0d..367248508218 100644 --- a/nova/conf/workarounds.py +++ b/nova/conf/workarounds.py @@ -184,6 +184,34 @@ deploying the new code during an upgrade. Related options: * ``[consoleauth]/token_ttl`` +"""), + + cfg.BoolOpt( + 'enable_numa_live_migration', + default=False, + help=""" +Enable live migration of instances with NUMA topologies. + +Live migration of instances with NUMA topologies is disabled by default +when using the libvirt driver. This includes live migration of instances with +CPU pinning or hugepages. CPU pinning and huge page information for such +instances is not currently re-calculated, as noted in `bug #1289064`_. This +means that if instances were already present on the destination host, the +migrated instance could be placed on the same dedicated cores as these +instances or use hugepages allocated for another instance. Alternately, if the +host platforms were not homogeneous, the instance could be assigned to +non-existent cores or be inadvertently split across host NUMA nodes. + +Despite these known issues, there may be cases where live migration is +necessary. By enabling this option, operators that are aware of the issues and +are willing to manually work around them can enable live migration support for +these instances. + +Related options: + +* ``compute_driver``: Only the libvirt driver is affected. + +.. _bug #1289064: https://bugs.launchpad.net/nova/+bug/1289064 """), ] diff --git a/nova/tests/unit/api/openstack/compute/admin_only_action_common.py b/nova/tests/unit/api/openstack/compute/admin_only_action_common.py index 95c23cc64a12..fdda030cfd5e 100644 --- a/nova/tests/unit/api/openstack/compute/admin_only_action_common.py +++ b/nova/tests/unit/api/openstack/compute/admin_only_action_common.py @@ -48,6 +48,10 @@ class CommonMixin(object): # Reset the mock. self.mock_get.reset_mock() + expected_attrs = None + if action == '_migrate_live': + expected_attrs = ['numa_topology'] + uuid = uuidutils.generate_uuid() self.mock_get.side_effect = exception.InstanceNotFound( instance_id=uuid) @@ -57,7 +61,7 @@ class CommonMixin(object): controller_function, self.req, uuid, body=body_map) self.mock_get.assert_called_once_with(self.context, uuid, - expected_attrs=None, + expected_attrs=expected_attrs, cell_down_support=False) def _test_action(self, action, body=None, method=None, @@ -65,6 +69,10 @@ class CommonMixin(object): # Reset the mock. self.mock_get.reset_mock() + expected_attrs = None + if action == '_migrate_live': + expected_attrs = ['numa_topology'] + if method is None: method = action.replace('_', '') compute_api_args_map = compute_api_args_map or {} @@ -85,13 +93,17 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=expected_attrs, cell_down_support=False) def _test_not_implemented_state(self, action, method=None): # Reset the mock. self.mock_get.reset_mock() + expected_attrs = None + if action == '_migrate_live': + expected_attrs = ['numa_topology'] + if method is None: method = action.replace('_', '') @@ -110,7 +122,7 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=expected_attrs, cell_down_support=False) def _test_invalid_state(self, action, method=None, body_map=None, @@ -119,6 +131,10 @@ class CommonMixin(object): # Reset the mock. self.mock_get.reset_mock() + expected_attrs = None + if action == '_migrate_live': + expected_attrs = ['numa_topology'] + if method is None: method = action.replace('_', '') if body_map is None: @@ -146,7 +162,7 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=expected_attrs, cell_down_support=False) def _test_locked_instance(self, action, method=None, body=None, @@ -154,6 +170,10 @@ class CommonMixin(object): # Reset the mock. self.mock_get.reset_mock() + expected_attrs = None + if action == '_migrate_live': + expected_attrs = ['numa_topology'] + if method is None: method = action.replace('_', '') @@ -173,7 +193,7 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=expected_attrs, cell_down_support=False) def _test_instance_not_found_in_compute_api(self, action, @@ -181,6 +201,10 @@ class CommonMixin(object): # Reset the mock. self.mock_get.reset_mock() + expected_attrs = None + if action == '_migrate_live': + expected_attrs = ['numa_topology'] + if method is None: method = action.replace('_', '') compute_api_args_map = compute_api_args_map or {} @@ -200,7 +224,7 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=expected_attrs, cell_down_support=False) diff --git a/nova/tests/unit/api/openstack/compute/test_migrate_server.py b/nova/tests/unit/api/openstack/compute/test_migrate_server.py index 69f42f821538..0ea4d092d97a 100644 --- a/nova/tests/unit/api/openstack/compute/test_migrate_server.py +++ b/nova/tests/unit/api/openstack/compute/test_migrate_server.py @@ -150,7 +150,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=['numa_topology'], cell_down_support=False) def test_migrate_live_enabled(self): @@ -228,7 +228,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): self.context, instance, False, self.disk_over_commit, 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=['numa_topology'], cell_down_support=False) def test_migrate_live_compute_service_unavailable(self): @@ -446,7 +446,7 @@ class MigrateServerTestsV234(MigrateServerTestsV230): self.context, instance, None, self.disk_over_commit, 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=['numa_topology'], cell_down_support=False) def test_migrate_live_unexpected_error(self): @@ -465,7 +465,7 @@ class MigrateServerTestsV234(MigrateServerTestsV230): self.context, instance, None, self.disk_over_commit, 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None, + expected_attrs=['numa_topology'], cell_down_support=False) diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index b50845696ba7..139d4c858803 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -19,6 +19,7 @@ from nova.compute import power_state from nova.compute import rpcapi as compute_rpcapi from nova.compute import vm_states from nova.conductor.tasks import live_migrate +from nova import context as nova_context from nova import exception from nova.network import model as network_model from nova import objects @@ -39,7 +40,7 @@ fake_selection2 = objects.Selection(service_host="host2", nodename="node2", class LiveMigrationTaskTestCase(test.NoDBTestCase): def setUp(self): super(LiveMigrationTaskTestCase, self).setUp() - self.context = "context" + self.context = nova_context.get_admin_context() self.instance_host = "host" self.instance_uuid = uuids.instance self.instance_image = "image_ref" @@ -53,12 +54,14 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): self.instance = objects.Instance._from_db_object( self.context, objects.Instance(), db_instance) self.instance.system_metadata = {'image_hw_disk_bus': 'scsi'} + self.instance.numa_topology = None self.destination = "destination" self.block_migration = "bm" self.disk_over_commit = "doc" self.migration = objects.Migration() self.fake_spec = objects.RequestSpec() self._generate_task() + _p = mock.patch('nova.compute.utils.heal_reqspec_is_bfv') self.heal_reqspec_is_bfv_mock = _p.start() self.addCleanup(_p.stop) @@ -170,6 +173,45 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): self.assertRaises(exception.InstanceInvalidState, self.task._check_instance_is_active) + @mock.patch.object(objects.ComputeNode, 'get_by_host_and_nodename') + def test_check_instance_has_no_numa_passes_no_numa(self, mock_get): + self.flags(enable_numa_live_migration=False, group='workarounds') + self.task.instance.numa_topology = None + mock_get.return_value = objects.ComputeNode( + uuid=uuids.cn1, hypervisor_type='kvm') + self.task._check_instance_has_no_numa() + + @mock.patch.object(objects.ComputeNode, 'get_by_host_and_nodename') + def test_check_instance_has_no_numa_passes_non_kvm(self, mock_get): + self.flags(enable_numa_live_migration=False, group='workarounds') + self.task.instance.numa_topology = objects.InstanceNUMATopology( + cells=[objects.InstanceNUMACell(id=0, cpuset=set([0]), + memory=1024)]) + mock_get.return_value = objects.ComputeNode( + uuid=uuids.cn1, hypervisor_type='xen') + self.task._check_instance_has_no_numa() + + @mock.patch.object(objects.ComputeNode, 'get_by_host_and_nodename') + def test_check_instance_has_no_numa_passes_workaround(self, mock_get): + self.flags(enable_numa_live_migration=True, group='workarounds') + self.task.instance.numa_topology = objects.InstanceNUMATopology( + cells=[objects.InstanceNUMACell(id=0, cpuset=set([0]), + memory=1024)]) + mock_get.return_value = objects.ComputeNode( + uuid=uuids.cn1, hypervisor_type='kvm') + self.task._check_instance_has_no_numa() + + @mock.patch.object(objects.ComputeNode, 'get_by_host_and_nodename') + def test_check_instance_has_no_numa_fails(self, mock_get): + self.flags(enable_numa_live_migration=False, group='workarounds') + mock_get.return_value = objects.ComputeNode( + uuid=uuids.cn1, hypervisor_type='kvm') + self.task.instance.numa_topology = objects.InstanceNUMATopology( + cells=[objects.InstanceNUMACell(id=0, cpuset=set([0]), + memory=1024)]) + self.assertRaises(exception.MigrationPreCheckError, + self.task._check_instance_has_no_numa) + @mock.patch.object(objects.Service, 'get_by_compute_host') @mock.patch.object(servicegroup.API, 'service_is_up') def test_check_instance_host_is_up(self, mock_is_up, mock_get): diff --git a/releasenotes/notes/disable-live-migration-with-numa-bc710a1bcde25957.yaml b/releasenotes/notes/disable-live-migration-with-numa-bc710a1bcde25957.yaml new file mode 100644 index 000000000000..4297c186f7f3 --- /dev/null +++ b/releasenotes/notes/disable-live-migration-with-numa-bc710a1bcde25957.yaml @@ -0,0 +1,25 @@ +--- +upgrade: + - | + Live migration of instances with NUMA topologies is now disabled by default + when using the libvirt driver. This includes live migration of instances + with CPU pinning or hugepages. CPU pinning and huge page information for + such instances is not currently re-calculated, as noted in `bug #1289064`_. + This means that if instances were already present on the destination host, + the migrated instance could be placed on the same dedicated cores as these + instances or use hugepages allocated for another instance. Alternately, if + the host platforms were not homogeneous, the instance could be assigned to + non-existent cores or be inadvertently split across host NUMA nodes. + + The `long term solution`_ to these issues is to recalculate the XML on the + destination node. When this work is completed, the restriction on live + migration with NUMA topologies will be lifted. + + For operators that are aware of the issues and are able to manually work + around them, the ``[workarounds] enable_numa_live_migration`` option can + be used to allow the broken behavior. + + For more information, refer to `bug #1289064`_. + + .. _bug #1289064: https://bugs.launchpad.net/nova/+bug/1289064 + .. _long term solution: https://blueprints.launchpad.net/nova/+spec/numa-aware-live-migration