From 2363fd18ba18215640bc9bca2b7f807233538d28 Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Wed, 14 Jan 2015 15:18:21 -0500 Subject: [PATCH] Xenapi: Attempt clean shutdown when deleting instance A guest should be given the chance to shut down cleanly even when being deleted in case there is a volume attached. The volume might be re-used later so we should minimize the risk of filesystem corruption. Change-Id: I449fa7f2ab0ef34d0d0d4e2bd648c628737faaa8 --- nova/tests/unit/virt/xenapi/test_vmops.py | 40 +++++++++++++++++++++++ nova/virt/xenapi/vmops.py | 20 ++++++++---- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/nova/tests/unit/virt/xenapi/test_vmops.py b/nova/tests/unit/virt/xenapi/test_vmops.py index 8d0bd82de43c..4ca80df12f9f 100644 --- a/nova/tests/unit/virt/xenapi/test_vmops.py +++ b/nova/tests/unit/virt/xenapi/test_vmops.py @@ -63,6 +63,8 @@ class VMOpsTestCase(VMOpsTestBase): def setUp(self): super(VMOpsTestCase, self).setUp() self._setup_mock_vmops() + self.context = context.RequestContext('user', 'project') + self.instance = fake_instance.fake_instance_obj(self.context) def _setup_mock_vmops(self, product_brand=None, product_version=None): self._session = self._get_mock_session(product_brand, product_version) @@ -141,6 +143,44 @@ class VMOpsTestCase(VMOpsTestBase): self.assertRaises(exception.InstanceNotFound, self._vmops._get_vm_opaque_ref, instance) + @mock.patch.object(vm_utils, 'destroy_vm') + @mock.patch.object(vm_utils, 'clean_shutdown_vm') + @mock.patch.object(vm_utils, 'hard_shutdown_vm') + def test_clean_shutdown_no_bdm_on_destroy(self, hard_shutdown_vm, + clean_shutdown_vm, destroy_vm): + vm_ref = 'vm_ref' + self._vmops._destroy(self.instance, vm_ref, destroy_disks=False) + hard_shutdown_vm.assert_called_once_with(self._vmops._session, + self.instance, vm_ref) + self.assertEqual(0, clean_shutdown_vm.call_count) + + @mock.patch.object(vm_utils, 'destroy_vm') + @mock.patch.object(vm_utils, 'clean_shutdown_vm') + @mock.patch.object(vm_utils, 'hard_shutdown_vm') + def test_clean_shutdown_with_bdm_on_destroy(self, hard_shutdown_vm, + clean_shutdown_vm, destroy_vm): + vm_ref = 'vm_ref' + block_device_info = {'block_device_mapping': ['fake']} + self._vmops._destroy(self.instance, vm_ref, destroy_disks=False, + block_device_info=block_device_info) + clean_shutdown_vm.assert_called_once_with(self._vmops._session, + self.instance, vm_ref) + self.assertEqual(0, hard_shutdown_vm.call_count) + + @mock.patch.object(vm_utils, 'destroy_vm') + @mock.patch.object(vm_utils, 'clean_shutdown_vm', return_value=False) + @mock.patch.object(vm_utils, 'hard_shutdown_vm') + def test_clean_shutdown_with_bdm_failed_on_destroy(self, hard_shutdown_vm, + clean_shutdown_vm, destroy_vm): + vm_ref = 'vm_ref' + block_device_info = {'block_device_mapping': ['fake']} + self._vmops._destroy(self.instance, vm_ref, destroy_disks=False, + block_device_info=block_device_info) + clean_shutdown_vm.assert_called_once_with(self._vmops._session, + self.instance, vm_ref) + hard_shutdown_vm.assert_called_once_with(self._vmops._session, + self.instance, vm_ref) + class InjectAutoDiskConfigTestCase(VMOpsTestBase): def test_inject_auto_disk_config_when_present(self): diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 8c1c3140bc76..39a72d81e6ba 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -1472,11 +1472,11 @@ class VMOps(object): if rescue_vm_ref: self._destroy_rescue_instance(rescue_vm_ref, vm_ref) - # NOTE(sirp): `block_device_info` is not used, information about which - # volumes should be detached is determined by the - # VBD.other_config['osvol'] attribute - # NOTE(alaski): `block_device_info` is used to determine if there's a - # volume still attached if the VM is not present. + # NOTE(sirp): information about which volumes should be detached is + # determined by the VBD.other_config['osvol'] attribute + # NOTE(alaski): `block_device_info` is used to efficiently determine if + # there's a volume attached, or which volumes to cleanup if there is + # no VM present. return self._destroy(instance, vm_ref, network_info=network_info, destroy_disks=destroy_disks, block_device_info=block_device_info) @@ -1524,7 +1524,15 @@ class VMOps(object): volume_id, instance=instance) return - vm_utils.hard_shutdown_vm(self._session, instance, vm_ref) + # NOTE(alaski): Attempt clean shutdown first if there's an attached + # volume to reduce the risk of corruption. + if block_device_info and block_device_info['block_device_mapping']: + if not vm_utils.clean_shutdown_vm(self._session, instance, vm_ref): + LOG.debug("Clean shutdown did not complete successfully, " + "trying hard shutdown.", instance=instance) + vm_utils.hard_shutdown_vm(self._session, instance, vm_ref) + else: + vm_utils.hard_shutdown_vm(self._session, instance, vm_ref) if destroy_disks: self._volumeops.detach_all(vm_ref)