From 0ee31073c5cb432a9cdd2648e99aa802b0ed0a17 Mon Sep 17 00:00:00 2001 From: Rongze Zhu Date: Wed, 10 Jul 2013 09:25:32 -0700 Subject: [PATCH] Enable zero the snapshot when delete snapshot in LVMVolumeDriver Because snapshot without 'size' field, So clear_volume method in LVMVolumeDriver will skip secure deleting. Get the size of snapshot from 'volume_size' filed, So it can zero the snapshot. Remove the 'size_in_g' parameter in _delete_volume method, because it never used. Add a unittest for clear_volume method. Fixes Bug #1198185 Change-Id: Ie919b50ce4fb276f29ab2e0279f868a691ea7bef --- cinder/tests/test_volume.py | 25 ++++++++++++++++++++++++- cinder/volume/drivers/lvm.py | 15 +++++++-------- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index f8d82c1f9..32274011b 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -1439,7 +1439,7 @@ class VolumeDriverTestCase(DriverTestCase): self.stubs.Set(self.volume.driver, '_volume_not_present', lambda x: False) self.stubs.Set(self.volume.driver, '_delete_volume', - lambda x, y: False) + lambda x: False) # Want DriverTestCase._fake_execute to return 'o' so that # volume.driver.delete_volume() raises the VolumeIsBusy exception. self.output = 'o' @@ -1490,6 +1490,29 @@ class LVMVolumeDriverTestCase(DriverTestCase): self.assertEquals(bs, '1M') self.assertEquals(count, 1024) + def test_clear_volume(self): + configuration = conf.Configuration(fake_opt, 'fake_group') + configuration.volume_clear = 'zero' + configuration.volume_clear_size = 0 + lvm_driver = lvm.LVMVolumeDriver(configuration=configuration) + self.stubs.Set(lvm_driver, '_copy_volume', lambda *a, **kw: True) + + fake_volume = {'name': 'test1', + 'volume_name': 'test1', + 'id': 'test1'} + + # Test volume has 'size' field + volume = dict(fake_volume, size='123') + self.assertEquals(True, lvm_driver.clear_volume(volume)) + + # Test volume has 'volume_size' field + volume = dict(fake_volume, volume_size='123') + self.assertEquals(True, lvm_driver.clear_volume(volume)) + + # Test volume without 'size' field and 'volume_size' field + volume = dict(fake_volume) + self.assertEquals(None, lvm_driver.clear_volume(volume)) + class ISCSITestCase(DriverTestCase): """Test Case for ISCSIDriver""" diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 4fba2db4e..2623a1fd7 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -162,7 +162,7 @@ class LVMVolumeDriver(driver.VolumeDriver): return True return False - def _delete_volume(self, volume, size_in_g): + def _delete_volume(self, volume): """Deletes a logical volume.""" # zero out old volumes to prevent data leaking between users # TODO(ja): reclaiming space should be done lazy and low priority @@ -218,19 +218,18 @@ class LVMVolumeDriver(driver.VolumeDriver): if (out[0] == 'o') or (out[0] == 'O'): raise exception.VolumeIsBusy(volume_name=volume['name']) - self._delete_volume(volume, volume['size']) + self._delete_volume(volume) def clear_volume(self, volume): """unprovision old volumes to prevent data leaking between users.""" vol_path = self.local_path(volume) - size_in_g = volume.get('size') - size_in_m = self.configuration.volume_clear_size - - if not size_in_g: + size_in_g = volume.get('size', volume.get('volume_size', None)) + if size_in_g is None: LOG.warning(_("Size for volume: %s not found, " - "skipping secure delete.") % volume['name']) + "skipping secure delete.") % volume['id']) return + size_in_m = self.configuration.volume_clear_size if self.configuration.volume_clear == 'none': return @@ -275,7 +274,7 @@ class LVMVolumeDriver(driver.VolumeDriver): # TODO(yamahata): zeroing out the whole snapshot triggers COW. # it's quite slow. - self._delete_volume(snapshot, snapshot['volume_size']) + self._delete_volume(snapshot) def local_path(self, volume): # NOTE(vish): stops deprecation warning -- 2.45.2