LVM: Support efficient data copy for LVM driver

The create_volume_from_snapshot() and migrate_volume() use
dd command for data copy, but the copy always copies full blocks
even if the source data contains many null or zero blocks.
When we use thin provisioned LVM, blocks are not pre-allocated,
so unused region returns zero.
If we copy full block for destination volume, unnecessary blocks
will be allocated and the usage will be 100%.

The dd command has conv=sparse option in order to copy data more
efficiently. This patch enables conv=sparse option for
create_volume_from_snapshot() and migrate_volume() when we use
thin provisioned LVM.

Change-Id: I104c4127bc4a33da9c11ad2aac93766f862e2981
Closes-bug: #1472486

diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py
index 315d006af05d7fb0b4370221013602e52c7dbe01..7a757473e7f374eb1ca89525a2ba8d80914e9ea6 100644 (file)
--- a/cinder/tests/unit/test_volume.py
+++ b/cinder/tests/unit/test_volume.py
@@ -5790,7 +5790,56 @@ class LVMISCSIVolumeDriverTestCase(DriverTestCase):
                 '/dev/mapper/cinder--volumes--2-testvol',
                 2048,
                 '1M',
-                execute=mock_execute)
+                execute=mock_execute,
+                sparse=False)
+
+    def test_lvm_migrate_volume_proceed_with_thin(self):
+        hostname = socket.gethostname()
+        capabilities = {'location_info': 'LVMVolumeDriver:%s:'
+                        'cinder-volumes-2:default:0' % hostname}
+        host = {'capabilities': capabilities}
+        vol = {'name': 'testvol', 'id': 1, 'size': 2, 'status': 'available'}
+
+        def fake_execute(*args, **kwargs):
+            pass
+
+        def get_all_volume_groups():
+            # NOTE(flaper87) Return just the destination
+            # host to test the check of dest VG existence.
+            return [{'name': 'cinder-volumes-2'}]
+
+        def _fake_get_all_physical_volumes(obj, root_helper, vg_name):
+            return [{}]
+
+        self.configuration.lvm_type = 'thin'
+        lvm_driver = lvm.LVMVolumeDriver(configuration=self.configuration,
+                                         db=db)
+
+        with mock.patch.object(brick_lvm.LVM, 'get_all_physical_volumes',
+                               return_value = [{}]), \
+                mock.patch.object(lvm_driver, '_execute') \
+                as mock_execute, \
+                mock.patch.object(volutils, 'copy_volume') as mock_copy, \
+                mock.patch.object(volutils, 'get_all_volume_groups',
+                                  side_effect = get_all_volume_groups), \
+                mock.patch.object(lvm_driver, '_delete_volume'):
+
+            lvm_driver.vg = fake_lvm.FakeBrickLVM('cinder-volumes',
+                                                  False,
+                                                  None,
+                                                  'default')
+            lvm_driver.sparse_copy_volume = True
+            moved, model_update = \
+                lvm_driver.migrate_volume(self.context, vol, host)
+            self.assertTrue(moved)
+            self.assertIsNone(model_update)
+            mock_copy.assert_called_once_with(
+                '/dev/mapper/cinder--volumes-testvol',
+                '/dev/mapper/cinder--volumes--2-testvol',
+                2048,
+                '1M',
+                execute=mock_execute,
+                sparse=True)
 
     @staticmethod
     def _get_manage_existing_lvs(name):
@@ -6085,6 +6134,61 @@ class LVMVolumeDriverTestCase(DriverTestCase):
 
         mock_volume_get.assert_called_with(self.context, vol['id'])
 
+    def test_create_volume_from_snapshot_none_sparse(self):
+
+        with mock.patch.object(self.volume.driver, 'vg'), \
+                mock.patch.object(self.volume.driver, '_create_volume'), \
+                mock.patch.object(volutils, 'copy_volume') as mock_copy:
+
+            # Test case for thick LVM
+            src_volume = tests_utils.create_volume(self.context)
+            snapshot_ref = tests_utils.create_snapshot(self.context,
+                                                       src_volume['id'])
+            dst_volume = tests_utils.create_volume(self.context)
+            self.volume.driver.create_volume_from_snapshot(dst_volume,
+                                                           snapshot_ref)
+
+            volume_path = self.volume.driver.local_path(dst_volume)
+            snapshot_path = self.volume.driver.local_path(snapshot_ref)
+            volume_size = 1024
+            block_size = '1M'
+            mock_copy.assert_called_with(snapshot_path,
+                                         volume_path,
+                                         volume_size,
+                                         block_size,
+                                         execute=self.volume.driver._execute,
+                                         sparse=False)
+
+    def test_create_volume_from_snapshot_sparse(self):
+
+        self.configuration.lvm_type = 'thin'
+        lvm_driver = lvm.LVMVolumeDriver(configuration=self.configuration,
+                                         db=db)
+
+        with mock.patch.object(lvm_driver, 'vg'), \
+                mock.patch.object(lvm_driver, '_create_volume'), \
+                mock.patch.object(volutils, 'copy_volume') as mock_copy:
+
+            # Test case for thin LVM
+            lvm_driver.sparse_copy_volume = True
+            src_volume = tests_utils.create_volume(self.context)
+            snapshot_ref = tests_utils.create_snapshot(self.context,
+                                                       src_volume['id'])
+            dst_volume = tests_utils.create_volume(self.context)
+            lvm_driver.create_volume_from_snapshot(dst_volume,
+                                                   snapshot_ref)
+
+            volume_path = lvm_driver.local_path(dst_volume)
+            snapshot_path = lvm_driver.local_path(snapshot_ref)
+            volume_size = 1024
+            block_size = '1M'
+            mock_copy.assert_called_with(snapshot_path,
+                                         volume_path,
+                                         volume_size,
+                                         block_size,
+                                         execute=lvm_driver._execute,
+                                         sparse=True)
+
 
 class ISCSITestCase(DriverTestCase):
     """Test Case for ISCSIDriver"""

diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index 56512205c324a563bfacb7c37b7d21abc0e393c5..a814a71f88ae0b1c596cf4680d6b644cde72fc0b 100644 (file)
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -97,6 +97,7 @@ class LVMVolumeDriver(driver.VolumeDriver):
             db=self.db,
             executor=self._execute)
         self.protocol = self.target_driver.protocol
+        self.sparse_copy_volume = False
 
     def _sizestr(self, size_in_g):
         return '%sg' % size_in_g
@@ -295,6 +296,9 @@ class LVMVolumeDriver(driver.VolumeDriver):
                     raise exception.VolumeBackendAPIException(
                         data=exception_message)
 
+            # Enable sparse copy since lvm_type is 'thin'
+            self.sparse_copy_volume = True
+
     def create_volume(self, volume):
         """Creates a logical volume."""
         mirror_count = 0
@@ -358,7 +362,8 @@ class LVMVolumeDriver(driver.VolumeDriver):
                              self.local_path(volume),
                              snapshot['volume_size'] * units.Ki,
                              self.configuration.volume_dd_blocksize,
-                             execute=self._execute)
+                             execute=self._execute,
+                             sparse=self.sparse_copy_volume)
 
     def delete_volume(self, volume):
         """Deletes a logical volume."""
@@ -449,14 +454,13 @@ class LVMVolumeDriver(driver.VolumeDriver):
                                 mirror_count)
 
             self.vg.activate_lv(temp_snapshot['name'], is_snapshot=True)
-            sparse = True if self.configuration.lvm_type == 'thin' else False
             volutils.copy_volume(
                 self.local_path(temp_snapshot),
                 self.local_path(volume),
                 src_vref['size'] * units.Ki,
                 self.configuration.volume_dd_blocksize,
                 execute=self._execute,
-                sparse=sparse)
+                sparse=self.sparse_copy_volume)
         finally:
             self.delete_snapshot(temp_snapshot)
 
@@ -621,7 +625,8 @@ class LVMVolumeDriver(driver.VolumeDriver):
                                  self.local_path(volume, vg=dest_vg),
                                  size_in_mb,
                                  self.configuration.volume_dd_blocksize,
-                                 execute=self._execute)
+                                 execute=self._execute,
+                                 sparse=self.sparse_copy_volume)
             self._delete_volume(volume)
 
             return (True, None)