From e2e9f415a977c725835f040d3c0f1c4cef9ad39c Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Wed, 11 Sep 2019 19:24:05 +0100 Subject: [PATCH] libvirt: Ignore volume exceptions during post_live_migration Previously errors while disconnecting volumes from the source host during post_live_migration within LibvirtDriver would result in the overall failure of the migration. This would also mean that while the instance would be running on the destination it would still be listed as running on the source within the db. This change simply ignores any exceptions raised while attempting to disconnect volumes on the source. These errors can be safely ignored as they will have no impact on the running instance on the destination. In the future Nova could wire up the force and ignore_errors kwargs when calling down into the associated os-brick connectors to help avoid this. Closes-Bug: #1843639 Change-Id: Ieff5243854321ec40f642845e87a0faecaca8721 (cherry picked from commit ac68cffd43a2f5103c28a2d4b31e087c3f5c24b9) (cherry picked from commit ff36b6d97ff289ddc34d7776f6a9141b09eb3ad9) (cherry picked from commit 022ea2819425b5ab3001791455dda36ed638c22d) (cherry picked from commit a07c612ea6fb8553effecef7454caa179589e916) --- nova/tests/unit/virt/libvirt/test_driver.py | 27 +++++++++++++++++++++ nova/virt/libvirt/driver.py | 22 ++++++++++++----- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index b65a8bfb5575..acd95fc8d76c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -11776,6 +11776,33 @@ class LibvirtConnTestCase(test.NoDBTestCase, _test() + @mock.patch.object(libvirt_driver.LibvirtDriver, '_disconnect_volume') + @mock.patch.object(driver, 'block_device_info_get_mapping') + def test_post_live_migration_exception_swallowed(self, mock_get_bdm, + mock_disconnect_volume): + vol_1_conn_info = {'data': {'volume_id': uuids.vol_1_id}} + vol_2_conn_info = {'data': {'volume_id': uuids.vol_2_id}} + mock_get_bdm.return_value = [{'connection_info': vol_1_conn_info}, + {'connection_info': vol_2_conn_info}] + + # Raise an exception with the first call to disconnect_volume + mock_disconnect_volume.side_effect = [test.TestingException, None] + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr.post_live_migration(mock.sentinel.ctxt, mock.sentinel.instance, + mock.sentinel.bdi) + + # Assert disconnect_volume is called twice despite the exception + mock_disconnect_volume.assert_has_calls([ + mock.call(mock.sentinel.ctxt, vol_1_conn_info, + mock.sentinel.instance), + mock.call(mock.sentinel.ctxt, vol_2_conn_info, + mock.sentinel.instance)]) + + # Assert that we log the failure to disconnect the first volume + self.assertIn("Ignoring exception while attempting to disconnect " + "volume %s" % uuids.vol_1_id, self.stdlog.logger.output) + @mock.patch('os.stat') @mock.patch('os.path.getsize') @mock.patch('nova.virt.disk.api.get_disk_info') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index dd3e1f6b5ee6..cae981041fe2 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7804,15 +7804,25 @@ class LibvirtDriver(driver.ComputeDriver): def post_live_migration(self, context, instance, block_device_info, migrate_data=None): - # Disconnect from volume server + # NOTE(mdbooth): The block_device_info we were passed was initialized + # with BDMs from the source host before they were updated to point to + # the destination. We can safely use this to disconnect the source + # without re-fetching. block_device_mapping = driver.block_device_info_get_mapping( block_device_info) + for vol in block_device_mapping: - # NOTE(mdbooth): The block_device_info we were passed was - # initialized with BDMs from the source host before they were - # updated to point to the destination. We can safely use this to - # disconnect the source without re-fetching. - self._disconnect_volume(context, vol['connection_info'], instance) + connection_info = vol['connection_info'] + # NOTE(lyarwood): Ignore exceptions here to avoid the instance + # being left in an ERROR state and still marked on the source. + try: + self._disconnect_volume(context, connection_info, instance) + except Exception: + volume_id = driver_block_device.get_volume_id(connection_info) + LOG.exception("Ignoring exception while attempting to " + "disconnect volume %s from the source host " + "during post_live_migration", volume_id, + instance=instance) def post_live_migration_at_source(self, context, instance, network_info): """Unplug VIFs from networks at source.