The create_cloned_volume uses dd command for data copy, but the
copy always copy 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 as an argument
of dd command for create_cloned_volume when we use thin provisioned
LVM.
[NOTE]
This patch only enables conv=sparse parameter of dd command for
create_cloned_volume() path of LVM driver. There are some places
using dd in Cinder, but we should carefully consider to apply
this parameter for other places because misuse of this parameter
causes security issues or data corruptions. Also we DO NOT use this
parameter for volume wiping case because the volume is not cleared
at all.
Here are some results for this option.
- Without conv=sparse option
LV VG Attr LSize Pool Origin Data%
vg1-pool vg1 twi-a-tz-- 3.80g 31.45
volume-clone vg1 Vwi-a-tz-- 1.00g vg1-pool 100.00
volume-source vg1 Vwi-a-tz-- 1.00g vg1-pool 19.53
- With conv=sparse option
LV VG Attr LSize Pool Origin Data%
vg1-pool vg1 twi-a-tz-- 3.80g 10.28
volume-clone vg1 Vwi-a-tz-- 1.00g vg1-pool 19.53
volume-source vg1 Vwi-a-tz-- 1.00g vg1-pool 19.53
Change-Id: I743f823ca38529b12301a89308d1d406aa3fa45f
Closes-bug: #
1224671
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')
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')
'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):
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)
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'):
# 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)
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,
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:
Code Review / openstack-build / cinder-build.git / commitdiff