openstack
/
nova-powervm
Code Issues Proposed changes

Remove delete vopt if nothing to delete 

This change set optimizes the delete of the virtual optical a bit.
If the user has choosen not to use config drive (which is what uses
vopt) the delete of the media would still call an update to the media
repo.  This wasn't that expensive but didn't need to be called in this
scenario.

Change-Id: Ic9e7ba2aad7887ffaeec727e799996c91d593c5a
This commit is contained in:
Drew Thorstensen 2016-07-07 13:21:07 -04:00
parent 66a9c171fc
commit 273a10381c
4 changed files with 27 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
nova_powervm/tests/virt/powervm/test_driver.py
View File

@ -899,14 +899,17 @@ class TestPowerVMDriver(test.TestCase):
@mock.patch('nova_powervm.virt.powervm.media.ConfigDrivePowerVM.'
'dlt_vopt')
@mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid')
@mock.patch('nova.virt.configdrive.required_by')
@mock.patch('nova.objects.flavor.Flavor.get_by_id')
@mock.patch('nova_powervm.virt.powervm.slot.build_slot_mgr')
def test_destroy_internal(self, mock_bld_slot_mgr, mock_get_flv,
mock_pvmuuid, mock_dlt_vopt, mock_pwroff,
mock_dlt, mock_boot_from_vol, mock_unplug_vifs):
def test_destroy_internal(
self, mock_bld_slot_mgr, mock_get_flv, mock_cfg, mock_pvmuuid,
mock_dlt_vopt, mock_pwroff, mock_dlt, mock_boot_from_vol,
mock_unplug_vifs):
"""Validates the basic PowerVM destroy."""
# NVRAM Manager
self.drv.nvram_mgr = mock.Mock()
mock_cfg.return_value = True
# BDMs
mock_bdms = self._fake_bdms()
@ -1050,13 +1053,13 @@ class TestPowerVMDriver(test.TestCase):
@mock.patch('nova_powervm.virt.powervm.media.ConfigDrivePowerVM.'
'dlt_vopt')
@mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid')
@mock.patch('nova.virt.configdrive.required_by')
@mock.patch('nova.objects.flavor.Flavor.get_by_id')
@mock.patch('nova_powervm.virt.powervm.slot.build_slot_mgr')
def test_destroy_internal_no_nvram_cleanup(self, mock_bld_slot_mgr,
mock_get_flv, mock_pvmuuid,
mock_dlt_vopt, mock_pwroff,
mock_dlt, mock_boot_from_vol,
mock_unplug_vifs):
def test_destroy_internal_no_nvram_cleanup(
self, mock_bld_slot_mgr, mock_get_flv, mock_cfg, mock_pvmuuid,
mock_dlt_vopt, mock_pwroff, mock_dlt, mock_boot_from_vol,
mock_unplug_vifs):
"""Validates the basic PowerVM destroy, without NVRAM cleanup.
Used to validate the behavior when destroying evacuated instances.
@ -1065,6 +1068,7 @@ class TestPowerVMDriver(test.TestCase):
# NVRAM Manager
self.drv.nvram_mgr = mock.Mock()
self.inst.host = 'other'
mock_cfg.return_value = True
# BDMs
mock_bdms = self._fake_bdms()
@ -1207,12 +1211,14 @@ class TestPowerVMDriver(test.TestCase):
@mock.patch('nova_powervm.virt.powervm.vm.power_off')
@mock.patch('nova_powervm.virt.powervm.media.ConfigDrivePowerVM.'
'dlt_vopt')
@mock.patch('nova.virt.configdrive.required_by')
@mock.patch('nova.objects.flavor.Flavor.get_by_id')
def test_destroy_rollback(self, mock_get_flv, mock_dlt_vopt,
def test_destroy_rollback(self, mock_get_flv, mock_cfg, mock_dlt_vopt,
mock_pwroff, mock_dlt, mock_unplug_vifs):
"""Validates the basic PowerVM destroy rollback mechanism works."""
# Set up the mocks to the tasks.
mock_get_flv.return_value = self.inst.get_flavor()
mock_cfg.return_value = True
# BDMs
mock_bdms = self._fake_bdms()

8
nova_powervm/tests/virt/powervm/test_media.py
View File

@ -167,21 +167,22 @@ class TestConfigDrivePowerVM(test.TestCase):
'add_dlt_vopt_tasks')
@mock.patch('pypowervm.wrappers.virtual_io_server.VIOS.wrap',
new=mock.MagicMock())
@mock.patch('pypowervm.tasks.scsi_mapper.find_maps')
@mock.patch('pypowervm.utils.transaction.FeedTask')
@mock.patch('pypowervm.utils.transaction.FeedTask.execute')
def test_dlt_vopt_no_map(self, mock_execute, mock_class_feed_task,
mock_add_dlt_vopt_tasks):
mock_add_dlt_vopt_tasks, mock_find_maps):
# Init objects to test with
mock_feed_task = mock.MagicMock()
mock_class_feed_task.return_value = mock_feed_task
mock_find_maps.return_value = []
# Invoke the operation
cfg_dr = m.ConfigDrivePowerVM(self.apt, 'fake_host')
cfg_dr.dlt_vopt('2', remove_mappings=False)
# Verify expected methods were called
mock_add_dlt_vopt_tasks.assert_called_with(
'2', mock_feed_task, remove_mappings=False)
mock_add_dlt_vopt_tasks.assert_not_called()
self.assertTrue(mock_feed_task.execute.called)
@mock.patch('nova_powervm.virt.powervm.vm.get_vm_id',
@ -194,6 +195,7 @@ class TestConfigDrivePowerVM(test.TestCase):
stg_ftsk = mock.MagicMock()
cfg_dr.vios_uuid = 'vios_uuid'
lpar_uuid = 'lpar_uuid'
mock_find_maps.return_value = [mock.Mock(backing_storage='stor')]
# Run
cfg_dr.add_dlt_vopt_tasks(lpar_uuid, stg_ftsk)

6
nova_powervm/virt/powervm/driver.py
View File

@ -588,8 +588,10 @@ class PowerVMDriver(driver.ComputeDriver):
# Add the disconnect/deletion of the vOpt to the transaction
# manager.
flow.add(tf_stg.DeleteVOpt(self.adapter, self.host_uuid, instance,
pvm_inst_uuid, stg_ftsk=stg_ftsk))
if configdrive.required_by(instance):
flow.add(tf_stg.DeleteVOpt(
self.adapter, self.host_uuid, instance, pvm_inst_uuid,
stg_ftsk=stg_ftsk))
# Determine if there are volumes to disconnect. If so, remove each
# volume (within the transaction manager)

4
nova_powervm/virt/powervm/media.py
View File

@ -302,4 +302,6 @@ class ConfigDrivePowerVM(object):
child_id=self.vg_uuid)
tsk_stg.rm_vg_storage(pvm_stg.VG.wrap(vg_rsp), vopts=media_elems)
stg_ftsk.add_post_execute(task.FunctorTask(rm_vopt))
# Don't add this task if there is no media to delete (eg. config drive)
if media_elems:
stg_ftsk.add_post_execute(task.FunctorTask(rm_vopt))