Refactor _vol_drv_iter to use bdm.is_volume
In [1] it was pointed out [2] that the way we were attempting to determine whether a BDM was a volume wasn't correct. The right way is to consult its is_volume property. But that property didn't exist until [3]. Since that's now merged, this change set refactors to use that property. In the process, we remove the unused 'context' parameter from _vol_drv_iter. [1] https://review.openstack.org/#/c/526094/ [2] https://review.openstack.org/#/c/526094/45/nova/virt/powervm/driver.py@559 [3] https://review.openstack.org/#/c/564017/ Change-Id: I9f33a85cfd2e76d72bb6664eb247bdbf49b14079
This commit is contained in:
parent
af0e814d62
commit
db6a646cd5
|
@ -357,9 +357,8 @@ class TestPowerVMDriver(test.NoDBTestCase):
|
||||||
|
|
||||||
# _vol_drv_iter not called from spawn because not recreate; but still
|
# _vol_drv_iter not called from spawn because not recreate; but still
|
||||||
# called from _add_volume_connection_tasks.
|
# called from _add_volume_connection_tasks.
|
||||||
mock_vdi.assert_has_calls([mock.call(
|
mock_vdi.assert_called_once_with(
|
||||||
'context', self.inst, [],
|
self.inst, [], stg_ftsk=self.build_tx_feed.return_value)
|
||||||
stg_ftsk=self.build_tx_feed.return_value)])
|
|
||||||
# Assert the correct tasks were called
|
# Assert the correct tasks were called
|
||||||
self.assertTrue(mock_plug_vifs.called)
|
self.assertTrue(mock_plug_vifs.called)
|
||||||
self.assertTrue(mock_plug_mgmt_vif.called)
|
self.assertTrue(mock_plug_mgmt_vif.called)
|
||||||
|
@ -621,8 +620,7 @@ class TestPowerVMDriver(test.NoDBTestCase):
|
||||||
# _add_volume_connection_tasks.
|
# _add_volume_connection_tasks.
|
||||||
# TODO(IBM): Find a way to make the call just once. Unless it's cheap.
|
# TODO(IBM): Find a way to make the call just once. Unless it's cheap.
|
||||||
mock_vol_drv_iter.assert_has_calls([mock.call(
|
mock_vol_drv_iter.assert_has_calls([mock.call(
|
||||||
'context', self.inst, [],
|
self.inst, [], stg_ftsk=self.build_tx_feed.return_value)] * 2)
|
||||||
stg_ftsk=self.build_tx_feed.return_value)] * 2)
|
|
||||||
mock_build_slot_mgr.assert_called_once_with(
|
mock_build_slot_mgr.assert_called_once_with(
|
||||||
self.inst, self.drv.store_api, adapter=self.drv.adapter,
|
self.inst, self.drv.store_api, adapter=self.drv.adapter,
|
||||||
vol_drv_iter=mock_vol_drv_iter.return_value)
|
vol_drv_iter=mock_vol_drv_iter.return_value)
|
||||||
|
@ -927,7 +925,7 @@ class TestPowerVMDriver(test.NoDBTestCase):
|
||||||
return_value=vals) as mock_vdi:
|
return_value=vals) as mock_vdi:
|
||||||
self.drv._add_volume_connection_tasks(
|
self.drv._add_volume_connection_tasks(
|
||||||
'context', 'instance', 'bdms', flow, 'stg_ftsk', 'slot_mgr')
|
'context', 'instance', 'bdms', flow, 'stg_ftsk', 'slot_mgr')
|
||||||
mock_vdi.assert_called_once_with('context', 'instance', 'bdms',
|
mock_vdi.assert_called_once_with('instance', 'bdms',
|
||||||
stg_ftsk='stg_ftsk')
|
stg_ftsk='stg_ftsk')
|
||||||
mock_conn_vol.assert_has_calls([mock.call(vol_drv1, 'slot_mgr'),
|
mock_conn_vol.assert_has_calls([mock.call(vol_drv1, 'slot_mgr'),
|
||||||
mock.call(vol_drv2, 'slot_mgr')])
|
mock.call(vol_drv2, 'slot_mgr')])
|
||||||
|
@ -948,7 +946,7 @@ class TestPowerVMDriver(test.NoDBTestCase):
|
||||||
return_value=vals) as mock_vdi:
|
return_value=vals) as mock_vdi:
|
||||||
self.drv._add_volume_disconnection_tasks(
|
self.drv._add_volume_disconnection_tasks(
|
||||||
'context', 'instance', 'bdms', flow, 'stg_ftsk', 'slot_mgr')
|
'context', 'instance', 'bdms', flow, 'stg_ftsk', 'slot_mgr')
|
||||||
mock_vdi.assert_called_once_with('context', 'instance', 'bdms',
|
mock_vdi.assert_called_once_with('instance', 'bdms',
|
||||||
stg_ftsk='stg_ftsk')
|
stg_ftsk='stg_ftsk')
|
||||||
mock_disconn_vol.assert_has_calls([mock.call(vol_drv1, 'slot_mgr'),
|
mock_disconn_vol.assert_has_calls([mock.call(vol_drv1, 'slot_mgr'),
|
||||||
mock.call(vol_drv2, 'slot_mgr')])
|
mock.call(vol_drv2, 'slot_mgr')])
|
||||||
|
@ -1965,9 +1963,8 @@ class TestPowerVMDriver(test.NoDBTestCase):
|
||||||
# Patch so we get the same mock back each time.
|
# Patch so we get the same mock back each time.
|
||||||
with mock.patch('nova_powervm.virt.powervm.volume.'
|
with mock.patch('nova_powervm.virt.powervm.volume.'
|
||||||
'build_volume_driver', return_value=vol_adpt):
|
'build_volume_driver', return_value=vol_adpt):
|
||||||
return [
|
return [(bdm, vol_drv) for bdm, vol_drv in
|
||||||
(bdm, vol_drv) for bdm, vol_drv in self.drv._vol_drv_iter(
|
self.drv._vol_drv_iter(self.inst, bdms)]
|
||||||
'context', self.inst, bdms)]
|
|
||||||
|
|
||||||
results = _get_results(bdms)
|
results = _get_results(bdms)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
|
|
|
@ -392,8 +392,7 @@ class PowerVMDriver(driver.ComputeDriver):
|
||||||
# Build the PowerVM Slot lookup map. Only the recreate action needs
|
# Build the PowerVM Slot lookup map. Only the recreate action needs
|
||||||
# the volume driver iterator (to look up volumes and their client
|
# the volume driver iterator (to look up volumes and their client
|
||||||
# mappings).
|
# mappings).
|
||||||
vol_drv_iter = (self._vol_drv_iter(context, instance, bdms,
|
vol_drv_iter = (self._vol_drv_iter(instance, bdms, stg_ftsk=stg_ftsk)
|
||||||
stg_ftsk=stg_ftsk)
|
|
||||||
if recreate else None)
|
if recreate else None)
|
||||||
slot_mgr = slot.build_slot_mgr(
|
slot_mgr = slot.build_slot_mgr(
|
||||||
instance, self.store_api, adapter=self.adapter,
|
instance, self.store_api, adapter=self.adapter,
|
||||||
|
@ -493,7 +492,7 @@ class PowerVMDriver(driver.ComputeDriver):
|
||||||
:param slot_mgr: A NovaSlotManager. Used to store/retrieve the client
|
:param slot_mgr: A NovaSlotManager. Used to store/retrieve the client
|
||||||
slots used when a volume is attached to a VM.
|
slots used when a volume is attached to a VM.
|
||||||
"""
|
"""
|
||||||
for bdm, vol_drv in self._vol_drv_iter(context, instance, bdms,
|
for bdm, vol_drv in self._vol_drv_iter(instance, bdms,
|
||||||
stg_ftsk=stg_ftsk):
|
stg_ftsk=stg_ftsk):
|
||||||
# First connect the volume. This will update the
|
# First connect the volume. This will update the
|
||||||
# connection_info.
|
# connection_info.
|
||||||
|
@ -519,7 +518,7 @@ class PowerVMDriver(driver.ComputeDriver):
|
||||||
slots used when a volume is detached from a VM.
|
slots used when a volume is detached from a VM.
|
||||||
"""
|
"""
|
||||||
# TODO(thorst) Do we need to do something on the disconnect for slots?
|
# TODO(thorst) Do we need to do something on the disconnect for slots?
|
||||||
for bdm, vol_drv in self._vol_drv_iter(context, instance, bdms,
|
for bdm, vol_drv in self._vol_drv_iter(instance, bdms,
|
||||||
stg_ftsk=stg_ftsk):
|
stg_ftsk=stg_ftsk):
|
||||||
flow.add(tf_stg.DisconnectVolume(vol_drv, slot_mgr))
|
flow.add(tf_stg.DisconnectVolume(vol_drv, slot_mgr))
|
||||||
|
|
||||||
|
@ -1166,8 +1165,8 @@ class PowerVMDriver(driver.ComputeDriver):
|
||||||
# adapters in the same slots.
|
# adapters in the same slots.
|
||||||
slot_mgr = slot.build_slot_mgr(
|
slot_mgr = slot.build_slot_mgr(
|
||||||
instance, self.store_api, adapter=self.adapter,
|
instance, self.store_api, adapter=self.adapter,
|
||||||
vol_drv_iter=self._vol_drv_iter(
|
vol_drv_iter=self._vol_drv_iter(instance, bdms,
|
||||||
context, instance, bdms, stg_ftsk=stg_ftsk))
|
stg_ftsk=stg_ftsk))
|
||||||
|
|
||||||
# Determine if there are volumes to disconnect. If so, remove each
|
# Determine if there are volumes to disconnect. If so, remove each
|
||||||
# volume (within the transaction manager)
|
# volume (within the transaction manager)
|
||||||
|
@ -1263,8 +1262,8 @@ class PowerVMDriver(driver.ComputeDriver):
|
||||||
# b) If adding/removing block devices, to register the slots.
|
# b) If adding/removing block devices, to register the slots.
|
||||||
slot_mgr = slot.build_slot_mgr(
|
slot_mgr = slot.build_slot_mgr(
|
||||||
instance, self.store_api, adapter=self.adapter,
|
instance, self.store_api, adapter=self.adapter,
|
||||||
vol_drv_iter=self._vol_drv_iter(
|
vol_drv_iter=self._vol_drv_iter(instance, bdms,
|
||||||
context, instance, bdms, stg_ftsk=stg_ftsk))
|
stg_ftsk=stg_ftsk))
|
||||||
else:
|
else:
|
||||||
stg_ftsk = None
|
stg_ftsk = None
|
||||||
|
|
||||||
|
@ -1662,26 +1661,24 @@ class PowerVMDriver(driver.ComputeDriver):
|
||||||
mig.post_live_migration_at_destination(network_info, vol_drvs)
|
mig.post_live_migration_at_destination(network_info, vol_drvs)
|
||||||
del self.live_migrations[instance.uuid]
|
del self.live_migrations[instance.uuid]
|
||||||
|
|
||||||
def _vol_drv_iter(self, context, instance, bdms, stg_ftsk=None):
|
def _vol_drv_iter(self, instance, bdms, stg_ftsk=None):
|
||||||
"""Yields a bdm and volume driver."""
|
"""Yields a bdm and volume driver."""
|
||||||
# Get a volume driver for each volume
|
# Get a volume driver for each volume
|
||||||
for bdm in bdms or []:
|
for bdm in bdms or []:
|
||||||
conn_info = bdm.get('connection_info')
|
# Ignore if it's not a volume
|
||||||
# if it doesn't have connection_info, it's not a volume
|
if not bdm.is_volume:
|
||||||
if not conn_info:
|
|
||||||
continue
|
continue
|
||||||
|
|
||||||
vol_drv = vol_attach.build_volume_driver(
|
vol_drv = vol_attach.build_volume_driver(
|
||||||
self.adapter, self.host_uuid, instance, conn_info,
|
self.adapter, self.host_uuid, instance,
|
||||||
stg_ftsk=stg_ftsk)
|
bdm.get('connection_info'), stg_ftsk=stg_ftsk)
|
||||||
yield bdm, vol_drv
|
yield bdm, vol_drv
|
||||||
|
|
||||||
def _build_vol_drivers(self, context, instance, block_device_info):
|
def _build_vol_drivers(self, context, instance, block_device_info):
|
||||||
"""Builds the volume connector drivers for a block device info."""
|
"""Builds the volume connector drivers for a block device info."""
|
||||||
# Get a volume driver for each volume
|
# Get a volume driver for each volume
|
||||||
bdms = self._extract_bdm(block_device_info)
|
bdms = self._extract_bdm(block_device_info)
|
||||||
return [vol_drv for bdm, vol_drv in self._vol_drv_iter(
|
return [vol_drv for bdm, vol_drv in self._vol_drv_iter(instance, bdms)]
|
||||||
context, instance, bdms)]
|
|
||||||
|
|
||||||
def unfilter_instance(self, instance, network_info):
|
def unfilter_instance(self, instance, network_info):
|
||||||
"""Stop filtering instance."""
|
"""Stop filtering instance."""
|
||||||
|
|
Loading…
Reference in New Issue