openstack/nova
Code Issues Proposed changes

Xenapi: Attempt clean shutdown when deleting instance

Browse Source 
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
This commit is contained in:
Andrew Laski 2015-01-14 15:18:21 -05:00
parent 1e41005f5a
commit 2363fd18ba
2 changed files with 54 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
nova/tests/unit/virt/xenapi/test_vmops.py
View File

@ -63,6 +63,8 @@ class VMOpsTestCase(VMOpsTestBase):
def setUp(self): def setUp(self):
super(VMOpsTestCase, self).setUp() super(VMOpsTestCase, self).setUp()
self._setup_mock_vmops() 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): def _setup_mock_vmops(self, product_brand=None, product_version=None):
self._session = self._get_mock_session(product_brand, product_version) self._session = self._get_mock_session(product_brand, product_version)
@ -141,6 +143,44 @@ class VMOpsTestCase(VMOpsTestBase):
self.assertRaises(exception.InstanceNotFound, self.assertRaises(exception.InstanceNotFound,
self._vmops._get_vm_opaque_ref, instance) 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): class InjectAutoDiskConfigTestCase(VMOpsTestBase):
def test_inject_auto_disk_config_when_present(self): def test_inject_auto_disk_config_when_present(self):

18
nova/virt/xenapi/vmops.py
View File

@ -1472,11 +1472,11 @@ class VMOps(object):
if rescue_vm_ref: if rescue_vm_ref:
self._destroy_rescue_instance(rescue_vm_ref, vm_ref) self._destroy_rescue_instance(rescue_vm_ref, vm_ref)
# NOTE(sirp): `block_device_info` is not used, information about which # NOTE(sirp): information about which volumes should be detached is
# volumes should be detached is determined by the # determined by the VBD.other_config['osvol'] attribute
# VBD.other_config['osvol'] attribute # NOTE(alaski): `block_device_info` is used to efficiently determine if
# NOTE(alaski): `block_device_info` is used to determine if there's a # there's a volume attached, or which volumes to cleanup if there is
# volume still attached if the VM is not present. # no VM present.
return self._destroy(instance, vm_ref, network_info=network_info, return self._destroy(instance, vm_ref, network_info=network_info,
destroy_disks=destroy_disks, destroy_disks=destroy_disks,
block_device_info=block_device_info) block_device_info=block_device_info)
@ -1524,6 +1524,14 @@ class VMOps(object):
volume_id, instance=instance) volume_id, instance=instance)
return return
# 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) vm_utils.hard_shutdown_vm(self._session, instance, vm_ref)
if destroy_disks: if destroy_disks: