Skip to content

Commit

Permalink
Enable zero the snapshot when delete snapshot in LVMVolumeDriver
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zhurongze committed Jul 12, 2013
1 parent 2f5e26a commit 0ee3107
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 9 deletions.
25 changes: 24 additions & 1 deletion cinder/tests/test_volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ def test_delete_busy_volume(self):
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'
Expand Down Expand Up @@ -1490,6 +1490,29 @@ def test_convert_blocksize_option(self):
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"""
Expand Down
15 changes: 7 additions & 8 deletions cinder/volume/drivers/lvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def _volume_not_present(self, volume_name):
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
Expand Down Expand Up @@ -218,19 +218,18 @@ def delete_volume(self, volume):
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
Expand Down Expand Up @@ -275,7 +274,7 @@ def delete_snapshot(self, snapshot):

# 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
Expand Down

0 comments on commit 0ee3107

Please sign in to comment.