openstack
/
cinder
Code Issues Proposed changes

Fix problem of efficient volume copy for migration 

After introducing commit f586043fa9,
new volume copy method _copy_volume_data() was added into
manager.py. Originally driver.py had this method and it was copied
into manager.py. However new _copy_volume_data() lost efficient
volume copy logic during the reimplementation.

This patch simply add efficient volume copy logic again into new
_copy_volume_data() to fix the problem.

Change-Id: I183cbd2265c1f47c9047818e1d4915c896927280
Closes-Bug: 1491538
This commit is contained in:
Mitsuhiro Tanino 2015-09-02 14:33:18 -04:00
parent 0773a4130d
commit 05047c9167
2 changed files with 85 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
cinder/tests/unit/test_volume.py
View File

@ -4196,6 +4196,69 @@ class VolumeTestCase(BaseVolumeTestCase):
self.assertEqual(expected, azs)
@mock.patch.object(utils, 'brick_get_connector_properties')
@mock.patch.object(cinder.volume.manager.VolumeManager, '_attach_volume')
@mock.patch.object(cinder.volume.manager.VolumeManager, '_detach_volume')
@mock.patch.object(volutils, 'copy_volume')
@mock.patch.object(volume_rpcapi.VolumeAPI, 'get_capabilities')
def test_copy_volume_data(self,
mock_get_capabilities,
mock_copy,
mock_detach,
mock_attach,
mock_get_connector):
"""Test function of _copy_volume_data."""
src_vol = tests_utils.create_volume(self.context, size=1,
host=CONF.host)
dest_vol = tests_utils.create_volume(self.context, size=1,
host=CONF.host)
mock_get_connector.return_value = {}
self.volume.driver._throttle = mock.MagicMock()
attach_expected = [
mock.call(self.context, dest_vol, {}, remote=False),
mock.call(self.context, src_vol, {}, remote=False)]
detach_expected = [
mock.call(self.context, {'device': {'path': 'bar'}},
dest_vol, {}, force=False, remote=False),
mock.call(self.context, {'device': {'path': 'foo'}},
src_vol, {}, force=False, remote=False)]
attach_volume_returns = [
{'device': {'path': 'bar'}},
{'device': {'path': 'foo'}}
]
# Test case for sparse_copy_volume = False
mock_attach.side_effect = attach_volume_returns
mock_get_capabilities.return_value = {}
self.volume._copy_volume_data(self.context,
src_vol,
dest_vol)
self.assertEqual(attach_expected, mock_attach.mock_calls)
mock_copy.assert_called_with('foo', 'bar', 1024, '1M', sparse=False)
self.assertEqual(detach_expected, mock_detach.mock_calls)
# Test case for sparse_copy_volume = True
mock_attach.reset_mock()
mock_detach.reset_mock()
mock_attach.side_effect = attach_volume_returns
mock_get_capabilities.return_value = {'sparse_copy_volume': True}
self.volume._copy_volume_data(self.context,
src_vol,
dest_vol)
self.assertEqual(attach_expected, mock_attach.mock_calls)
mock_copy.assert_called_with('foo', 'bar', 1024, '1M', sparse=True)
self.assertEqual(detach_expected, mock_detach.mock_calls)
# cleanup resource
db.volume_destroy(self.context, src_vol['id'])
db.volume_destroy(self.context, dest_vol['id'])
def test_migrate_volume_driver(self):
"""Test volume migration done by driver."""
# stub out driver and rpc functions
@ -4491,17 +4554,25 @@ class VolumeTestCase(BaseVolumeTestCase):
mock.patch.object(self.volume, 'migrate_volume_completion')\
as mock_migrate_compl,\
mock.patch.object(self.volume.driver, 'create_export'), \
mock.patch.object(self.volume, '_attach_volume'), \
mock.patch.object(self.volume, '_attach_volume') \
as mock_attach, \
mock.patch.object(self.volume, '_detach_volume'), \
mock.patch.object(os_brick.initiator.connector,
'get_connector_properties') \
as mock_get_connector_properties:
as mock_get_connector_properties, \
mock.patch.object(volutils, 'copy_volume') as mock_copy, \
mock.patch.object(volume_rpcapi.VolumeAPI,
'get_capabilities') \
as mock_get_capabilities:
# Exception case at delete_volume
# source_volume['migration_status'] is 'completing'
mock_create_volume.side_effect = self._fake_create_volume
mock_migrate_compl.side_effect = fake_migrate_volume_completion
mock_get_connector_properties.return_value = {}
mock_attach.side_effect = [{'device': {'path': 'bar'}},
{'device': {'path': 'foo'}}]
mock_get_capabilities.return_value = {'sparse_copy_volume': True}
volume = tests_utils.create_volume(self.context, size=0,
host=CONF.host)
host_obj = {'host': 'newhost', 'capabilities': {}}
@ -4514,6 +4585,8 @@ class VolumeTestCase(BaseVolumeTestCase):
volume = db.volume_get(context.get_admin_context(), volume['id'])
self.assertEqual('error', volume['migration_status'])
self.assertEqual('available', volume['status'])
mock_copy.assert_called_once_with('foo', 'bar', 0, '1M',
sparse=True)
def _test_migrate_volume_completion(self, status='available',
instance_uuid=None, attached_host=None,

11
cinder/volume/manager.py
View File

@ -1471,13 +1471,22 @@ class VolumeManager(manager.SchedulerDependentManager):
self._detach_volume(ctxt, dest_attach_info, dest_vol,
properties, remote=dest_remote)
# Check the backend capabilities of migration destination host.
rpcapi = volume_rpcapi.VolumeAPI()
capabilities = rpcapi.get_capabilities(ctxt, dest_vol['host'],
False)
sparse_copy_volume = bool(capabilities and
capabilities.get('sparse_copy_volume',
False))
copy_error = True
try:
size_in_mb = int(src_vol['size']) * units.Ki # vol size is in GB
vol_utils.copy_volume(src_attach_info['device']['path'],
dest_attach_info['device']['path'],
size_in_mb,
self.configuration.volume_dd_blocksize)
self.configuration.volume_dd_blocksize,
sparse=sparse_copy_volume)
copy_error = False
except Exception:
with excutils.save_and_reraise_exception():