From 8a944bc8573eee775de8ea983191e0a4e808aaba Mon Sep 17 00:00:00 2001 From: Mitsuhiro Tanino Date: Thu, 9 Jul 2015 16:04:25 -0400 Subject: [PATCH] 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 --- cinder/tests/unit/test_volume.py | 106 ++++++++++++++++++++++++++++++- cinder/volume/drivers/lvm.py | 13 ++-- 2 files changed, 114 insertions(+), 5 deletions(-) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 315d006af..7a757473e 100644 --- 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 56512205c..a814a71f8 100644 --- 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) -- 2.45.2