]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Dell SC: init_volume stale volume info fix
authorTom Swanson <tom_swanson@dell.com>
Tue, 25 Aug 2015 20:13:31 +0000 (15:13 -0500)
committerTom Swanson <tom_swanson@dell.com>
Thu, 27 Aug 2015 16:09:11 +0000 (11:09 -0500)
In init_volume the volume information becomes stale after the volume
is mapped to a server.  The volume should be updated before being sent
to unmap.

The unmap function was testing for the volume being active.  This
should not be required.  It is unlikely an inactive volume will have
mappings but not impossible.  This check has been removed.

Removed LOG.error from _find_mapping_profiles as the API returning
an error is fine if the volume is inactive.  Plus we were not failing
on it.  The LOG.debug is sufficient.

Change-Id: I3ff8447360cea134e1008fa8db5cd912aa5effd3

cinder/tests/unit/test_dellscapi.py
cinder/volume/drivers/dell/dell_storagecenter_api.py

index 1fdbffaffb981a04719e0ce93e5a746f78b4ff29..2847bbe0fa80c78120bb659a5e6e0a2cc72b0a39 100644 (file)
@@ -1906,6 +1906,9 @@ class DellSCSanAPITestCase(test.TestCase):
         self.assertTrue(mock_create_folder_path.called)
         self.assertEqual(self.FLDR, res, 'Unexpected Folder')
 
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'find_volume',
+                       return_value=VOLUME)
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
                        'unmap_volume',
                        return_value=True)
@@ -1923,6 +1926,7 @@ class DellSCSanAPITestCase(test.TestCase):
                          mock_get_json,
                          mock_map_volume,
                          mock_unmap_volume,
+                         mock_find_volume,
                          mock_close_connection,
                          mock_open_connection,
                          mock_init):
index 40bd6293fca421b82e2fd78702dd997c1d477042..dbd47d38af4357cfcf459e119b95d7949277fd3f 100644 (file)
@@ -555,6 +555,8 @@ class StorageCenterApi(object):
                     # Map to actually create the volume
                     self.map_volume(scvolume,
                                     scserver)
+                    # We have changed the volume so grab a new copy of it.
+                    scvolume = self.find_volume(scvolume.get('name'))
                     self.unmap_volume(scvolume,
                                       scserver)
                     return
@@ -1079,19 +1081,14 @@ class StorageCenterApi(object):
         :returns: A list of Dell mapping profile objects.
         """
         mapping_profiles = []
-        if scvolume.get('active', False):
-            r = self.client.get('StorageCenter/ScVolume/%s/MappingProfileList'
-                                % self._get_id(scvolume))
-            if r.status_code == 200:
-                mapping_profiles = self._get_json(r)
-            else:
-                LOG.debug('MappingProfileList error: %(code)d %(reason)s',
-                          {'code': r.status_code,
-                           'reason': r.reason})
-                LOG.error(_LE('Unable to find volume mapping profiles: %s'),
-                          scvolume.get('name'))
+        r = self.client.get('StorageCenter/ScVolume/%s/MappingProfileList'
+                            % self._get_id(scvolume))
+        if r.status_code == 200:
+            mapping_profiles = self._get_json(r)
         else:
-            LOG.error(_LE('_find_mappings: volume is not active'))
+            LOG.debug('MappingProfileList error: %(code)d %(reason)s',
+                      {'code': r.status_code,
+                       'reason': r.reason})
         LOG.debug(mapping_profiles)
         return mapping_profiles