SMBFS: enhance volume cloning
This change enhances the volume clone operation by simply copying the image file if the cloned volume doesn't have any snapshots. This way, we don't have to convert a chain of images. Also, this means that we may benefit from copy offloading. This improvement especially helps image caching, which relies on efficient volume cloning. This is basically implemented in the base remotefs module, any similar driver leveraging it may enable it by switching a flag, assuming that it implements the required methods. Note that the copied image needs to be extended according to the new volume size. We cannot simply call the 'extend_volume' method as most of the RemoteFS based drivers will apply a lock to it, in which case we'd get into a deadlock. For this reason, we follow the common practice among those drivers, involving a helper that won't get a lock decorator. Change-Id: I96ac2f019d522ebbeb6c28fc4c11b41d9ba8fdc0
This commit is contained in:
Lucian Petrut
parent
1e8d7fda4f
commit
1dd3dea3cd
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -709,6 +709,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}
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
@ -396,14 +398,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.',
|
||||
|
|
@ -542,6 +539,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]
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue