From: Csaba Henk Date: Sat, 16 Aug 2014 15:49:00 +0000 (+0100) Subject: GlusterFS: Use image_utils for tempfile creation X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=875ce5238fb1669a61305c8160381cc831a1a57f;p=openstack-build%2Fcinder-build.git 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 8891c1b08..3d1e8570a 100644 --- 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 13f30f73d..af16cef3f 100644 --- 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):