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

diff --git a/cinder/tests/unit/test_nfs.py b/cinder/tests/unit/test_nfs.py
index 528ac2f0b22b3d30179afd85157c609f278a70bb..53b93271157841e3507342f1040374ab422f9856 100644 (file)
--- 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 d65687787b788b7677020d011dccf4683ae2c5a8..7d847d5c3435987fdf4a64416c3b6d63baeb11f8 100644 (file)
--- 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 1e933bafc336cc678c494c0dcc6423b6737bf8b6..36111157e668e527dd1cff7d7f2ad42d99d73af8 100644 (file)
--- 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 6786439c0656da402171deec8f0337d2335002e5..6a54eb3c6865a4b6f4527d73db1b4b0abe7c4ef2 100644 (file)
--- 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 78c178434984c3a389b81b4872a1bcd406e96982..3f59f598948540041a9128ba58611e9d7b6af265 100644 (file)
--- 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