diff --git a/cinder/tests/unit/volume/drivers/test_remotefs.py b/cinder/tests/unit/volume/drivers/test_remotefs.py
index 3f276305a3d..c36e8a70d67 100644
--- a/cinder/tests/unit/volume/drivers/test_remotefs.py
+++ b/cinder/tests/unit/volume/drivers/test_remotefs.py
@@ -460,8 +460,65 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
basedir=basedir,
valid_backing_file=False)
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_local_volume_dir')
@mock.patch.object(remotefs.RemoteFSSnapDriver,
- '_validate_state')
+ 'get_active_image_from_info')
+ def test_local_path_active_image(self, mock_get_active_img,
+ mock_local_vol_dir):
+ fake_vol_dir = 'fake_vol_dir'
+ fake_active_img = 'fake_active_img_fname'
+
+ mock_get_active_img.return_value = fake_active_img
+ mock_local_vol_dir.return_value = fake_vol_dir
+
+ active_img_path = self._driver._local_path_active_image(
+ mock.sentinel.volume)
+ exp_act_img_path = os.path.join(fake_vol_dir, fake_active_img)
+
+ self.assertEqual(exp_act_img_path, active_img_path)
+ mock_get_active_img.assert_called_once_with(mock.sentinel.volume)
+ mock_local_vol_dir.assert_called_once_with(mock.sentinel.volume)
+
+ @ddt.data({},
+ {'provider_location': None},
+ {'active_fpath': 'last_snap_img',
+ 'expect_snaps': True})
+ @ddt.unpack
+ @mock.patch.object(remotefs.RemoteFSSnapDriver,
+ '_local_path_active_image')
+ @mock.patch.object(remotefs.RemoteFSSnapDriver,
+ 'local_path')
+ def test_snapshots_exist(self, mock_local_path,
+ mock_local_path_active_img,
+ provider_location='fake_share',
+ active_fpath='base_img_path',
+ base_vol_path='base_img_path',
+ expect_snaps=False):
+ self._fake_volume.provider_location = provider_location
+
+ mock_local_path.return_value = base_vol_path
+ mock_local_path_active_img.return_value = active_fpath
+
+ snaps_exist = self._driver._snapshots_exist(self._fake_volume)
+
+ self.assertEqual(expect_snaps, snaps_exist)
+
+ if provider_location:
+ mock_local_path.assert_called_once_with(self._fake_volume)
+ mock_local_path_active_img.assert_called_once_with(
+ self._fake_volume)
+ else:
+ self.assertFalse(mock_local_path.called)
+
+ @ddt.data({},
+ {'snapshots_exist': True},
+ {'force_temp_snap': True})
+ @ddt.unpack
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, 'local_path')
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_snapshots_exist')
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_copy_volume_image')
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_extend_volume')
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_validate_state')
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_create_snapshot')
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_delete_snapshot')
@mock.patch.object(remotefs.RemoteFSSnapDriver,
@@ -469,7 +526,13 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
def test_create_cloned_volume(self, mock_copy_volume_from_snapshot,
mock_delete_snapshot,
mock_create_snapshot,
- mock_validate_state):
+ mock_validate_state,
+ mock_extend_volme,
+ mock_copy_volume_image,
+ mock_snapshots_exist,
+ mock_local_path,
+ snapshots_exist=False,
+ force_temp_snap=False):
drv = self._driver
volume = fake_volume.fake_volume_obj(self.context)
@@ -479,6 +542,9 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
id=src_vref_id,
name='volume-%s' % src_vref_id)
+ mock_snapshots_exist.return_value = snapshots_exist
+ drv._always_use_temp_snap_when_cloning = force_temp_snap
+
vol_attrs = ['provider_location', 'size', 'id', 'name', 'status',
'volume_type', 'metadata']
Volume = collections.namedtuple('Volume', vol_attrs)
@@ -509,10 +575,33 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
src_vref.status,
exp_acceptable_states,
obj_description='source volume')
- mock_create_snapshot.assert_called_once_with(snap_ref)
- mock_copy_volume_from_snapshot.assert_called_once_with(
- snap_ref, volume_ref, volume['size'])
- self.assertTrue(mock_delete_snapshot.called)
+
+ if snapshots_exist or force_temp_snap:
+ mock_create_snapshot.assert_called_once_with(snap_ref)
+ mock_copy_volume_from_snapshot.assert_called_once_with(
+ snap_ref, volume_ref, volume['size'])
+ self.assertTrue(mock_delete_snapshot.called)
+ else:
+ self.assertFalse(mock_create_snapshot.called)
+
+ mock_snapshots_exist.assert_called_once_with(src_vref)
+
+ mock_copy_volume_image.assert_called_once_with(
+ mock_local_path.return_value,
+ mock_local_path.return_value)
+ mock_local_path.assert_has_calls(
+ [mock.call(src_vref), mock.call(volume_ref)])
+ mock_extend_volme.assert_called_once_with(volume_ref,
+ volume.size)
+
+ @mock.patch('shutil.copyfile')
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_set_rw_permissions')
+ def test_copy_volume_image(self, mock_set_perm, mock_copyfile):
+ self._driver._copy_volume_image(mock.sentinel.src, mock.sentinel.dest)
+
+ mock_copyfile.assert_called_once_with(mock.sentinel.src,
+ mock.sentinel.dest)
+ mock_set_perm.assert_called_once_with(mock.sentinel.dest)
def test_create_regular_file(self):
self._driver._create_regular_file('/path', 1)
diff --git a/cinder/tests/unit/windows/test_smbfs.py b/cinder/tests/unit/windows/test_smbfs.py
index ab7e76b50c5..aedefcf394e 100644
--- a/cinder/tests/unit/windows/test_smbfs.py
+++ b/cinder/tests/unit/windows/test_smbfs.py
@@ -719,6 +719,12 @@ class WindowsSmbFsTestCase(test.TestCase):
drv._vhdutils.reconnect_parent_vhd.assert_called_once_with(
self._FAKE_SNAPSHOT_PATH, self._FAKE_VOLUME_PATH)
+ def test_copy_volume_image(self):
+ self._smbfs_driver._copy_volume_image(mock.sentinel.src,
+ mock.sentinel.dest)
+ self._smbfs_driver._pathutils.copy.assert_called_once_with(
+ mock.sentinel.src, mock.sentinel.dest)
+
def test_get_pool_name_from_share(self):
self._smbfs_driver._pool_mappings = {
mock.sentinel.share: mock.sentinel.pool}
diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py
index d771f50b464..0052c16ce5f 100644
--- a/cinder/volume/drivers/remotefs.py
+++ b/cinder/volume/drivers/remotefs.py
@@ -20,6 +20,7 @@ import inspect
import json
import os
import re
+import shutil
import tempfile
import time
@@ -671,6 +672,11 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
"""
_VALID_IMAGE_EXTENSIONS = []
+ # The following flag may be overriden by the concrete drivers in order
+ # to avoid using temporary volume snapshots when creating volume clones,
+ # when possible.
+
+ _always_use_temp_snap_when_cloning = True
def __init__(self, *args, **kwargs):
self._remotefsclient = None
@@ -955,6 +961,22 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
return snap_info['active']
+ def _local_path_active_image(self, volume):
+ active_fname = self.get_active_image_from_info(volume)
+ vol_dir = self._local_volume_dir(volume)
+
+ active_fpath = os.path.join(vol_dir, active_fname)
+ return active_fpath
+
+ def _snapshots_exist(self, volume):
+ if not volume.provider_location:
+ return False
+
+ active_fpath = self._local_path_active_image(volume)
+ base_vol_path = self.local_path(volume)
+
+ return not utils.paths_normcase_equal(active_fpath, base_vol_path)
+
def _create_cloned_volume(self, volume, src_vref):
LOG.info('Cloning volume %(src)s to volume %(dst)s',
{'src': src_vref.id,
@@ -972,10 +994,6 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
'volume_type', 'metadata']
Volume = collections.namedtuple('Volume', vol_attrs)
- snap_attrs = ['volume_name', 'volume_size', 'name',
- 'volume_id', 'id', 'volume']
- Snapshot = collections.namedtuple('Snapshot', snap_attrs)
-
volume_info = Volume(provider_location=src_vref.provider_location,
size=src_vref.size,
id=volume.id,
@@ -984,24 +1002,38 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
volume_type=src_vref.volume_type,
metadata=src_vref.metadata)
- temp_snapshot = Snapshot(volume_name=volume_name,
- volume_size=src_vref.size,
- name='clone-snap-%s' % src_vref.id,
- volume_id=src_vref.id,
- id='tmp-snap-%s' % src_vref.id,
- volume=src_vref)
+ if (self._always_use_temp_snap_when_cloning or
+ self._snapshots_exist(src_vref)):
+ snap_attrs = ['volume_name', 'volume_size', 'name',
+ 'volume_id', 'id', 'volume']
+ Snapshot = collections.namedtuple('Snapshot', snap_attrs)
- self._create_snapshot(temp_snapshot)
- try:
- self._copy_volume_from_snapshot(temp_snapshot,
- volume_info,
- volume.size)
+ temp_snapshot = Snapshot(volume_name=volume_name,
+ volume_size=src_vref.size,
+ name='clone-snap-%s' % src_vref.id,
+ volume_id=src_vref.id,
+ id='tmp-snap-%s' % src_vref.id,
+ volume=src_vref)
- finally:
- self._delete_snapshot(temp_snapshot)
+ self._create_snapshot(temp_snapshot)
+ try:
+ self._copy_volume_from_snapshot(temp_snapshot,
+ volume_info,
+ volume.size)
+
+ finally:
+ self._delete_snapshot(temp_snapshot)
+ else:
+ self._copy_volume_image(self.local_path(src_vref),
+ self.local_path(volume_info))
+ self._extend_volume(volume_info, volume.size)
return {'provider_location': src_vref.provider_location}
+ def _copy_volume_image(self, src_path, dest_path):
+ shutil.copyfile(src_path, dest_path)
+ self._set_rw_permissions(dest_path)
+
def _delete_stale_snapshot(self, snapshot):
info_path = self._local_path_volume_info(snapshot.volume)
snap_info = self._read_info_file(info_path)
@@ -1544,6 +1576,9 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
{'id': snapshot.id}
raise exception.RemoteFSException(msg)
+ def _extend_volume(self, volume, size_gb):
+ raise NotImplementedError()
+
class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
@locked_volume_id_operation
@@ -1575,6 +1610,10 @@ class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
return self._copy_volume_to_image(context, volume, image_service,
image_meta)
+ @locked_volume_id_operation
+ def extend_volume(self, volume, size_gb):
+ return self._extend_volume(volume, size_gb)
+
class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
@coordination.synchronized('{self.driver_prefix}-{snapshot.volume.id}')
@@ -1606,6 +1645,10 @@ class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
return self._copy_volume_to_image(context, volume, image_service,
image_meta)
+ @coordination.synchronized('{self.driver_prefix}-{volume.id}')
+ def extend_volume(self, volume, size_gb):
+ return self._extend_volume(volume, size_gb)
+
class RemoteFSPoolMixin(object):
"""Drivers inheriting this will report each share as a pool."""
diff --git a/cinder/volume/drivers/windows/smbfs.py b/cinder/volume/drivers/windows/smbfs.py
index f44b0922acc..2abb630df2b 100644
--- a/cinder/volume/drivers/windows/smbfs.py
+++ b/cinder/volume/drivers/windows/smbfs.py
@@ -105,6 +105,8 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
_SUPPORTED_IMAGE_FORMATS = [_DISK_FORMAT_VHD, _DISK_FORMAT_VHDX]
_VALID_IMAGE_EXTENSIONS = _SUPPORTED_IMAGE_FORMATS
+ _always_use_temp_snap_when_cloning = False
+
def __init__(self, *args, **kwargs):
self._remotefsclient = None
super(WindowsSmbfsDriver, self).__init__(*args, **kwargs)
@@ -398,14 +400,9 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
self._vhdutils.create_differencing_vhd(new_snap_path,
backing_file_full_path)
- @coordination.synchronized('{self.driver_prefix}-{volume.id}')
- def extend_volume(self, volume, size_gb):
- LOG.info('Extending volume %s.', volume.id)
-
- self._check_extend_volume_support(volume, size_gb)
- self._extend_volume(volume, size_gb)
-
def _extend_volume(self, volume, size_gb):
+ self._check_extend_volume_support(volume, size_gb)
+
volume_path = self.local_path(volume)
LOG.info('Resizing file %(volume_path)s to %(size_gb)sGB.',
@@ -544,6 +541,9 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
self._vhdutils.resize_vhd(volume_path, volume_size * units.Gi,
is_file_max_size=False)
+ def _copy_volume_image(self, src_path, dest_path):
+ self._pathutils.copy(src_path, dest_path)
+
def _get_share_name(self, share):
return share.replace('/', '\\').lstrip('\\').split('\\', 1)[1]