]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Enable zero the snapshot when delete snapshot in LVMVolumeDriver
authorRongze Zhu <rongze@unitedstack.com>
Wed, 10 Jul 2013 16:25:32 +0000 (09:25 -0700)
committerRongze Zhu <rongze@unitedstack.com>
Fri, 12 Jul 2013 14:50:15 +0000 (14:50 +0000)
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
cinder/volume/drivers/lvm.py

index f8d82c1f96595765560d10d286703e39399226ec..32274011b2d500e9c2c163341833257feab37c21 100644 (file)
@@ -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"""
index 4fba2db4ebaf76325bdacf1b1f320199cf50081e..2623a1fd746641df5a003d65608ad3405d448a60 100644 (file)
@@ -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