Update gpfs driver volume creation process

Modify gpfs driver to set file permissions in
a more consistent way.

Modify image_utils.resize_image to allow caller
to request it be run as root.

SecurityImpact
Change-Id: Ic01d91c0d660c74095e8d2b212279b39b9b9dc05
Partial-Bug: #1260679

diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py
index e73e08e4bded9ebb562d609f24388eaafbe1f85d..883f736350d2944d3a2b90ac7a474cbbfdf456fc 100644 (file)
--- a/cinder/image/image_utils.py
+++ b/cinder/image/image_utils.py
@@ -65,10 +65,10 @@ def convert_image(source, dest, out_format):
     utils.execute(*cmd, run_as_root=True)
 
 
-def resize_image(source, size):
+def resize_image(source, size, run_as_root=False):
     """Changes the virtual size of the image."""
     cmd = ('qemu-img', 'resize', source, '%sG' % size)
-    utils.execute(*cmd, run_as_root=False)
+    utils.execute(*cmd, run_as_root=run_as_root)
 
 
 def fetch(context, image_service, image_id, path, _user_id, _project_id):

diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py
index ffa85c179d448b1a9a46a820cf87a7e9d2f2db47..32569661ce92559d48bb0cdf31bbe62cf4e412eb 100644 (file)
--- a/cinder/tests/test_gpfs.py
+++ b/cinder/tests/test_gpfs.py
@@ -378,7 +378,7 @@ class GPFSDriverTestCase(test.TestCase):
         backing file: %s
         """ % (volume['name'], new_vol_size, new_vol_size * units.GiB, volpath)
         mox.StubOutWithMock(image_utils, 'resize_image')
-        image_utils.resize_image(volpath, new_vol_size)
+        image_utils.resize_image(volpath, new_vol_size, run_as_root=True)
 
         mox.StubOutWithMock(image_utils, 'qemu_img_info')
         img_info = imageutils.QemuImgInfo(qemu_img_info_output)
@@ -395,8 +395,8 @@ class GPFSDriverTestCase(test.TestCase):
         volpath = os.path.join(self.volumes_path, volume['name'])
 
         mox.StubOutWithMock(image_utils, 'resize_image')
-        image_utils.resize_image(volpath, new_vol_size).AndRaise(
-            processutils.ProcessExecutionError('error'))
+        image_utils.resize_image(volpath, new_vol_size, run_as_root=True).\
+            AndRaise(processutils.ProcessExecutionError('error'))
         mox.ReplayAll()
 
         self.assertRaises(exception.VolumeBackendAPIException,
@@ -537,8 +537,7 @@ class GPFSDriverTestCase(test.TestCase):
         data.virtual_size = 1 * units.GiB
         return data
 
-    def _fake_qemu_image_resize(self, path, size):
-        LOG.info('wtf')
+    def _fake_qemu_image_resize(self, path, size, run_as_root=False):
         pass
 
     def _fake_delete_gpfs_file(self, fchild):

diff --git a/cinder/volume/drivers/gpfs.py b/cinder/volume/drivers/gpfs.py
index e965b8df322101d11479441e307b614310f8c06a..28bf324066dec326020e342a89562c3fea0a5fb6 100644 (file)
--- a/cinder/volume/drivers/gpfs.py
+++ b/cinder/volume/drivers/gpfs.py
@@ -132,6 +132,10 @@ class GPFSDriver(driver.VolumeDriver):
             return True
         return False
 
+    def _set_rw_permission(self, path, modebits='660'):
+        """Set permission bits for the path."""
+        self._execute('chmod', modebits, path, run_as_root=True)
+
     def check_for_setup_error(self):
         """Returns an error if prerequisites aren't met."""
         self._check_gpfs_state()
@@ -208,7 +212,6 @@ class GPFSDriver(driver.VolumeDriver):
 
         sizestr = self._sizestr(size)
         self._execute('truncate', '-s', sizestr, path, run_as_root=True)
-        self._execute('chmod', '666', path, run_as_root=True)
 
     def _allocate_file_blocks(self, path, size):
         """Preallocate file blocks by writing zeros."""
@@ -259,7 +262,7 @@ class GPFSDriver(driver.VolumeDriver):
 
         # Create a sparse file first; allocate blocks later if requested
         self._create_sparse_file(volume_path, volume_size)
-
+        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')
@@ -283,6 +286,7 @@ class GPFSDriver(driver.VolumeDriver):
         volume_path = self.local_path(volume)
         snapshot_path = self.local_path(snapshot)
         self._create_gpfs_copy(src=snapshot_path, dest=volume_path)
+        self._set_rw_permission(volume_path)
         self._gpfs_redirect(volume_path)
         virt_size = self._resize_volume_file(volume, volume['size'])
         return {'size': math.ceil(virt_size / units.GiB)}
@@ -291,6 +295,7 @@ class GPFSDriver(driver.VolumeDriver):
         src = self.local_path(src_vref)
         dest = self.local_path(volume)
         self._create_gpfs_clone(src, dest)
+        self._set_rw_permission(dest)
         virt_size = self._resize_volume_file(volume, volume['size'])
         return {'size': math.ceil(virt_size / units.GiB)}
 
@@ -360,17 +365,14 @@ class GPFSDriver(driver.VolumeDriver):
         if(self._gpfs_redirect(src) and self._gpfs_redirect(dest)):
             self._execute('rm', '-f', snap, run_as_root=True)
 
-    def _create_gpfs_copy(self, src, dest, modebits='666'):
+    def _create_gpfs_copy(self, src, dest):
         self._execute('mmclone', 'copy', src, dest, run_as_root=True)
-        self._execute('chmod', modebits, dest, run_as_root=True)
 
-    def _create_gpfs_snap(self, src, dest=None, modebits='644'):
+    def _create_gpfs_snap(self, src, dest=None):
         if dest is None:
             self._execute('mmclone', 'snap', src, run_as_root=True)
-            self._execute('chmod', modebits, src, run_as_root=True)
         else:
             self._execute('mmclone', 'snap', src, dest, run_as_root=True)
-            self._execute('chmod', modebits, dest, run_as_root=True)
 
     def _is_gpfs_parent_file(self, gpfs_file):
         out, _ = self._execute('mmclone', 'show', gpfs_file, run_as_root=True)
@@ -383,6 +385,7 @@ class GPFSDriver(driver.VolumeDriver):
         volume_path = os.path.join(self.configuration.gpfs_mount_point_base,
                                    snapshot['volume_name'])
         self._create_gpfs_snap(src=volume_path, dest=snapshot_path)
+        self._set_rw_permission(snapshot_path, modebits='640')
         self._gpfs_redirect(volume_path)
 
     def delete_snapshot(self, snapshot):
@@ -498,7 +501,7 @@ class GPFSDriver(driver.VolumeDriver):
         vol_path = self.local_path(volume)
         # if the image is not already a GPFS snap file make it so
         if not self._is_gpfs_parent_file(image_path):
-            self._create_gpfs_snap(image_path, modebits='666')
+            self._create_gpfs_snap(image_path)
 
         data = image_utils.qemu_img_info(image_path)
 
@@ -514,15 +517,14 @@ class GPFSDriver(driver.VolumeDriver):
                 LOG.debug('Clone image to vol %s using copyfile' %
                           volume['id'])
                 shutil.copyfile(image_path, vol_path)
-                self._execute('chmod', '666', vol_path, run_as_root=True)
 
         # if image is not raw convert it to raw into vol_path destination
         else:
             LOG.debug('Clone image to vol %s using qemu convert' %
                       volume['id'])
             image_utils.convert_image(image_path, vol_path, 'raw')
-            self._execute('chmod', '666', vol_path, run_as_root=True)
 
+        self._set_rw_permission(vol_path)
         self._resize_volume_file(volume, volume['size'])
 
         return {'provider_location': None}, True
@@ -551,7 +553,7 @@ class GPFSDriver(driver.VolumeDriver):
         """Resize volume file to new size."""
         vol_path = self.local_path(volume)
         try:
-            image_utils.resize_image(vol_path, new_size)
+            image_utils.resize_image(vol_path, new_size, run_as_root=True)
         except processutils.ProcessExecutionError as exc:
             LOG.error(_("Failed to resize volume "
                         "%(volume_id)s, error: %(error)s") %