Don't remove the hdisk if multiattach

Follow up change set for multiattach, If the multiple mapping found
for the same hdisk, do not attempt to remove the hdisk.

Change-Id: I0175bb4827dc76dfb774907e1db93d92c39731f6
Partially-implements: blueprint multi-attach-volume

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."""

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):