diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py index cd49bf6b939..62500acf3fb 100644 --- a/cinder/tests/unit/test_volume_utils.py +++ b/cinder/tests/unit/test_volume_utils.py @@ -382,7 +382,7 @@ class ClearVolumeTestCase(test.TestCase): mock_copy.assert_called_once_with('/dev/zero', 'volume_path', 1024, '1M', sync=True, execute=utils.execute, ionice='-c3', - throttle=None) + throttle=None, sparse=False) @mock.patch('cinder.volume.utils.copy_volume', return_value=None) @mock.patch('cinder.volume.utils.CONF') @@ -397,7 +397,7 @@ class ClearVolumeTestCase(test.TestCase): mock_copy.assert_called_once_with('/dev/zero', 'volume_path', 1, '1M', sync=True, execute=utils.execute, ionice='-c0', - throttle=None) + throttle=None, sparse=False) @mock.patch('cinder.utils.execute') @mock.patch('cinder.volume.utils.CONF') @@ -523,6 +523,39 @@ class CopyVolumeTestCase(test.TestCase): 'count=5678', 'bs=1234', 'conv=fdatasync', run_as_root=True) + @mock.patch('cinder.volume.utils._calculate_count', + return_value=(1234, 5678)) + @mock.patch('cinder.volume.utils.check_for_odirect_support', + return_value=False) + @mock.patch('cinder.utils.execute') + def test_copy_volume_dd_with_sparse(self, mock_exec, + mock_support, mock_count): + output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1, + sync=True, execute=utils.execute, + sparse=True) + self.assertIsNone(output) + mock_exec.assert_called_once_with('dd', 'if=/dev/zero', 'of=/dev/null', + 'count=5678', 'bs=1234', + 'conv=fdatasync,sparse', + run_as_root=True) + + @mock.patch('cinder.volume.utils._calculate_count', + return_value=(1234, 5678)) + @mock.patch('cinder.volume.utils.check_for_odirect_support', + return_value=True) + @mock.patch('cinder.utils.execute') + def test_copy_volume_dd_with_sparse_iflag_and_oflag(self, mock_exec, + mock_support, + mock_count): + output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1, + sync=True, execute=utils.execute, + sparse=True) + self.assertIsNone(output) + mock_exec.assert_called_once_with('dd', 'if=/dev/zero', 'of=/dev/null', + 'count=5678', 'bs=1234', + 'iflag=direct', 'oflag=direct', + 'conv=sparse', run_as_root=True) + class VolumeUtilsTestCase(test.TestCase): def test_null_safe_str(self): diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 04fd9698b71..317d983df30 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -413,12 +413,14 @@ 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) + execute=self._execute, + sparse=sparse) finally: self.delete_snapshot(temp_snapshot) diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index bccb7714ed8..63b857d9f04 100644 --- a/cinder/volume/utils.py +++ b/cinder/volume/utils.py @@ -278,7 +278,7 @@ def check_for_odirect_support(src, dest, flag='oflag=direct'): def _copy_volume(prefix, srcstr, deststr, size_in_m, blocksize, sync=False, - execute=utils.execute, ionice=None): + execute=utils.execute, ionice=None, sparse=False): # Use O_DIRECT to avoid thrashing the system buffer cache extra_flags = [] if check_for_odirect_support(srcstr, deststr, 'iflag=direct'): @@ -290,8 +290,14 @@ def _copy_volume(prefix, srcstr, deststr, size_in_m, blocksize, sync=False, # If the volume is being unprovisioned then # request the data is persisted before returning, # so that it's not discarded from the cache. + conv = [] if sync and not extra_flags: - extra_flags.append('conv=fdatasync') + conv.append('fdatasync') + if sparse: + conv.append('sparse') + if conv: + conv_options = 'conv=' + ",".join(conv) + extra_flags.append(conv_options) blocksize, count = _calculate_count(size_in_m, blocksize) @@ -325,13 +331,14 @@ def _copy_volume(prefix, srcstr, deststr, size_in_m, blocksize, sync=False, def copy_volume(srcstr, deststr, size_in_m, blocksize, sync=False, - execute=utils.execute, ionice=None, throttle=None): + execute=utils.execute, ionice=None, throttle=None, + sparse=False): if not throttle: throttle = throttling.Throttle.get_default() with throttle.subcommand(srcstr, deststr) as throttle_cmd: _copy_volume(throttle_cmd['prefix'], srcstr, deststr, size_in_m, blocksize, sync=sync, - execute=execute, ionice=ionice) + execute=execute, ionice=ionice, sparse=sparse) def clear_volume(volume_size, volume_path, volume_clear=None, @@ -352,12 +359,14 @@ def clear_volume(volume_size, volume_path, volume_clear=None, LOG.info(_LI("Performing secure delete on volume: %s"), volume_path) + # We pass sparse=False explicitly here so that zero blocks are not + # skipped in order to clear the volume. if volume_clear == 'zero': return copy_volume('/dev/zero', volume_path, volume_clear_size, CONF.volume_dd_blocksize, sync=True, execute=utils.execute, ionice=volume_clear_ionice, - throttle=throttle) + throttle=throttle, sparse=False) elif volume_clear == 'shred': clear_cmd = ['shred', '-n3'] if volume_clear_size: