From 5f5a37d31a4466e32fad39000ecf71156cc09b2d Mon Sep 17 00:00:00 2001 From: Bill Owen Date: Fri, 16 Aug 2013 15:49:06 -0700 Subject: [PATCH] GPFS use clone_image for creating volumes If both source and target of gpfs create volume from image operation are backed by gpfs storage use clone_image method for implementing the move of image data to the new volume. The copy_image_to_volume method is used only if this is not true, and uses image_utils.fetch_to_raw to move image data to the new volume. Fixes bug #1213248 Change-Id: I3958febe67cc86bc3cb608288f7d064f74d3a731 --- cinder/tests/test_gpfs.py | 38 +++++---- cinder/volume/drivers/gpfs.py | 149 ++++++++++++++++++---------------- 2 files changed, 98 insertions(+), 89 deletions(-) diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py index 171649e8b..d387a3395 100644 --- a/cinder/tests/test_gpfs.py +++ b/cinder/tests/test_gpfs.py @@ -39,6 +39,11 @@ class FakeImageService(): def update(self, context, image_id, path): pass + def show(self, context, image_id): + image_meta = {'disk_format': None, + 'container_format': None} + return image_meta + def download(self, context, image_id, image_fd): for b in range(256): image_fd.write('some_image_data') @@ -88,6 +93,8 @@ class GPFSDriverTestCase(test.TestCase): self._fake_gpfs_redirect) self.stubs.Set(GPFSDriver, '_is_gpfs_parent_file', self._fake_is_gpfs_parent) + self.stubs.Set(GPFSDriver, '_is_gpfs_path', + self._fake_is_gpfs_path) self.stubs.Set(GPFSDriver, '_delete_gpfs_file', self._fake_delete_gpfs_file) self.stubs.Set(GPFSDriver, '_create_sparse_file', @@ -100,6 +107,8 @@ class GPFSDriverTestCase(test.TestCase): self._fake_qemu_qcow2_image_info) self.stubs.Set(image_utils, 'convert_image', self._fake_convert_image) + self.stubs.Set(image_utils, 'resize_image', + self._fake_qemu_image_resize) self.context = context.get_admin_context() CONF.gpfs_images_dir = self.images_dir @@ -247,7 +256,7 @@ class GPFSDriverTestCase(test.TestCase): self.volume.delete_volume(self.context, volume_src['id']) self.volume.delete_snapshot(self.context, snapshot_id) - def test_copy_image_to_volume_with_copy_on_write_mode(self): + def test_clone_image_to_volume_with_copy_on_write_mode(self): """Test the function of copy_image_to_volume focusing on the integretion of the image_util using copy_on_write image sharing mode. @@ -260,16 +269,15 @@ class GPFSDriverTestCase(test.TestCase): volume = self._create_volume() volumepath = os.path.join(self.volumes_path, volume['name']) CONF.gpfs_images_share_mode = 'copy_on_write' - self.driver.copy_image_to_volume(self.context, - volume, - FakeImageService(), - self.image_id) + self.driver.clone_image(volume, + None, + self.image_id) self.assertTrue(os.path.exists(volumepath)) self.volume.delete_volume(self.context, volume['id']) self.assertFalse(os.path.exists(volumepath)) - def test_copy_image_to_volume_with_copy_mode(self): + def test_clone_image_to_volume_with_copy_mode(self): """Test the function of copy_image_to_volume focusing on the integretion of the image_util using copy image sharing mode. @@ -282,10 +290,9 @@ class GPFSDriverTestCase(test.TestCase): volume = self._create_volume() volumepath = os.path.join(self.volumes_path, volume['name']) CONF.gpfs_images_share_mode = 'copy' - self.driver.copy_image_to_volume(self.context, - volume, - FakeImageService(), - self.image_id) + self.driver.clone_image(volume, + None, + self.image_id) self.assertTrue(os.path.exists(volumepath)) self.volume.delete_volume(self.context, volume['id']) @@ -340,8 +347,6 @@ class GPFSDriverTestCase(test.TestCase): def test_check_for_setup_error_ok(self): self.stubs.Set(GPFSDriver, '_get_gpfs_state', self._fake_gpfs_get_state_active) - self.stubs.Set(GPFSDriver, '_is_gpfs_path', - self._fake_is_gpfs_path) self.stubs.Set(GPFSDriver, '_get_gpfs_cluster_release_level', self._fake_gpfs_compatible_cluster_release_level) self.stubs.Set(GPFSDriver, '_get_gpfs_filesystem_release_level', @@ -351,8 +356,6 @@ class GPFSDriverTestCase(test.TestCase): def test_check_for_setup_error_gpfs_not_active(self): self.stubs.Set(GPFSDriver, '_get_gpfs_state', self._fake_gpfs_get_state_not_active) - self.stubs.Set(GPFSDriver, '_is_gpfs_path', - self._fake_is_gpfs_path) self.assertRaises(exception.VolumeBackendAPIException, self.driver.check_for_setup_error) @@ -371,8 +374,6 @@ class GPFSDriverTestCase(test.TestCase): def test_check_for_setup_error_incompatible_cluster_version(self): self.stubs.Set(GPFSDriver, '_get_gpfs_state', self._fake_gpfs_get_state_active) - self.stubs.Set(GPFSDriver, '_is_gpfs_path', - self._fake_is_gpfs_path) self.stubs.Set(GPFSDriver, '_get_gpfs_cluster_release_level', self._fake_gpfs_incompatible_cluster_release_level) self.assertRaises(exception.VolumeBackendAPIException, @@ -381,8 +382,6 @@ class GPFSDriverTestCase(test.TestCase): def test_check_for_setup_error_incompatible_filesystem_version(self): self.stubs.Set(GPFSDriver, '_get_gpfs_state', self._fake_gpfs_get_state_active) - self.stubs.Set(GPFSDriver, '_is_gpfs_path', - self._fake_is_gpfs_path) self.stubs.Set(GPFSDriver, '_get_gpfs_cluster_release_level', self._fake_gpfs_compatible_cluster_release_level) self.stubs.Set(GPFSDriver, '_get_gpfs_filesystem_release_level', @@ -476,6 +475,9 @@ class GPFSDriverTestCase(test.TestCase): data.virtual_size = 1024 * 1024 * 1024 return data + def _fake_qemu_image_resize(self, path, size): + pass + def _fake_delete_gpfs_file(self, fchild): volume_path = fchild vol_name = os.path.basename(fchild) diff --git a/cinder/volume/drivers/gpfs.py b/cinder/volume/drivers/gpfs.py index b0c76b46a..144e1c1d0 100644 --- a/cinder/volume/drivers/gpfs.py +++ b/cinder/volume/drivers/gpfs.py @@ -21,6 +21,7 @@ GPFS Volume Driver. import math import os import re +import shutil from oslo.config import cfg @@ -415,10 +416,84 @@ class GPFSDriver(driver.VolumeDriver): return '100M' return '%sG' % size_in_g + def clone_image(self, volume, image_location, image_id): + return self._clone_image(volume, image_location, image_id) + + def _is_cloneable(self, image_id): + if not((self.configuration.gpfs_images_dir and + self.configuration.gpfs_images_share_mode)): + reason = 'glance repository not configured to use GPFS' + return False, reason, None + + image_path = os.path.join(self.configuration.gpfs_images_dir, image_id) + try: + self._is_gpfs_path(image_path) + except exception.ProcessExecutionError: + reason = 'image file not in GPFS' + return False, reason, None + + return True, None, image_path + + def _clone_image(self, volume, image_location, image_id): + """Attempt to create a volume by efficiently copying image to volume. + + If both source and target are backed by gpfs storage and the source + image is in raw format move the image to create a volume using either + gpfs clone operation or with a file copy. If the image format is not + raw, convert it to raw at the volume path. + """ + cloneable_image, reason, image_path = self._is_cloneable(image_id) + if not cloneable_image: + LOG.debug('Image %(img)s not cloneable: %(reas)s' % + {'img': image_id, 'reas': reason}) + return (None, False) + + 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') + + data = image_utils.qemu_img_info(image_path) + + # if image format is already raw either clone it or + # copy it depending on config file settings + if data.file_format == 'raw': + if (self.configuration.gpfs_images_share_mode == + 'copy_on_write'): + LOG.debug('Clone image to vol %s using mmclone' % + volume['id']) + self._create_gpfs_copy(image_path, vol_path) + elif self.configuration.gpfs_images_share_mode == 'copy': + 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) + + image_utils.resize_image(vol_path, volume['size']) + + return {'provider_location': None}, True + def copy_image_to_volume(self, context, volume, image_service, image_id): - """Fetch the image from image_service and write it to the volume.""" - return self._gpfs_fetch_to_raw(context, image_service, image_id, - self.local_path(volume)) + """Fetch the image from image_service and write it to the volume. + + Note that cinder.volume.flows.create_volume will attempt to use + clone_image to efficiently create volume from image when both + source and target are backed by gpfs storage. If that is not the + case, this function is invoked and uses fetch_to_raw to create the + volume. + """ + LOG.debug('Copy image to vol %s using image_utils fetch_to_raw' % + volume['id']) + image_utils.fetch_to_raw(context, image_service, image_id, + self.local_path(volume)) + image_utils.resize_image(self.local_path(volume), volume['size']) def copy_volume_to_image(self, context, volume, image_service, image_meta): """Copy the volume to the specified image.""" @@ -462,71 +537,3 @@ class GPFSDriver(driver.VolumeDriver): size = int(out.split()[1]) available = int(out.split()[3]) return available, size - - def _gpfs_fetch(self, context, image_service, image_id, path, _user_id, - _project_id): - if not (self.configuration.gpfs_images_share_mode and - self.configuration.gpfs_images_dir and - os.path.exists( - os.path.join(self.configuration.gpfs_images_dir, - image_id))): - with fileutils.remove_path_on_error(path): - with open(path, "wb") as image_file: - image_service.download(context, image_id, image_file) - else: - image_path = os.path.join(self.configuration.gpfs_images_dir, - image_id) - if self.configuration.gpfs_images_share_mode == 'copy_on_write': - # check if the image is a GPFS snap file - if not self._is_gpfs_parent_file(image_path): - self._create_gpfs_snap(image_path, modebits='666') - self._execute('ln', '-s', image_path, path, run_as_root=True) - else: # copy - self._execute('cp', image_path, path, run_as_root=True) - self._execute('chmod', '666', path, run_as_root=True) - - def _gpfs_fetch_to_raw(self, context, image_service, image_id, dest, - user_id=None, project_id=None): - if (self.configuration.image_conversion_dir and not - os.path.exists(self.configuration.image_conversion_dir)): - os.makedirs(self.configuration.image_conversion_dir) - - tmp = "%s.part" % dest - with fileutils.remove_path_on_error(tmp): - self._gpfs_fetch(context, image_service, image_id, tmp, user_id, - project_id) - - data = image_utils.qemu_img_info(tmp) - fmt = data.file_format - backing_file = data.backing_file - - if backing_file is not None: - msg = (_("fmt = %(fmt)s backed by: %(backing_file)s") % - {'fmt': fmt, 'backing_file': backing_file}) - LOG.error(msg) - raise exception.ImageUnacceptable( - image_id=image_id, - reason=msg) - - if fmt is None: - msg = _("'qemu-img info' parsing failed.") - LOG.error(msg) - raise exception.ImageUnacceptable( - reason=msg, - image_id=image_id) - elif fmt == 'raw': # already in raw format - just rename to dest - self._execute('mv', tmp, dest, run_as_root=True) - else: # conversion to raw format required - LOG.debug("%s was %s, converting to raw" % (image_id, fmt)) - image_utils.convert_image(tmp, dest, 'raw') - os.unlink(tmp) - - data = image_utils.qemu_img_info(dest) - if data.file_format != "raw": - msg = (_("Expected image to be in raw format, but is %s") % - data.file_format) - LOG.error(msg) - raise exception.ImageUnacceptable( - image_id=image_id, - reason=msg) - return {'size': math.ceil(data.virtual_size / 1024.0 ** 3)} -- 2.45.2