GlusterFS: Use image_utils for tempfile creation

Besides the aesthetical aspect of replacing ad-hoc code
with an utility function, it's more correct
to perform image conversions in CONF.image_conversion_dir,
which is taken care by the image_utils.temporary_file()
context guard.

Change-Id: Ied5607156f0ab1f1f8cfc889d5b57f3486d6b407

diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py
index 8891c1b081dc5234a1b706367ca7f9943520b882..3d1e8570a814590e6b5dd8ca7d8267226df16089 100644 (file)
--- a/cinder/tests/test_glusterfs.py
+++ b/cinder/tests/test_glusterfs.py
@@ -81,6 +81,7 @@ class GlusterFsDriverTestCase(test.TestCase):
     TEST_LOCAL_PATH = '/mnt/glusterfs/volume-123'
     TEST_FILE_NAME = 'test.txt'
     TEST_SHARES_CONFIG_FILE = '/etc/cinder/test-shares.conf'
+    TEST_TMP_FILE = '/tmp/tempfile'
     VOLUME_UUID = 'abcdefab-cdef-abcd-efab-cdefabcdefab'
     SNAP_UUID = 'bacadaca-baca-daca-baca-dacadacadaca'
     SNAP_UUID_2 = 'bebedede-bebe-dede-bebe-dedebebedede'
@@ -2221,18 +2222,23 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         volume = self._simple_volume()
         volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume['name'])
+        image_meta = {'id': '10958016-e196-42e3-9e7f-5d8927ae3099'}
 
         with contextlib.nested(
             mock.patch.object(drv, 'get_active_image_from_info'),
             mock.patch.object(drv, '_local_volume_dir'),
             mock.patch.object(image_utils, 'qemu_img_info'),
-            mock.patch.object(image_utils, 'upload_volume')
+            mock.patch.object(image_utils, 'upload_volume'),
+            mock.patch.object(image_utils, 'create_temporary_file')
         ) as (mock_get_active_image_from_info, mock_local_volume_dir,
-              mock_qemu_img_info, mock_upload_volume):
+              mock_qemu_img_info, mock_upload_volume,
+              mock_create_temporary_file):
             mock_get_active_image_from_info.return_value = volume['name']
 
             mock_local_volume_dir.return_value = self.TEST_MNT_POINT
 
+            mock_create_temporary_file.return_value = self.TEST_TMP_FILE
+
             qemu_img_output = """image: %s
             file format: raw
             virtual size: 1.0G (1073741824 bytes)
@@ -2243,13 +2249,14 @@ class GlusterFsDriverTestCase(test.TestCase):
 
             upload_path = volume_path
 
-            drv.copy_volume_to_image(mock.ANY, volume, mock.ANY, mock.ANY)
+            drv.copy_volume_to_image(mock.ANY, volume, mock.ANY, image_meta)
 
             mock_get_active_image_from_info.assert_called_once_with(volume)
             mock_local_volume_dir.assert_called_once_with(volume)
             mock_qemu_img_info.assert_called_once_with(volume_path)
             mock_upload_volume.assert_called_once_with(
                 mock.ANY, mock.ANY, mock.ANY, upload_path)
+            mock_create_temporary_file.assert_once_called_with()
 
     def test_copy_volume_to_image_qcow2_image(self):
         """Upload a qcow2 image file which has to be converted to raw first."""
@@ -2265,14 +2272,16 @@ class GlusterFsDriverTestCase(test.TestCase):
             mock.patch.object(image_utils, 'qemu_img_info'),
             mock.patch.object(image_utils, 'convert_image'),
             mock.patch.object(image_utils, 'upload_volume'),
-            mock.patch.object(drv, '_execute')
+            mock.patch.object(image_utils, 'create_temporary_file')
         ) as (mock_get_active_image_from_info, mock_local_volume_dir,
               mock_qemu_img_info, mock_convert_image, mock_upload_volume,
-              mock_execute):
+              mock_create_temporary_file):
             mock_get_active_image_from_info.return_value = volume['name']
 
             mock_local_volume_dir.return_value = self.TEST_MNT_POINT
 
+            mock_create_temporary_file.return_value = self.TEST_TMP_FILE
+
             qemu_img_output = """image: %s
             file format: qcow2
             virtual size: 1.0G (1073741824 bytes)
@@ -2281,9 +2290,7 @@ class GlusterFsDriverTestCase(test.TestCase):
             img_info = imageutils.QemuImgInfo(qemu_img_output)
             mock_qemu_img_info.return_value = img_info
 
-            upload_path = '%s/%s.temp_image.%s' % (self.TEST_MNT_POINT,
-                                                   volume['id'],
-                                                   image_meta['id'])
+            upload_path = self.TEST_TMP_FILE
 
             drv.copy_volume_to_image(mock.ANY, volume, mock.ANY, image_meta)
 
@@ -2294,7 +2301,7 @@ class GlusterFsDriverTestCase(test.TestCase):
                 volume_path, upload_path, 'raw')
             mock_upload_volume.assert_called_once_with(
                 mock.ANY, mock.ANY, mock.ANY, upload_path)
-            mock_execute.assert_called_once_with('rm', '-f', upload_path)
+            mock_create_temporary_file.assert_once_called_with()
 
     def test_copy_volume_to_image_snapshot_exists(self):
         """Upload an active snapshot which has to be converted to raw first."""
@@ -2311,14 +2318,16 @@ class GlusterFsDriverTestCase(test.TestCase):
             mock.patch.object(image_utils, 'qemu_img_info'),
             mock.patch.object(image_utils, 'convert_image'),
             mock.patch.object(image_utils, 'upload_volume'),
-            mock.patch.object(drv, '_execute')
+            mock.patch.object(image_utils, 'create_temporary_file')
         ) as (mock_get_active_image_from_info, mock_local_volume_dir,
               mock_qemu_img_info, mock_convert_image, mock_upload_volume,
-              mock_execute):
+              mock_create_temporary_file):
             mock_get_active_image_from_info.return_value = volume['name']
 
             mock_local_volume_dir.return_value = self.TEST_MNT_POINT
 
+            mock_create_temporary_file.return_value = self.TEST_TMP_FILE
+
             qemu_img_output = """image: volume-%s.%s
             file format: qcow2
             virtual size: 1.0G (1073741824 bytes)
@@ -2328,9 +2337,7 @@ class GlusterFsDriverTestCase(test.TestCase):
             img_info = imageutils.QemuImgInfo(qemu_img_output)
             mock_qemu_img_info.return_value = img_info
 
-            upload_path = '%s/%s.temp_image.%s' % (self.TEST_MNT_POINT,
-                                                   volume['id'],
-                                                   image_meta['id'])
+            upload_path = self.TEST_TMP_FILE
 
             drv.copy_volume_to_image(mock.ANY, volume, mock.ANY, image_meta)
 
@@ -2341,4 +2348,4 @@ class GlusterFsDriverTestCase(test.TestCase):
                 volume_path, upload_path, 'raw')
             mock_upload_volume.assert_called_once_with(
                 mock.ANY, mock.ANY, mock.ANY, upload_path)
-            mock_execute.assert_called_once_with('rm', '-f', upload_path)
+            mock_create_temporary_file.assert_once_called_with()

diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py
index 13f30f73d8774798f93de1eb8807d909ce890f12..af16cef3f30f4a4e19b111b0d4989d7e68a902c7 100644 (file)
--- a/cinder/volume/drivers/glusterfs.py
+++ b/cinder/volume/drivers/glusterfs.py
@@ -1016,18 +1016,15 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
 
         root_file_fmt = info.file_format
 
-        temp_path = None
-
-        try:
+        tmp_params = {
+            'prefix': '%s.temp_image.%s' % (volume['id'], image_meta['id']),
+            'suffix': '.img'
+        }
+        with image_utils.temporary_file(**tmp_params) as temp_path:
             if snapshots_exist or (root_file_fmt != 'raw'):
                 # Convert due to snapshots
                 # or volume data not being stored in raw format
                 #  (upload_volume assumes raw format input)
-                temp_path = '%s/%s.temp_image.%s' % (
-                    self._local_volume_dir(volume),
-                    volume['id'],
-                    image_meta['id'])
-
                 image_utils.convert_image(active_file_path, temp_path, 'raw')
                 upload_path = temp_path
             else:
@@ -1037,9 +1034,6 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
                                       image_service,
                                       image_meta,
                                       upload_path)
-        finally:
-            if temp_path is not None:
-                self._execute('rm', '-f', temp_path)
 
     @utils.synchronized('glusterfs', external=False)
     def extend_volume(self, volume, size_gb):