From fb49b475cb69105cfd0123a28a13382a9ed9358e Mon Sep 17 00:00:00 2001 From: Duncan Thomas Date: Wed, 19 Feb 2014 19:03:24 +0000 Subject: [PATCH] Add optional ionice to volume clearing process Allow the volume clearing process to have an ionice priority set to reduce the performance impact of volume zeroing. Note that this may cause volume clearing processes to get backed up on a busy system, use with care. DocImpact Change-Id: I2d556f57aaca8a8ccc6f0f767f1cec28c3f9bc86 Implements: blueprint when-deleting-volume-dd-performance --- cinder/tests/test_volume_utils.py | 29 ++++++++++++++++++++++++++++- cinder/volume/driver.py | 5 +++++ cinder/volume/utils.py | 22 +++++++++++++++------- etc/cinder/cinder.conf.sample | 5 +++++ 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/cinder/tests/test_volume_utils.py b/cinder/tests/test_volume_utils.py index 4bb1464ea..9ebda3077 100644 --- a/cinder/tests/test_volume_utils.py +++ b/cinder/tests/test_volume_utils.py @@ -158,19 +158,46 @@ class ClearVolumeTestCase(test.TestCase): CONF.volume_clear = 'zero' CONF.volume_clear_size = 0 CONF.volume_dd_blocksize = '1M' + CONF.volume_clear_ionice = None self.mox.StubOutWithMock(volume_utils, 'copy_volume') volume_utils.copy_volume("/dev/zero", "volume_path", 1024, CONF.volume_dd_blocksize, sync=True, - execute=utils.execute) + ionice=None, execute=utils.execute) self.mox.ReplayAll() volume_utils.clear_volume(1024, "volume_path") def test_clear_volume_zero(self): CONF.volume_clear = 'zero' CONF.volume_clear_size = 1 + CONF.volume_clear_ionice = None + self.mox.StubOutWithMock(volume_utils, 'copy_volume') + volume_utils.copy_volume("/dev/zero", "volume_path", 1, + CONF.volume_dd_blocksize, sync=True, + ionice=None, execute=utils.execute) + self.mox.ReplayAll() + volume_utils.clear_volume(1024, "volume_path") + + def test_clear_volume_ionice(self): + CONF.volume_clear = 'zero' + CONF.volume_clear_size = 0 + CONF.volume_dd_blocksize = '1M' + CONF.volume_clear_ionice = '-c3' + self.mox.StubOutWithMock(volume_utils, 'copy_volume') + volume_utils.copy_volume("/dev/zero", "volume_path", 1024, + CONF.volume_dd_blocksize, sync=True, + ionice=CONF.volume_clear_ionice, + execute=utils.execute) + self.mox.ReplayAll() + volume_utils.clear_volume(1024, "volume_path") + + def test_clear_volume_zero_ionice(self): + CONF.volume_clear = 'zero' + CONF.volume_clear_size = 1 + CONF.volume_clear_ionice = '-c3' self.mox.StubOutWithMock(volume_utils, 'copy_volume') volume_utils.copy_volume("/dev/zero", "volume_path", 1, CONF.volume_dd_blocksize, sync=True, + ionice=CONF.volume_clear_ionice, execute=utils.execute) self.mox.ReplayAll() volume_utils.clear_volume(1024, "volume_path") diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 22dc586b6..59311d965 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -73,6 +73,11 @@ volume_opts = [ cfg.IntOpt('volume_clear_size', default=0, help='Size in MiB to wipe at start of old volumes. 0 => all'), + cfg.StrOpt('volume_clear_ionice', + default=None, + help='The flag to pass to ionice to alter the i/o priority ' + 'of the process used to zero a volume after deletion, ' + 'for example "-c3" for idle only priority.'), cfg.StrOpt('iscsi_helper', default='tgtadm', help='iscsi target user-land tool to use'), diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index 4827f8833..adcad8905 100644 --- a/cinder/volume/utils.py +++ b/cinder/volume/utils.py @@ -158,7 +158,7 @@ def _calculate_count(size_in_m, blocksize): def copy_volume(srcstr, deststr, size_in_m, blocksize, sync=False, - execute=utils.execute): + execute=utils.execute, ionice=None): # Use O_DIRECT to avoid thrashing the system buffer cache extra_flags = ['iflag=direct', 'oflag=direct'] @@ -177,15 +177,19 @@ def copy_volume(srcstr, deststr, size_in_m, blocksize, sync=False, blocksize, count = _calculate_count(size_in_m, blocksize) + cmd = ['dd', 'if=%s' % srcstr, 'of=%s' % deststr, + 'count=%d' % count, 'bs=%s' % blocksize] + cmd.extend(extra_flags) + + if ionice is not None: + cmd = ['ionice', ionice] + cmd + # Perform the copy - execute('dd', 'if=%s' % srcstr, 'of=%s' % deststr, - 'count=%d' % count, - 'bs=%s' % blocksize, - *extra_flags, run_as_root=True) + execute(*cmd, run_as_root=True) def clear_volume(volume_size, volume_path, volume_clear=None, - volume_clear_size=None): + volume_clear_size=None, volume_clear_ionice=None): """Unprovision old volumes to prevent data leaking between users.""" if volume_clear is None: volume_clear = CONF.volume_clear @@ -196,12 +200,16 @@ def clear_volume(volume_size, volume_path, volume_clear=None, if volume_clear_size == 0: volume_clear_size = volume_size + if volume_clear_ionice is None: + volume_clear_ionice = CONF.volume_clear_ionice + LOG.info(_("Performing secure delete on volume: %s") % volume_path) if volume_clear == 'zero': return copy_volume('/dev/zero', volume_path, volume_clear_size, CONF.volume_dd_blocksize, - sync=True, execute=utils.execute) + sync=True, execute=utils.execute, + ionice=volume_clear_ionice) elif volume_clear == 'shred': clear_cmd = ['shred', '-n3'] if volume_clear_size: diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index 2fb267998..73de85242 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -999,6 +999,11 @@ # (integer value) #volume_clear_size=0 +# The flag to pass to ionice to alter the i/o priority of the +# process used to zero a volume after deletion, for example +# "-c3" for idle only priority. (string value) +#volume_clear_ionice= + # iscsi target user-land tool to use (string value) #iscsi_helper=tgtadm -- 2.45.2