Skip to content

Commit

Permalink
Pass allocations to virt drivers when resizing
Browse files Browse the repository at this point in the history
Some features like vGPUs require to have allocations to be checked locally
by the virt driver but resize (and cold migrate) was not passing them.

This change just gives the allocations to the target node when the resize
is done by adding the allocations to the finish_migration() interface.

Related-Bug: #1778563
Change-Id: Ifd76289964d513e9544544e5787f5f8999589475
  • Loading branch information
sbauza committed Apr 10, 2020
1 parent b6f3d39 commit 580eedb
Show file tree
Hide file tree
Showing 12 changed files with 48 additions and 23 deletions.
8 changes: 8 additions & 0 deletions nova/compute/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -5779,11 +5779,19 @@ def _finish_resize(self, context, instance, migration, disk_info,
# automatically power on the instance after it's migrated
power_on = old_vm_state != vm_states.STOPPED

# NOTE(sbauza): During a migration, the original allocation is against
# the migration UUID while the target allocation (for the destination
# node) is related to the instance UUID, so here we need to pass the
# new ones.
allocations = self.reportclient.get_allocs_for_consumer(
context, instance.uuid)['allocations']

try:
self.driver.finish_migration(context, migration, instance,
disk_info,
network_info,
image_meta, resize_instance,
allocations,
block_device_info, power_on)
except Exception:
# Note that we do not rollback port bindings to the source host
Expand Down
21 changes: 15 additions & 6 deletions nova/tests/unit/compute/test_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,17 @@ def fake_get_nw_info(cls, ctxt, instance, *args, **kwargs):
# Just to make long lines short
self.rt = self.compute.rt

self.mock_get_allocs = self.useFixture(
self.mock_get_allocations = self.useFixture(
fixtures.fixtures.MockPatch(
'nova.scheduler.client.report.SchedulerReportClient.'
'get_allocations_for_consumer')).mock
self.mock_get_allocs.return_value = {}
self.mock_get_allocations.return_value = {}

self.mock_get_allocs = self.useFixture(
fixtures.fixtures.MockPatch(
'nova.scheduler.client.report.SchedulerReportClient.'
'get_allocs_for_consumer')).mock
self.mock_get_allocs.return_value = {'allocations': {}}

def tearDown(self):
ctxt = context.get_admin_context()
Expand Down Expand Up @@ -4928,7 +4934,7 @@ def _instance_save1(expected_task_state=None):
# which makes this a resize
mock_virt_mig.assert_called_once_with(self.context, migration,
instance, disk_info, 'fake-nwinfo1',
test.MatchType(objects.ImageMeta), resize_instance,
test.MatchType(objects.ImageMeta), resize_instance, mock.ANY,
'fake-bdminfo', power_on)
mock_get_blk.assert_called_once_with(self.context, instance,
refresh_conn_info=True,
Expand All @@ -4943,6 +4949,8 @@ def _instance_save1(expected_task_state=None):
mock_get_vol_connector.return_value, '/dev/vdb')
mock_attachment_complete.assert_called_once_with(
self.context, uuids.attachment_id)
self.mock_get_allocs.assert_called_once_with(self.context,
instance.uuid)

def test_finish_resize_from_active(self):
self._test_finish_resize(power_on=True)
Expand Down Expand Up @@ -12763,7 +12771,8 @@ def _test_rebuild(mock_context, mock_setup_instance_network_on_host,
mock_setup_instance_network_on_host.assert_called_once_with(
ctxt, self.inst, self.inst.host, migration,
provider_mappings=mock.sentinel.mapping)
self.mock_get_allocs.assert_called_once_with(ctxt, self.inst.uuid)
self.mock_get_allocations.assert_called_once_with(ctxt,
self.inst.uuid)

mock_update_pci_req.assert_called_once_with(
ctxt, self.compute.reportclient, self.inst,
Expand Down Expand Up @@ -13109,8 +13118,8 @@ def _spawn(self, context, instance, image_meta, injected_files,
admin_password, allocations, nw_info, block_device_info,
db_api=None):
self.assertEqual(self.expected, injected_files)
self.assertEqual(self.mock_get_allocs.return_value, allocations)
self.mock_get_allocs.assert_called_once_with(instance.uuid)
self.assertEqual(self.mock_get_allocations.return_value, allocations)
self.mock_get_allocations.assert_called_once_with(instance.uuid)

def _test(self, injected_files, decoded_files):
self.expected = decoded_files
Expand Down
8 changes: 4 additions & 4 deletions nova/tests/unit/compute/test_shelve.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,8 @@ def check_save(expected_task_state=None):
test.MatchType(objects.ImageMeta), injected_files=[],
admin_password=None, allocations={}, network_info=[],
block_device_info='fake_bdm')
self.mock_get_allocs.assert_called_once_with(self.context,
instance.uuid)
self.mock_get_allocations.assert_called_once_with(self.context,
instance.uuid)
mock_get_power_state.assert_called_once_with(self.context, instance)

self.assertNotIn('shelved_at', instance.system_metadata)
Expand Down Expand Up @@ -470,8 +470,8 @@ def check_save(expected_task_state=None):
test.MatchType(objects.ImageMeta),
injected_files=[], admin_password=None,
allocations={}, network_info=[], block_device_info='fake_bdm')
self.mock_get_allocs.assert_called_once_with(self.context,
instance.uuid)
self.mock_get_allocations.assert_called_once_with(self.context,
instance.uuid)
mock_get_power_state.assert_called_once_with(self.context, instance)

@mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid')
Expand Down
3 changes: 2 additions & 1 deletion nova/tests/unit/virt/hyperv/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,8 @@ def test_finish_migration(self):
mock.sentinel.context, mock.sentinel.migration,
mock.sentinel.instance, mock.sentinel.disk_info,
mock.sentinel.network_info, mock.sentinel.image_meta,
mock.sentinel.resize_instance, mock.sentinel.block_device_info,
mock.sentinel.resize_instance, mock.sentinel.allocations,
mock.sentinel.block_device_info,
mock.sentinel.power_on)

self.driver._migrationops.finish_migration.assert_called_once_with(
Expand Down
2 changes: 1 addition & 1 deletion nova/tests/unit/virt/libvirt/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -21443,7 +21443,7 @@ def _test_finish_migration(self, mock_instance_metadata,
self.drvr.finish_migration(
context.get_admin_context(), migration, instance,
disk_info_text, [], image_meta,
resize_instance, bdi, power_on)
resize_instance, mock.ANY, bdi, power_on)

# Assert that we converted the root, ephemeral, and swap disks
instance_path = libvirt_utils.get_instance_path(instance)
Expand Down
12 changes: 8 additions & 4 deletions nova/tests/unit/virt/xenapi/test_xenapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1852,7 +1852,8 @@ def fake_finish_revert_migration(*args, **kwargs):
conn.finish_migration(self.context, self.migration, instance,
dict(base_copy=base_uuid, cow=cow_uuid),
network_info, image_meta, resize_instance=True,
block_device_info=None, power_on=power_on)
allocations={}, block_device_info=None,
power_on=power_on)
self.assertTrue(self.called)
self.assertEqual(self.fake_vm_start_called, power_on)

Expand Down Expand Up @@ -1893,7 +1894,8 @@ def fake_vdi_resize(*args, **kwargs):
conn.finish_migration(self.context, self.migration, instance,
dict(base_copy='hurr', cow='durr'),
network_info, image_meta, resize_instance=True,
block_device_info=None, power_on=power_on)
allocations={}, block_device_info=None,
power_on=power_on)
self.assertTrue(self.called)
self.assertEqual(self.fake_vm_start_called, power_on)

Expand Down Expand Up @@ -1923,7 +1925,8 @@ def fake_vdi_resize(*args, **kwargs):
{'id': instance['image_ref'], 'disk_format': 'vhd'})
conn.finish_migration(self.context, self.migration, instance,
dict(base_copy='hurr', cow='durr'),
network_info, image_meta, resize_instance=True)
network_info, image_meta, resize_instance=True,
allocations={})

def test_finish_migrate_no_resize_vdi(self):
instance = create_instance_with_system_metadata(self.context,
Expand All @@ -1942,7 +1945,8 @@ def fake_vdi_resize(*args, **kwargs):
{'id': instance['image_ref'], 'disk_format': 'vhd'})
conn.finish_migration(self.context, self.migration, instance,
dict(base_copy='hurr', cow='durr'),
network_info, image_meta, resize_instance=False)
network_info, image_meta, resize_instance=False,
allocations={})

@stub_vm_utils_with_vdi_attached
def test_migrate_too_many_partitions_no_resize_down(self):
Expand Down
5 changes: 4 additions & 1 deletion nova/virt/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ def post_interrupted_snapshot_cleanup(self, context, instance):

def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
block_device_info=None, power_on=True):
allocations, block_device_info=None, power_on=True):
"""Completes a resize/migration.
:param context: the context for the migration/resize
Expand All @@ -742,6 +742,9 @@ def finish_migration(self, context, migration, instance, disk_info,
The metadata of the image of the instance.
:param resize_instance: True if the instance is being resized,
False otherwise
:param allocations: Information about resources allocated to the
instance via placement, of the form returned by
SchedulerReportClient.get_allocs_for_consumer.
:param block_device_info: instance volume block device info
:param power_on: True if the instance should be powered on, False
otherwise
Expand Down
4 changes: 2 additions & 2 deletions nova/virt/fake.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,8 @@ def check_can_live_migrate_source(self, context, instance,

def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
block_device_info=None, power_on=True):
injected_files = admin_password = allocations = None
allocations, block_device_info=None, power_on=True):
injected_files = admin_password = None
# Finish migration is just like spawning the guest on a destination
# host during resize/cold migrate, so re-use the spawn() fake to
# claim resources and track the instance on this "hypervisor".
Expand Down
2 changes: 1 addition & 1 deletion nova/virt/hyperv/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def finish_revert_migration(self, context, instance, network_info,

def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
block_device_info=None, power_on=True):
allocations, block_device_info=None, power_on=True):
self._migrationops.finish_migration(context, migration, instance,
disk_info, network_info,
image_meta, resize_instance,
Expand Down
2 changes: 1 addition & 1 deletion nova/virt/libvirt/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -10039,7 +10039,7 @@ def _disk_raw_to_qcow2(path):

def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
block_device_info=None, power_on=True):
allocations, block_device_info=None, power_on=True):
LOG.debug("Starting finish_migration", instance=instance)

block_disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type,
Expand Down
2 changes: 1 addition & 1 deletion nova/virt/vmwareapi/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ def finish_revert_migration(self, context, instance, network_info,

def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
block_device_info=None, power_on=True):
allocations, block_device_info=None, power_on=True):
"""Completes a resize, turning on the migrated instance."""
self._vmops.finish_migration(context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
Expand Down
2 changes: 1 addition & 1 deletion nova/virt/xenapi/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def finish_revert_migration(self, context, instance, network_info,

def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
block_device_info=None, power_on=True):
allocations, block_device_info=None, power_on=True):
"""Completes a resize, turning on the migrated instance."""
self._vmops.finish_migration(context, migration, instance, disk_info,
network_info, image_meta, resize_instance,
Expand Down

0 comments on commit 580eedb

Please sign in to comment.