From 723b6dfba0ee0261002485a1397c5b69a266527c Mon Sep 17 00:00:00 2001 From: Gaurang Tapase Date: Wed, 18 Nov 2015 05:56:39 -0500 Subject: [PATCH] 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 --- cinder/tests/unit/test_gpfs.py | 43 +++++++++++++++++++---------- cinder/volume/drivers/ibm/gpfs.py | 46 ++++++++++++++++++------------- 2 files changed, 55 insertions(+), 34 deletions(-) diff --git a/cinder/tests/unit/test_gpfs.py b/cinder/tests/unit/test_gpfs.py index e7de73e9af7..75756906a77 100644 --- 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 ba669317822..5fbb70e2f8f 100644 --- 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):