Fix Infinidat driver generic volume migration

- Use volume name_id to resolve Infinidat volume name.
- Add update_migrated_volume function to fix generic volume
  migration between two pools within the same storage cluster.

Closes-bug: #1982405
Change-Id: I80923f9968de31738c554b77c0942a6182f0e67c
Signed-off-by: Alexander Deiter <adeiter@infinidat.com>

cinder/tests/unit/volume/drivers/test_infinidat.py

@@ -57,8 +57,10 @@ TEST_SNAPSHOT_SOURCE_NAME = 'test-snapshot'
 TEST_SNAPSHOT_SOURCE_ID = 67890
 TEST_SNAPSHOT_METADATA = {'cinder_id': fake.SNAPSHOT_ID}
 
-test_volume = mock.Mock(id=fake.VOLUME_ID, size=1,
+test_volume = mock.Mock(id=fake.VOLUME_ID, name_id=fake.VOLUME_ID, size=1,
                         volume_type_id=fake.VOLUME_TYPE_ID)
+test_volume2 = mock.Mock(id=fake.VOLUME2_ID, name_id=fake.VOLUME2_ID, size=1,
+                         volume_type_id=fake.VOLUME_TYPE_ID)
 test_snapshot = mock.Mock(id=fake.SNAPSHOT_ID, volume=test_volume,
                           volume_id=test_volume.id)
 test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1)
@@ -104,6 +106,7 @@ class InfiniboxDriverTestCaseBase(test.TestCase):
         # mock external library dependencies
         infinisdk = self.patch("cinder.volume.drivers.infinidat.infinisdk")
         capacity = self.patch("cinder.volume.drivers.infinidat.capacity")
+        self._log = self.patch("cinder.volume.drivers.infinidat.LOG")
         self._iqn = self.patch("cinder.volume.drivers.infinidat.iqn")
         self._wwn = self.patch("cinder.volume.drivers.infinidat.wwn")
         self._wwn.WWN = mock.Mock
@@ -858,6 +861,62 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
     def test_unmanage_snapshot(self):
         self.driver.unmanage_snapshot(test_snapshot)
 
+    def test_update_migrated_volume_new_volume_not_found(self):
+        self._system.volumes.safe_get.side_effect = [
+            None, self._mock_volume]
+        self.assertRaises(exception.VolumeNotFound,
+                          self.driver.update_migrated_volume,
+                          None, test_volume, test_volume2,
+                          'available')
+
+    @mock.patch('cinder.volume.drivers.infinidat.InfiniboxVolumeDriver.'
+                '_set_cinder_object_metadata')
+    def test_update_migrated_volume_volume_not_found(self, set_metadata):
+        self._system.volumes.safe_get.side_effect = [
+            self._mock_new_volume, None]
+        update = self.driver.update_migrated_volume(None,
+                                                    test_volume,
+                                                    test_volume2,
+                                                    'available')
+        expected = {'_name_id': None, 'provider_location': None}
+        self.assertEqual(expected, update)
+        set_metadata.assert_called_once_with(self._mock_new_volume,
+                                             test_volume)
+
+    @mock.patch('cinder.volume.drivers.infinidat.InfiniboxVolumeDriver.'
+                '_set_cinder_object_metadata')
+    def test_update_migrated_new_volume_rename_error(self, set_metadata):
+        self._system.volumes.safe_get.side_effect = [
+            self._mock_new_volume, None]
+        self._mock_new_volume.update_name.side_effect = [
+            FakeInfinisdkException]
+        update = self.driver.update_migrated_volume(None,
+                                                    test_volume,
+                                                    test_volume2,
+                                                    'available')
+        expected = {'_name_id': test_volume2.name_id,
+                    'provider_location': None}
+        self.assertEqual(expected, update)
+        set_metadata.assert_called_once_with(self._mock_new_volume,
+                                             test_volume)
+
+    @mock.patch('cinder.volume.drivers.infinidat.InfiniboxVolumeDriver.'
+                '_set_cinder_object_metadata')
+    def test_update_migrated(self, set_metadata):
+        self._system.volumes.safe_get.side_effect = [
+            self._mock_new_volume, self._mock_volume]
+        self._mock_new_volume.update_name.side_effect = None
+        update = self.driver.update_migrated_volume(None,
+                                                    test_volume,
+                                                    test_volume2,
+                                                    'available')
+        expected = {'_name_id': test_volume2.name_id,
+                    'provider_location': None}
+        self.assertEqual(expected, update)
+        set_metadata.assert_called_once_with(self._mock_new_volume,
+                                             test_volume)
+        self.assertEqual(0, self._log.error.call_count)
+
 
 class InfiniboxDriverTestCaseFC(InfiniboxDriverTestCaseBase):
     def test_initialize_connection_multiple_wwpns(self):

cinder/volume/drivers/infinidat.py

@@ -125,10 +125,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         1.8 - added revert to snapshot
         1.9 - added manage/unmanage/manageable-list volume/snapshot
         1.10 - added support for TLS/SSL communication
+        1.11 - fixed generic volume migration
 
     """
 
-    VERSION = '1.10'
+    VERSION = '1.11'
 
     # ThirdPartySystems wiki page
     CI_WIKI_NAME = "INFINIDAT_CI"
@@ -202,8 +203,17 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
                       'in the connector.', {'data': required})
             raise exception.InvalidConnectorException(missing=required)
 
-    def _make_volume_name(self, cinder_volume):
+    def _make_volume_name(self, cinder_volume, migration=False):
-        return 'openstack-vol-%s' % cinder_volume.id
+        """Return the Infinidat volume name.
+
+        Use Cinder volume id in case of volume migration
+        and use Cinder volume name_id for all other cases.
+        """
+        if migration:
+            key = cinder_volume.id
+        else:
+            key = cinder_volume.name_id
+        return 'openstack-vol-%s' % key
 
     def _make_snapshot_name(self, cinder_snapshot):
         return 'openstack-snap-%s' % cinder_snapshot.id
@@ -655,7 +665,7 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         # we need a cinder-volume-like object to map the clone by name
         # (which is derived from the cinder id) but the clone is internal
         # so there is no such object. mock one
-        clone = mock.Mock(id=str(volume.id) + '-internal')
+        clone = mock.Mock(name_id=str(volume.name_id) + '-internal')
         try:
             infinidat_volume = self._create_volume(volume)
             try:
@@ -1194,3 +1204,41 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         """
         infinidat_snapshot = self._get_infinidat_snapshot(snapshot)
         infinidat_snapshot.clear_metadata()
+
+    @infinisdk_to_cinder_exceptions
+    def update_migrated_volume(self, ctxt, volume, new_volume,
+                               original_volume_status):
+        """Return model update from Infinidat for migrated volume.
+
+        This method should rename the back-end volume name(id) on the
+        destination host back to its original name(id) on the source host.
+
+        :param ctxt: The context used to run the method update_migrated_volume
+        :param volume: The original volume that was migrated to this backend
+        :param new_volume: The migration volume object that was created on
+                           this backend as part of the migration process
+        :param original_volume_status: The status of the original volume
+        :returns: model_update to update DB with any needed changes
+        """
+        model_update = {'_name_id': new_volume.name_id,
+                        'provider_location': None}
+        new_volume_name = self._make_volume_name(new_volume, migration=True)
+        new_infinidat_volume = self._get_infinidat_volume(new_volume)
+        self._set_cinder_object_metadata(new_infinidat_volume, volume)
+        volume_name = self._make_volume_name(volume, migration=True)
+        try:
+            infinidat_volume = self._get_infinidat_volume(volume)
+        except exception.VolumeNotFound:
+            LOG.debug('Source volume %s not found', volume_name)
+        else:
+            volume_pool = infinidat_volume.get_pool_name()
+            LOG.debug('Found source volume %s in pool %s',
+                      volume_name, volume_pool)
+            return model_update
+        try:
+            new_infinidat_volume.update_name(volume_name)
+        except infinisdk.core.exceptions.InfiniSDKException as error:
+            LOG.error('Failed to rename destination volume %s -> %s: %s',
+                      new_volume_name, volume_name, error)
+            return model_update
+        return {'_name_id': None, 'provider_location': None}

releasenotes/notes/bug-1982405-infinidat-fix-generic-volume-migration-da33a6fe980ac4eb.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Infinidat Driver `bug #1982405
+    <https://bugs.launchpad.net/cinder/+bug/1982405>`_:
+    Fixed Infinidat driver to allow generic volume migration
+    between two storage pools within the same cluster.