From 0982ad131f74095e80b78d9b2f79271c3fa062f9 Mon Sep 17 00:00:00 2001 From: Mitsuhiro Tanino Date: Sun, 26 Jul 2015 23:15:58 -0400 Subject: [PATCH] Efficient volume copy for generic volume migration Currently Cinder uses dd command for data copy of volume migration, but the copy always copy full blocks even if the source data contains many null and zero blocks. The dd command has an option conv=sparse to skip null or zero blocks for more efficient data copy. However, if the destination volume is not zero cleared beforehand, we should copy full block from source to dest volume to cleanup dest volume in order to avoid security issue. If the volume pre-initilization(zero cleared) is ensured beforehand, we can skip copy of null and zero blocks to destination volume by using sparse copy. In order to use this option properly, we have to check sparse_copy_volume capability for destination backend driver via RPC API before volume copy. This patch also adds sparse_copy_volume capability flag into volume stats of LVM and NFS drivers to enable efficient copy for these backends. Implements: blueprint efficient-volume-copy-for-cinder-assisted-migration Change-Id: Ic343860d37276907724fce3a9c0f7c9d034c4aaa --- cinder/tests/unit/test_nfs.py | 1 + cinder/tests/unit/test_volume.py | 79 ++++++++++++++++++++++++++++++++ cinder/volume/driver.py | 14 ++++-- cinder/volume/drivers/lvm.py | 3 ++ cinder/volume/drivers/nfs.py | 8 +++- 5 files changed, 98 insertions(+), 7 deletions(-) diff --git a/cinder/tests/unit/test_nfs.py b/cinder/tests/unit/test_nfs.py index 528ac2f0b..53b932711 100644 --- a/cinder/tests/unit/test_nfs.py +++ b/cinder/tests/unit/test_nfs.py @@ -850,6 +850,7 @@ class NfsDriverTestCase(test.TestCase): self.assertEqual(30.0, drv._stats['total_capacity_gb']) self.assertEqual(5.0, drv._stats['free_capacity_gb']) self.assertEqual(0, drv._stats['reserved_percentage']) + self.assertTrue(drv._stats['sparse_copy_volume']) mox.VerifyAll() diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index d65687787..7d847d5c3 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -6132,6 +6132,74 @@ class GenericVolumeDriverTestCase(DriverTestCase): volume_api.disable_replication(ctxt, volume) self.assertTrue(mock_disable_rep.called) + @mock.patch.object(utils, 'brick_get_connector_properties') + @mock.patch.object(cinder.volume.driver.VolumeDriver, '_attach_volume') + @mock.patch.object(cinder.volume.driver.VolumeDriver, '_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): + + 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'}}, dest_vol), + ({'device': {'path': 'foo'}}, src_vol), + ] + + # Test case for sparse_copy_volume = False + mock_attach.side_effect = attach_volume_returns + mock_get_capabilities.return_value = {} + self.volume.driver.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', + throttle=self.volume.driver._throttle, + 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.driver.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', + throttle=self.volume.driver._throttle, + 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']) + class LVMISCSIVolumeDriverTestCase(DriverTestCase): """Test case for VolumeDriver""" @@ -6994,6 +7062,17 @@ class ISCSITestCase(DriverTestCase): float('5.0'), stats['pools'][0]['provisioned_capacity_gb']) self.assertEqual( int('1'), stats['pools'][0]['total_volumes']) + self.assertFalse(stats['sparse_copy_volume']) + + # Check value of sparse_copy_volume for thin enabled case. + self.configuration = conf.Configuration(None) + self.configuration.lvm_type = 'thin' + lvm_driver = lvm.LVMVolumeDriver(configuration=self.configuration, + db=db) + lvm_driver.vg = brick_lvm.LVM('cinder-volumes', 'sudo') + lvm_driver._update_volume_stats() + stats = lvm_driver._stats + self.assertTrue(stats['sparse_copy_volume']) def test_validate_connector(self): iscsi_driver =\ diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 1e933bafc..36111157e 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -341,10 +341,6 @@ class BaseVD(object): # set True by manager after successful check_for_setup self._initialized = False - # Copy volume data in a sparse fashion. - # (overload in drivers where this is desired) - self._sparse_copy_volume_data = False - def _is_non_recoverable(self, err, non_recoverable_list): for item in non_recoverable_list: if item in err: @@ -784,6 +780,14 @@ class BaseVD(object): self._detach_volume(context, dest_attach_info, dest_vol, properties, force=True, remote=dest_remote) + # Check the backend capabilities of migration destination host. + rpcapi = volume_rpcapi.VolumeAPI() + capabilities = rpcapi.get_capabilities(context, 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']) * 1024 # vol size is in GB @@ -793,7 +797,7 @@ class BaseVD(object): size_in_mb, self.configuration.volume_dd_blocksize, throttle=self._throttle, - sparse=self._sparse_copy_volume_data) + sparse=sparse_copy_volume) copy_error = False except Exception: with excutils.save_and_reraise_exception(): diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 6786439c0..6a54eb3c6 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -246,6 +246,9 @@ class LVMVolumeDriver(driver.VolumeDriver): )) data["pools"].append(single_pool) + # Check availability of sparse volume copy. + data['sparse_copy_volume'] = self.configuration.lvm_type == 'thin' + self._stats = data def check_for_setup_error(self): diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 78c178434..3f59f5989 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -110,8 +110,6 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver): nfs_mount_point_base=self.base, nfs_mount_options=opts) - self._sparse_copy_volume_data = True - def set_execute(self, execute): super(NfsDriver, self).set_execute(execute) if self._remotefsclient: @@ -368,3 +366,9 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver): "environment. Please see %s " "for information on a secure NAS configuration."), doc_html) + + def _update_volume_stats(self): + """Retrieve stats info from volume group.""" + + super(NfsDriver, self)._update_volume_stats() + self._stats['sparse_copy_volume'] = True -- 2.45.2