Fix metadata retrieval in GPFS driver

GPFS driver was using 'volume_metadata' to get the
volume metadata. With versionedobjects, it is to be
retrieved using 'metadata' from volume.
This patch fixes the issue. It first tries for 'volume_metadata'
in the volume object, if not present, it will try for 'metadata'
to get the metadata.

Change-Id: I97649874ab54187a469193c8cae773409e9913e6
Closes-bug: 1517404

diff --git a/cinder/tests/unit/test_gpfs.py b/cinder/tests/unit/test_gpfs.py
index e7de73e9af73fd64555094b743d8b2c757474152..75756906a77850711d27eee7c13312ace1e42f44 100644 (file)
--- a/cinder/tests/unit/test_gpfs.py
+++ b/cinder/tests/unit/test_gpfs.py
@@ -688,23 +688,23 @@ class GPFSDriverTestCase(test.TestCase):
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.'
                 '_gpfs_change_attributes')
     def test_set_volume_attributes(self, mock_change_attributes, mock_mkfs):
-        metadata = [{'key': 'data_pool_name', 'value': 'test'},
-                    {'key': 'replicas', 'value': 'test'},
-                    {'key': 'dio', 'value': 'test'},
-                    {'key': 'write_affinity_depth', 'value': 'test'},
-                    {'key': 'block_group_factor', 'value': 'test'},
-                    {'key': 'write_affinity_failure_group', 'value': 'test'},
-                    {'key': 'test', 'value': 'test'},
-                    {'key': 'fstype', 'value': 'test'},
-                    {'key': 'fslabel', 'value': 'test'},
-                    {'key': 'test', 'value': 'test'}]
+        metadata = {'data_pool_name': 'test',
+                    'replicas': 'test',
+                    'dio': 'test',
+                    'write_affinity_depth': 'test',
+                    'block_group_factor': 'test',
+                    'write_affinity_failure_group': 'test',
+                    'test': 'test',
+                    'fstype': 'test',
+                    'fslabel': 'test',
+                    'test': 'test'}
 
         self.driver._set_volume_attributes('', '', metadata)
 
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.'
                 '_gpfs_change_attributes')
     def test_set_volume_attributes_no_attributes(self, mock_change_attributes):
-        metadata = []
+        metadata = {}
         org_value = self.driver.configuration.gpfs_storage_pool
         self.flags(volume_driver=self.driver_name, gpfs_storage_pool='system')
         self.driver._set_volume_attributes('', '', metadata)
@@ -714,13 +714,26 @@ class GPFSDriverTestCase(test.TestCase):
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.'
                 '_gpfs_change_attributes')
     def test_set_volume_attributes_no_options(self, mock_change_attributes):
-        metadata = []
+        metadata = {}
         org_value = self.driver.configuration.gpfs_storage_pool
         self.flags(volume_driver=self.driver_name, gpfs_storage_pool='')
         self.driver._set_volume_attributes('', '', metadata)
         self.flags(volume_driver=self.driver_name,
                    gpfs_storage_pool=org_value)
 
+    def test_get_volume_metadata(self):
+        volume = self._fake_volume()
+        volume['volume_metadata'] = [{'key': 'fake_key',
+                                      'value': 'fake_value'}]
+        expected_metadata = {'fake_key': 'fake_value'}
+        v_metadata = self.driver._get_volume_metadata(volume)
+        self.assertEqual(expected_metadata, v_metadata)
+        volume.pop('volume_metadata')
+        volume['metadata'] = {'key': 'value'}
+        expected_metadata = {'key': 'value'}
+        v_metadata = self.driver._get_volume_metadata(volume)
+        self.assertEqual(expected_metadata, v_metadata)
+
     @mock.patch('cinder.utils.execute')
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.'
                 '_allocate_file_blocks')
@@ -808,7 +821,7 @@ class GPFSDriverTestCase(test.TestCase):
         value = {}
         value['value'] = 'test'
         mock_set_volume_attributes.return_value = True
-        metadata = [{'key': 'fake_key', 'value': 'fake_value'}]
+        metadata = {'fake_key': 'fake_value'}
 
         org_value = self.driver.configuration.gpfs_sparse_volumes
         self.flags(volume_driver=self.driver_name, gpfs_sparse_volumes=False)
@@ -875,7 +888,7 @@ class GPFSDriverTestCase(test.TestCase):
         snapshot = self._fake_snapshot()
         mock_snapshot_path.return_value = "/tmp/fakepath"
         mock_set_volume_attributes.return_value = True
-        metadata = [{'key': 'fake_key', 'value': 'fake_value'}]
+        metadata = {'fake_key': 'fake_value'}
 
         self.assertTrue(self.driver._set_volume_attributes(volume, 'test',
                                                            metadata))
@@ -927,7 +940,7 @@ class GPFSDriverTestCase(test.TestCase):
         volume = self._fake_volume()
         src_volume = self._fake_volume()
         mock_set_volume_attributes.return_value = True
-        metadata = [{'key': 'fake_key', 'value': 'fake_value'}]
+        metadata = {'fake_key': 'fake_value'}
 
         self.assertTrue(self.driver._set_volume_attributes(volume, 'test',
                                                            metadata))

diff --git a/cinder/volume/drivers/ibm/gpfs.py b/cinder/volume/drivers/ibm/gpfs.py
index ba669317822402505dc666910d097e9e1589d45a..5fbb70e2f8f2b52ca2ccde4363628fd6a0ffe7ed 100644 (file)
--- a/cinder/volume/drivers/ibm/gpfs.py
+++ b/cinder/volume/drivers/ibm/gpfs.py
@@ -478,20 +478,20 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         set_pool = False
         options = []
         for item in metadata:
-            if item['key'] == 'data_pool_name':
-                options.extend(['-P', item['value']])
+            if item == 'data_pool_name':
+                options.extend(['-P', metadata[item]])
                 set_pool = True
-            elif item['key'] == 'replicas':
-                options.extend(['-r', item['value'], '-m', item['value']])
-            elif item['key'] == 'dio':
-                options.extend(['-D', item['value']])
-            elif item['key'] == 'write_affinity_depth':
-                options.extend(['--write-affinity-depth', item['value']])
-            elif item['key'] == 'block_group_factor':
-                options.extend(['--block-group-factor', item['value']])
-            elif item['key'] == 'write_affinity_failure_group':
+            elif item == 'replicas':
+                options.extend(['-r', metadata[item], '-m', metadata[item]])
+            elif item == 'dio':
+                options.extend(['-D', metadata[item]])
+            elif item == 'write_affinity_depth':
+                options.extend(['--write-affinity-depth', metadata[item]])
+            elif item == 'block_group_factor':
+                options.extend(['--block-group-factor', metadata[item]])
+            elif item == 'write_affinity_failure_group':
                 options.extend(['--write-affinity-failure-group',
-                               item['value']])
+                               metadata[item]])
 
         # metadata value has precedence over value set in volume type
         if self.configuration.gpfs_storage_pool and not set_pool:
@@ -503,13 +503,21 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         fstype = None
         fslabel = None
         for item in metadata:
-            if item['key'] == 'fstype':
-                fstype = item['value']
-            elif item['key'] == 'fslabel':
-                fslabel = item['value']
+            if item == 'fstype':
+                fstype = metadata[item]
+            elif item == 'fslabel':
+                fslabel = metadata[item]
         if fstype:
             self._mkfs(volume, fstype, fslabel)
 
+    def _get_volume_metadata(self, volume):
+        volume_metadata = {}
+        if 'volume_metadata' in volume:
+            for metadata in volume['volume_metadata']:
+                volume_metadata[metadata['key']] = metadata['value']
+            return volume_metadata
+        return volume['metadata'] if 'metadata' in volume else {}
+
     def create_volume(self, volume):
         """Creates a GPFS volume."""
         # Check if GPFS is mounted
@@ -523,7 +531,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         self._set_rw_permission(volume_path)
         # Set the attributes prior to allocating any blocks so that
         # they are allocated according to the policy
-        v_metadata = volume.get('volume_metadata')
+        v_metadata = self._get_volume_metadata(volume)
         self._set_volume_attributes(volume, volume_path, v_metadata)
 
         if not self.configuration.gpfs_sparse_volumes:
@@ -548,7 +556,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
             self._gpfs_full_copy(snapshot_path, volume_path)
 
         self._set_rw_permission(volume_path)
-        v_metadata = volume.get('volume_metadata')
+        v_metadata = self._get_volume_metadata(volume)
         self._set_volume_attributes(volume, volume_path, v_metadata)
 
     def create_volume_from_snapshot(self, volume, snapshot):
@@ -568,7 +576,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         else:
             self._gpfs_full_copy(src, dest)
         self._set_rw_permission(dest)
-        v_metadata = volume.get('volume_metadata')
+        v_metadata = self._get_volume_metadata(volume)
         self._set_volume_attributes(volume, dest, v_metadata)
 
     def create_cloned_volume(self, volume, src_vref):