diff --git a/nova_powervm/tests/virt/powervm/volume/test_vscsi.py b/nova_powervm/tests/virt/powervm/volume/test_vscsi.py index 4337ff67..4ed42488 100644 --- a/nova_powervm/tests/virt/powervm/volume/test_vscsi.py +++ b/nova_powervm/tests/virt/powervm/volume/test_vscsi.py @@ -280,6 +280,36 @@ class TestVSCSIAdapter(BaseVSCSITest): mock_remove_hdisk.assert_called_once_with( self.adpt, mock.ANY, 'device_name', self.vios_uuid) + @mock.patch('pypowervm.tasks.scsi_mapper.find_maps') + @mock.patch('pypowervm.tasks.hdisk.remove_hdisk') + @mock.patch('pypowervm.wrappers.virtual_io_server.VIOS.hdisk_from_uuid') + @mock.patch('pypowervm.tasks.scsi_mapper.remove_maps') + @mock.patch('nova_powervm.virt.powervm.vm.get_vm_id') + def test_disconnect_volume_shared(self, mock_get_vm_id, mock_remove_maps, + mock_hdisk_from_uuid, mock_remove_hdisk, + mock_find_maps): + # The mock return values + mock_hdisk_from_uuid.return_value = 'device_name' + mock_get_vm_id.return_value = 'partition_id' + # Consider there are multiple attachments + mock_find_maps.return_value = [mock.MagicMock(), mock.MagicMock()] + self.vol_drv._set_udid('UDIDIT!') + + def validate_remove_maps(vios_w, vm_uuid, match_func): + self.assertIsInstance(vios_w, pvm_vios.VIOS) + self.assertEqual('partition_id', vm_uuid) + return 'removed' + mock_remove_maps.side_effect = validate_remove_maps + + # Run the method + self.vol_drv.disconnect_volume() + + # As initialized above, remove_maps returns True to trigger update. + self.assertEqual(1, mock_remove_maps.call_count) + self.assertEqual(1, self.ft_fx.patchers['update'].mock.call_count) + # Since device has multiple mappings remove disk should not get called + self.assertEqual(0, mock_remove_hdisk.call_count) + @mock.patch('pypowervm.wrappers.virtual_io_server.VIOS.hdisk_from_uuid') @mock.patch('pypowervm.tasks.scsi_mapper.remove_maps') @mock.patch('nova_powervm.virt.powervm.vm.get_vm_id') @@ -427,6 +457,23 @@ class TestVSCSIAdapter(BaseVSCSITest): i_wwpn, t_wwpns, lun = self.vol_drv._get_hdisk_itls(mock_vios) self.assertListEqual([], i_wwpn) + @mock.patch('pypowervm.tasks.scsi_mapper.find_maps') + def test_check_host_mappings(self, mock_find): + mock_vios = mock.MagicMock() + mock_vios.uuid = self.vios_uuid + # Test when multiple matching entries found + mock_find.return_value = [mock.MagicMock(), mock.MagicMock()] + mapping = self.vol_drv._check_host_mappings(mock_vios, self.volume_id) + self.assertTrue(mapping) + # Test when single entry found check host mapping should return False + mock_find.return_value = [mock.MagicMock()] + mapping = self.vol_drv._check_host_mappings(mock_vios, self.volume_id) + self.assertFalse(mapping) + # Test when no entry found check host mapping should return False + mock_find.return_value = [] + mapping = self.vol_drv._check_host_mappings(mock_vios, self.volume_id) + self.assertFalse(mapping) + class TestVSCSIAdapterMultiVIOS(BaseVSCSITest): """Tests the vSCSI Volume Connector Adapter against multiple VIOSes.""" diff --git a/nova_powervm/virt/powervm/volume/vscsi.py b/nova_powervm/virt/powervm/volume/vscsi.py index 9954685b..81618872 100644 --- a/nova_powervm/virt/powervm/volume/vscsi.py +++ b/nova_powervm/virt/powervm/volume/vscsi.py @@ -365,8 +365,7 @@ class VscsiVolumeAdapter(v_driver.FibreChannelVolumeAdapter): self._add_remove_mapping(partition_id, vios_w.uuid, device_name) - # Add a step after the mapping removal to also remove the - # hdisk. + # Add a step to also remove the hdisk self._add_remove_hdisk(vios_w, device_name) # Found a valid element to remove @@ -399,6 +398,27 @@ class VscsiVolumeAdapter(v_driver.FibreChannelVolumeAdapter): 'instance_name': self.instance.name} raise p_exc.VolumeDetachFailed(**ex_args) + def _check_host_mappings(self, vios_wrap, device_name): + """Checks if the given hdisk has multiple mappings + + :param vio_wrap: The Virtual I/O Server wrapper to remove the disk + from. + :param device_name: The hdisk name to remove. + + :return: True is there are multiple instances using the given hdisk + """ + vios_scsi_mappings = next(v.scsi_mappings for v in self.stg_ftsk.feed + if v.uuid == vios_wrap.uuid) + mappings = tsk_map.find_maps( + vios_scsi_mappings, None, + tsk_map.gen_match_func(pvm_stor.PV, names=[device_name])) + + LOG.info(_LI("%(num)d Storage Mappings found for %(dev)s"), + {'num': len(mappings), 'dev': device_name}) + # the mapping is still present as the task feed removes + # the mapping later + return len(mappings) > 1 + def _add_remove_hdisk(self, vio_wrap, device_name, stg_ftsk=None): """Adds a post-mapping task to remove the hdisk from the VIOS. @@ -424,9 +444,15 @@ class VscsiVolumeAdapter(v_driver.FibreChannelVolumeAdapter): "%(disk)s from the Virtual I/O Server."), {'disk': device_name}) LOG.warning(e) - name = 'rm_hdisk_%s_%s' % (vio_wrap.name, device_name) - stg_ftsk = stg_ftsk or self.stg_ftsk - stg_ftsk.add_post_execute(task.FunctorTask(rm_hdisk, name=name)) + + # Check if there are not multiple mapping for the device + if not self._check_host_mappings(vio_wrap, device_name): + name = 'rm_hdisk_%s_%s' % (vio_wrap.name, device_name) + stg_ftsk = stg_ftsk or self.stg_ftsk + stg_ftsk.add_post_execute(task.FunctorTask(rm_hdisk, name=name)) + else: + LOG.info(_LI("hdisk %(disk)s is not removed because it has " + "existing storage mappings"), {'disk': device_name}) @lockutils.synchronized('vscsi_wwpns') def wwpns(self):