From: Eric Harney <eharney@redhat.com>
Date: Fri, 13 Sep 2013 20:56:44 +0000 (-0400)
Subject: LVM volume_clear: error on unexpected inputs
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=863ae9f484089aa5d9e576255b39288da82c1100;p=openstack-build%2Fcinder-build.git

LVM volume_clear: error on unexpected inputs

Currently if a user configures
volume_clear='non_existent_volume_clearer' in cinder.conf,
the LVM driver will silently delete a volume and not wipe it.

Instead, the delete operation should fail, leaving the volume
in the 'error_deleting' state.

Also fail if the volume reference does not contain either a
'size' or 'volume_size' field.

Closes-Bug: #1225194
Change-Id: I78fec32d7d5aeaa8e2deeac43066ca5e2e26d9ca
---

diff --git a/cinder/exception.py b/cinder/exception.py
index 6e5cf7be7..ba503ca3b 100644
--- a/cinder/exception.py
+++ b/cinder/exception.py
@@ -186,6 +186,11 @@ class InvalidAuthKey(Invalid):
     message = _("Invalid auth key: %(reason)s")
 
 
+class InvalidConfigurationValue(Invalid):
+    message = _('Value "%(value)s" is not valid for '
+                'configuration option "%(option)s"')
+
+
 class ServiceUnavailable(Invalid):
     message = _("Service is unavailable at this time.")
 
diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 8ce5b7357..22432a30b 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -2163,25 +2163,61 @@ class LVMVolumeDriverTestCase(DriverTestCase):
         configuration.volume_clear = 'zero'
         configuration.volume_clear_size = 0
         lvm_driver = lvm.LVMVolumeDriver(configuration=configuration)
-        self.stubs.Set(volutils, 'copy_volume',
-                       lambda x, y, z, sync=False, execute='foo': True)
-        self.stubs.Set(os.path, 'exists', lambda x: True)
+        self.mox.StubOutWithMock(volutils, 'copy_volume')
+        self.mox.StubOutWithMock(os.path, 'exists')
+        self.mox.StubOutWithMock(utils, 'execute')
 
         fake_volume = {'name': 'test1',
                        'volume_name': 'test1',
                        'id': 'test1'}
 
+        os.path.exists(mox.IgnoreArg()).AndReturn(True)
+        volutils.copy_volume('/dev/zero', mox.IgnoreArg(), 123 * 1024,
+                             execute=lvm_driver._execute, sync=True)
+
+        os.path.exists(mox.IgnoreArg()).AndReturn(True)
+        volutils.copy_volume('/dev/zero', mox.IgnoreArg(), 123 * 1024,
+                             execute=lvm_driver._execute, sync=True)
+
+        os.path.exists(mox.IgnoreArg()).AndReturn(True)
+
+        self.mox.ReplayAll()
+
         # Test volume has 'size' field
-        volume = dict(fake_volume, size='123')
-        self.assertEqual(True, lvm_driver.clear_volume(volume))
+        volume = dict(fake_volume, size=123)
+        lvm_driver.clear_volume(volume)
 
         # Test volume has 'volume_size' field
-        volume = dict(fake_volume, volume_size='123')
-        self.assertEqual(True, lvm_driver.clear_volume(volume))
+        volume = dict(fake_volume, volume_size=123)
+        lvm_driver.clear_volume(volume)
 
         # Test volume without 'size' field and 'volume_size' field
         volume = dict(fake_volume)
-        self.assertEqual(None, lvm_driver.clear_volume(volume))
+        self.assertRaises(exception.InvalidParameterValue,
+                          lvm_driver.clear_volume,
+                          volume)
+
+    def test_clear_volume_badopt(self):
+        configuration = conf.Configuration(fake_opt, 'fake_group')
+        configuration.volume_clear = 'non_existent_volume_clearer'
+        configuration.volume_clear_size = 0
+        lvm_driver = lvm.LVMVolumeDriver(configuration=configuration)
+        self.mox.StubOutWithMock(volutils, 'copy_volume')
+        self.mox.StubOutWithMock(os.path, 'exists')
+
+        fake_volume = {'name': 'test1',
+                       'volume_name': 'test1',
+                       'id': 'test1',
+                       'size': 123}
+
+        os.path.exists(mox.IgnoreArg()).AndReturn(True)
+
+        self.mox.ReplayAll()
+
+        volume = dict(fake_volume)
+        self.assertRaises(exception.InvalidConfigurationValue,
+                          lvm_driver.clear_volume,
+                          volume)
 
 
 class ISCSITestCase(DriverTestCase):
diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index 5f0510f16..10112fff6 100644
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -218,9 +218,10 @@ class LVMVolumeDriver(driver.VolumeDriver):
 
         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['id'])
-            return
+            msg = (_("Size for volume: %s not found, "
+                     "cannot secure delete.") % volume['id'])
+            LOG.error(msg)
+            raise exception.InvalidParameterValue(msg)
         size_in_m = self.configuration.volume_clear_size
 
         LOG.info(_("Performing secure delete on volume: %s") % volume['id'])
@@ -238,9 +239,9 @@ class LVMVolumeDriver(driver.VolumeDriver):
             if size_in_m:
                 clear_cmd.append('-s%dMiB' % size_in_m)
         else:
-            LOG.error(_("Error unrecognized volume_clear option: %s"),
-                      self.configuration.volume_clear)
-            return
+            raise exception.InvalidConfigurationValue(
+                option='volume_clear',
+                value=self.configuration.volume_clear)
 
         clear_cmd.append(dev_path)
         self._execute(*clear_cmd, run_as_root=True)