openstack/cinder
Code Issues Proposed changes

Merge "SMBFS: enhance volume cloning"

Browse Source
This commit is contained in:
Jenkins 2017-06-30 00:15:20 +00:00 committed by Gerrit Code Review
commit 31c695980a
4 changed files with 168 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
cinder/tests/unit/volume/drivers/test_remotefs.py
View File

@ -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)

6
cinder/tests/unit/windows/test_smbfs.py
View File

@ -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}

77
cinder/volume/drivers/remotefs.py
View File

@ -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."""

14
cinder/volume/drivers/windows/smbfs.py
View File

@ -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]