Don't zero out snapshot volume on snapshot_delete

 When trying to zero out an LVM snapshot on precise the kernel sometimes
 hangs when performing the dd.  Also the dd process itself can take
 an extremely long time even when it does succesfully complete.
 This can be up to 30 minutes for a 1 Gig volume/snapshot.

 I believe this is a kernel specific issue with LVM snapshots.  The
 zeroing process is unreliable and can cause kernel hangs to let's remove it.

 In order to protect against data leakage we'll implement the zeroing process
 on volume creation.  This doesn't seem to have a significant impact and doesn't
 suffer from the same isues tha zeroing out an LVM snapshot does.

 No reason to continue zero on delete, the zero on creation should
 probably be sufficient.

 Doesn't seem to cause any timing issues but need to keep this in mind.

 Fixes bug 1023755

Change-Id: I56209d8e5973cffa997b4ec3e51c3361838386de

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index ef07156367b482b77c22338272de170c5759068a..4d752b108e4dd226582d54799901db7814eed0a5 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -767,17 +767,17 @@ 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'
         self.assertRaises(exception.VolumeIsBusy,
                           self.volume.driver.delete_volume,
-                          {'name': 'test1', 'size': 1024})
+                          {'name': 'test1'})
         # when DriverTestCase._fake_execute returns something other than
         # 'o' volume.driver.delete_volume() does not raise an exception.
         self.output = 'x'
-        self.volume.driver.delete_volume({'name': 'test1', 'size': 1024})
+        self.volume.driver.delete_volume({'name': 'test1'})
 
 
 class ISCSITestCase(DriverTestCase):

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index 435e023e431cd8a8206923851673cc60c730e008..cdae2a18d859fecf71479b82b49ef595bc5eeceb 100644 (file)
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -143,11 +143,8 @@ class VolumeDriver(object):
             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
-        self._copy_volume('/dev/zero', self.local_path(volume), size_in_g)
         self._try_execute('lvremove', '-f', "%s/%s" %
                           (FLAGS.volume_group,
                            self._escape_snapshot(volume['name'])),
@@ -170,6 +167,15 @@ class VolumeDriver(object):
         changes to the volume object to be persisted."""
         self._create_volume(volume['name'], self._sizestr(volume['size']))
 
+        # NOTE(jdg): As per BUG 1023755 zeroing out snapshot volume
+        # is extremely slow and on 12.04 has a tendency to hang the kernel
+        # We're going to work around this by dropping the zero out process
+        # on snapshot delete, but we need to protect form data leakage still
+        # so we'll zero out newly created volumes to hack around this.
+        self._copy_volume('/dev/zero',
+                          self.local_path(volume),
+                          volume['size'])
+
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from a snapshot."""
         self._create_volume(volume['name'], self._sizestr(volume['size']))
@@ -195,7 +201,7 @@ class VolumeDriver(object):
             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 create_snapshot(self, snapshot):
         """Creates a snapshot."""
@@ -211,9 +217,7 @@ class VolumeDriver(object):
             # If the snapshot isn't present, then don't attempt to delete
             return True
 
-        # 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
@@ -350,7 +354,6 @@ class ISCSIDriver(VolumeDriver):
 
     def create_export(self, context, volume):
         """Creates an export for a logical volume."""
-        #BOOKMARK(jdg)
 
         iscsi_name = "%s%s" % (FLAGS.iscsi_target_prefix, volume['name'])
         volume_path = "/dev/%s/%s" % (FLAGS.volume_group, volume['name'])
@@ -380,6 +383,7 @@ class ISCSIDriver(VolumeDriver):
 
     def remove_export(self, context, volume):
         """Removes an export for a logical volume."""
+
         # NOTE(jdg): tgtadm doesn't use the iscsi_targets table
         # TODO(jdg): In the future move all of the dependent stuff into the
         # cooresponding target admin class