Don't use shred for volume clearing

Volume clearing is intended to provide safety against
data leaking between different volumes/tenants when
backing storage is reused.  dd is sufficient to
accomplish this, and since 'shred' does not add anything
meaningful and has not seen much real-world use,
we should remove it for simplicity.

This change makes 'volume_clear=shred' call dd and
warns that this option will be removed.

Change-Id: I190dc22c5edb6efc56b98c5eac870a35c03fcd3f
This commit is contained in:
Eric Harney 2016-07-08 12:38:50 -04:00
parent 572b84c073
commit c8a5e7d8ce
3 changed files with 18 additions and 23 deletions

View File

@ -437,24 +437,33 @@ class ClearVolumeTestCase(test.TestCase):
@mock.patch('cinder.utils.execute')
@mock.patch('cinder.volume.utils.CONF')
def test_clear_volume_shred(self, mock_conf, mock_exec):
# 'shred' now uses 'dd'. Remove this test when
# support for 'volume_clear=shred' is removed.
mock_conf.volume_clear = 'shred'
mock_conf.volume_clear_size = 1
mock_conf.volume_clear_ionice = None
mock_conf.volume_dd_blocksize = '1M'
output = volume_utils.clear_volume(1024, 'volume_path')
self.assertIsNone(output)
mock_exec.assert_called_once_with(
'shred', '-n3', '-s1MiB', "volume_path", run_as_root=True)
mock_exec.assert_called_with(
'dd', 'if=/dev/zero', 'of=volume_path', 'count=1', 'bs=1M',
'oflag=direct', run_as_root=True)
@mock.patch('cinder.utils.execute')
@mock.patch('cinder.volume.utils.CONF')
def test_clear_volume_shred_not_clear_size(self, mock_conf, mock_exec):
# 'shred' now uses 'dd'. Remove this test when
# support for 'volume_clear=shred' is removed.
mock_conf.volume_clear = 'shred'
mock_conf.volume_clear_size = None
mock_conf.volume_clear_ionice = None
mock_conf.volume_dd_blocksize = '1M'
mock_conf.volume_clear_size = 1
output = volume_utils.clear_volume(1024, 'volume_path')
self.assertIsNone(output)
mock_exec.assert_called_once_with(
'shred', '-n3', "volume_path", run_as_root=True)
mock_exec.assert_called_with(
'dd', 'if=/dev/zero', 'of=volume_path', 'count=1', 'bs=1M',
'oflag=direct', run_as_root=True)
@mock.patch('cinder.volume.utils.CONF')
def test_clear_volume_invalid_opt(self, mock_conf):

View File

@ -488,6 +488,11 @@ def clear_volume(volume_size, volume_path, volume_clear=None,
LOG.info(_LI("Performing secure delete on volume: %s"), volume_path)
if volume_clear == 'shred':
LOG.warning(_LW("volume_clear=shred has been deprecated and will "
"be removed in the next release. Clearing with dd."))
volume_clear = 'zero'
# We pass sparse=False explicitly here so that zero blocks are not
# skipped in order to clear the volume.
if volume_clear == 'zero':
@ -496,26 +501,11 @@ def clear_volume(volume_size, volume_path, volume_clear=None,
sync=True, execute=utils.execute,
ionice=volume_clear_ionice,
throttle=throttle, sparse=False)
elif volume_clear == 'shred':
clear_cmd = ['shred', '-n3']
if volume_clear_size:
clear_cmd.append('-s%dMiB' % volume_clear_size)
else:
raise exception.InvalidConfigurationValue(
option='volume_clear',
value=volume_clear)
clear_cmd.append(volume_path)
start_time = timeutils.utcnow()
utils.execute(*clear_cmd, run_as_root=True)
duration = timeutils.delta_seconds(start_time, timeutils.utcnow())
# NOTE(jdg): use a default of 1, mostly for unit test, but in
# some incredible event this is 0 (cirros image?) don't barf
if duration < 1:
duration = 1
LOG.info(_LI('Elapsed time for clear volume: %.2f sec'), duration)
def supports_thin_provisioning():
return brick_lvm.LVM.supports_thin_provisioning(

View File

@ -65,10 +65,6 @@ lvconvert: CommandFilter, lvconvert, root
# cinder/volume/driver.py: 'iscsiadm', '-m', 'node', '-T', ...
iscsiadm: CommandFilter, iscsiadm, root
# cinder/volume/drivers/lvm.py: 'shred', '-n3'
# cinder/volume/drivers/lvm.py: 'shred', '-n0', '-z', '-s%dMiB'
shred: CommandFilter, shred, root
# cinder/volume/utils.py: utils.temporary_chown(path, 0)
chown: CommandFilter, chown, root