Add volume attribute support to volume created using clone, snapshot

The following patch adds support for volume attributes (setting of
storage pool, replicas, dio, write affinity etc) in case of volume
creation triggered using snapshot, clone.

Change-Id: Ia6e24cafb85c0b2996e2524b45c189fb38e83848
Closes-Bug: #1386650

diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py
index fec6ad85c0da44d42582b8c81f152b9addcada9b..fbf3e6feb003f6139ce5701d3bf9af0c29969854 100644 (file)
--- a/cinder/tests/test_gpfs.py
+++ b/cinder/tests/test_gpfs.py
@@ -644,8 +644,9 @@ class GPFSDriverTestCase(test.TestCase):
         options.extend(['-T', 'test'])
         self.driver._gpfs_change_attributes(options, self.images_dir)
 
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._mkfs')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._gpfs_change_attributes')
-    def test_set_volume_attributes(self, mock_change_attributes):
+    def test_set_volume_attributes(self, mock_change_attributes, mock_mkfs):
         metadata = [dict([('key', 'data_pool_name'), ('value', 'test')]),
                     dict([('key', 'replicas'), ('value', 'test')]),
                     dict([('key', 'dio'), ('value', 'test')]),
@@ -653,16 +654,23 @@ class GPFSDriverTestCase(test.TestCase):
                     dict([('key', 'block_group_factor'), ('value', 'test')]),
                     dict([('key', 'write_affinity_failure_group'),
                           ('value', 'test')]),
+                    dict([('key', 'test'),
+                          ('value', 'test')]),
+                    dict([('key', 'fstype'),
+                          ('value', 'test')]),
+                    dict([('key', 'fslabel'),
+                          ('value', 'test')]),
                     dict([('key', 'test'),
                           ('value', 'test')])]
-        self.driver._set_volume_attributes('', metadata)
+
+        self.driver._set_volume_attributes('', '', metadata)
 
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._gpfs_change_attributes')
     def test_set_volume_attributes_no_attributes(self, mock_change_attributes):
         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)
+        self.driver._set_volume_attributes('', '', metadata)
         self.flags(volume_driver=self.driver_name,
                    gpfs_storage_pool=org_value)
 
@@ -671,7 +679,7 @@ class GPFSDriverTestCase(test.TestCase):
         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.driver._set_volume_attributes('', '', metadata)
         self.flags(volume_driver=self.driver_name,
                    gpfs_storage_pool=org_value)
 
@@ -695,12 +703,6 @@ class GPFSDriverTestCase(test.TestCase):
         volume['size'] = 1000
         value = {}
         value['value'] = 'test'
-        volume['volume_metadata'] = [dict([('key', 'fstype'),
-                                           ('value', 'test')]),
-                                     dict([('key', 'fslabel'),
-                                           ('value', 'test')]),
-                                     dict([('key', 'test'),
-                                           ('value', 'test')])]
 
         org_value = self.driver.configuration.gpfs_sparse_volumes
         self.flags(volume_driver=self.driver_name, gpfs_sparse_volumes=False)
@@ -728,12 +730,6 @@ class GPFSDriverTestCase(test.TestCase):
         volume['size'] = 1000
         value = {}
         value['value'] = 'test'
-        volume['volume_metadata'] = [dict([('key', 'fstype_'),
-                                           ('value', 'test')]),
-                                     dict([('key', 'fslabel'),
-                                           ('value', 'test')]),
-                                     dict([('key', 'test'),
-                                           ('value', 'test')])]
 
         org_value = self.driver.configuration.gpfs_sparse_volumes
         self.flags(volume_driver=self.driver_name, gpfs_sparse_volumes=True)
@@ -741,7 +737,39 @@ class GPFSDriverTestCase(test.TestCase):
         self.flags(volume_driver=self.driver_name,
                    gpfs_sparse_volumes=org_value)
 
+    @patch('cinder.utils.execute')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._allocate_file_blocks')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_volume_attributes')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_rw_permission')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._create_sparse_file')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.local_path')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._verify_gpfs_path_state')
+    def test_create_volume_with_metadata(self,
+                                         mock_gpfs_path_state,
+                                         mock_local_path,
+                                         mock_sparse_file,
+                                         mock_rw_permission,
+                                         mock_set_volume_attributes,
+                                         mock_allocate_file_blocks,
+                                         mock_exec):
+        mock_local_path.return_value = 'test'
+        volume = {}
+        volume['size'] = 1000
+        value = {}
+        value['value'] = 'test'
+        mock_set_volume_attributes.return_value = True
+        metadata = [{'key': 'fake_key', 'value': 'fake_value'}]
+
+        org_value = self.driver.configuration.gpfs_sparse_volumes
+        self.flags(volume_driver=self.driver_name, gpfs_sparse_volumes=False)
+        self.driver.create_volume(volume)
+        self.assertEqual(True, self.driver._set_volume_attributes(volume,
+                         'test', metadata))
+        self.flags(volume_driver=self.driver_name,
+                   gpfs_sparse_volumes=org_value)
+
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._resize_volume_file')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_volume_attributes')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._gpfs_redirect')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_rw_permission')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._create_gpfs_copy')
@@ -751,6 +779,7 @@ class GPFSDriverTestCase(test.TestCase):
                                          mock_create_gpfs_copy,
                                          mock_rw_permission,
                                          mock_gpfs_redirect,
+                                         mock_set_volume_attributes,
                                          mock_resize_volume_file):
         mock_resize_volume_file.return_value = 5 * units.Gi
         volume = {}
@@ -759,6 +788,31 @@ class GPFSDriverTestCase(test.TestCase):
                          {'size': 5.0})
 
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._resize_volume_file')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_volume_attributes')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._gpfs_redirect')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_rw_permission')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._create_gpfs_copy')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.local_path')
+    def test_create_volume_from_snapshot_metadata(self,
+                                                  mock_local_path,
+                                                  mock_create_gpfs_copy,
+                                                  mock_rw_permission,
+                                                  mock_gpfs_redirect,
+                                                  mock_set_volume_attributes,
+                                                  mock_resize_volume_file):
+        mock_resize_volume_file.return_value = 5 * units.Gi
+        volume = {}
+        volume['size'] = 1000
+        mock_set_volume_attributes.return_value = True
+        metadata = [{'key': 'fake_key', 'value': 'fake_value'}]
+
+        self.assertEqual(True, self.driver._set_volume_attributes(volume,
+                         'test', metadata))
+        self.assertEqual(self.driver.create_volume_from_snapshot(volume, ''),
+                         {'size': 5.0})
+
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._resize_volume_file')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_volume_attributes')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_rw_permission')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._create_gpfs_clone')
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.local_path')
@@ -766,6 +820,7 @@ class GPFSDriverTestCase(test.TestCase):
                                   mock_local_path,
                                   mock_create_gpfs_clone,
                                   mock_rw_permission,
+                                  mock_set_volume_attributes,
                                   mock_resize_volume_file):
         mock_resize_volume_file.return_value = 5 * units.Gi
         volume = {}
@@ -773,6 +828,28 @@ class GPFSDriverTestCase(test.TestCase):
         self.assertEqual(self.driver.create_cloned_volume(volume, ''),
                          {'size': 5.0})
 
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._resize_volume_file')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_volume_attributes')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._set_rw_permission')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._create_gpfs_clone')
+    @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.local_path')
+    def test_create_cloned_volume_with_metadata(self,
+                                                mock_local_path,
+                                                mock_create_gpfs_clone,
+                                                mock_rw_permission,
+                                                mock_set_volume_attributes,
+                                                mock_resize_volume_file):
+        mock_resize_volume_file.return_value = 5 * units.Gi
+        volume = {}
+        volume['size'] = 1000
+        mock_set_volume_attributes.return_value = True
+        metadata = [{'key': 'fake_key', 'value': 'fake_value'}]
+
+        self.assertEqual(True, self.driver._set_volume_attributes(volume,
+                         'test', metadata))
+        self.assertEqual(self.driver.create_cloned_volume(volume, ''),
+                         {'size': 5.0})
+
     @patch('cinder.utils.execute')
     def test_delete_gpfs_file_ok(self, mock_exec):
         mock_exec.side_effect = [('Parent  Depth   Parent inode   File name\n'

diff --git a/cinder/volume/drivers/ibm/gpfs.py b/cinder/volume/drivers/ibm/gpfs.py
index 65e3359c8c45a9dbf52763d3587b2b7bac07fa4d..e12ee4bb1e695f7abb94756ec0a9b182faec5f22 100644 (file)
--- a/cinder/volume/drivers/ibm/gpfs.py
+++ b/cinder/volume/drivers/ibm/gpfs.py
@@ -449,7 +449,7 @@ class GPFSDriver(driver.VolumeDriver):
         LOG.debug('Update volume attributes with mmchattr to %s.' % options)
         self._execute(*cmd, run_as_root=True)
 
-    def _set_volume_attributes(self, path, metadata):
+    def _set_volume_attributes(self, volume, path, metadata):
         """Set various GPFS attributes for this volume."""
 
         set_pool = False
@@ -477,6 +477,16 @@ class GPFSDriver(driver.VolumeDriver):
         if options:
             self._gpfs_change_attributes(options, path)
 
+        fstype = None
+        fslabel = None
+        for item in metadata:
+            if item['key'] == 'fstype':
+                fstype = item['value']
+            elif item['key'] == 'fslabel':
+                fslabel = item['value']
+        if fstype:
+            self._mkfs(volume, fstype, fslabel)
+
     def create_volume(self, volume):
         """Creates a GPFS volume."""
         # Check if GPFS is mounted
@@ -491,21 +501,11 @@ class GPFSDriver(driver.VolumeDriver):
         # Set the attributes prior to allocating any blocks so that
         # they are allocated according to the policy
         v_metadata = volume.get('volume_metadata')
-        self._set_volume_attributes(volume_path, v_metadata)
+        self._set_volume_attributes(volume, volume_path, v_metadata)
 
         if not self.configuration.gpfs_sparse_volumes:
             self._allocate_file_blocks(volume_path, volume_size)
 
-        fstype = None
-        fslabel = None
-        for item in v_metadata:
-            if item['key'] == 'fstype':
-                fstype = item['value']
-            elif item['key'] == 'fslabel':
-                fslabel = item['value']
-        if fstype:
-            self._mkfs(volume, fstype, fslabel)
-
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a GPFS volume from a snapshot."""
 
@@ -514,6 +514,8 @@ class GPFSDriver(driver.VolumeDriver):
         self._create_gpfs_copy(src=snapshot_path, dest=volume_path)
         self._set_rw_permission(volume_path)
         self._gpfs_redirect(volume_path)
+        v_metadata = volume.get('volume_metadata')
+        self._set_volume_attributes(volume, volume_path, v_metadata)
         virt_size = self._resize_volume_file(volume, volume['size'])
         return {'size': math.ceil(virt_size / units.Gi)}
 
@@ -524,6 +526,8 @@ class GPFSDriver(driver.VolumeDriver):
         dest = self.local_path(volume)
         self._create_gpfs_clone(src, dest)
         self._set_rw_permission(dest)
+        v_metadata = volume.get('volume_metadata')
+        self._set_volume_attributes(volume, dest, v_metadata)
         virt_size = self._resize_volume_file(volume, volume['size'])
         return {'size': math.ceil(virt_size / units.Gi)}