Merge "libvirt: remove_logical_volumes should remove each separately"

nova/exception.py

@@ -1526,6 +1526,10 @@ class KeyManagerError(NovaException):
     msg_fmt = _("Key manager error: %(reason)s")
 
 
+class VolumesNotRemoved(Invalid):
+    msg_fmt = _("Failed to remove volume(s): (%(reason)s)")
+
+
 class InvalidVideoMode(Invalid):
     msg_fmt = _("Provided video model (%(model)s) is not supported.")
 

nova/tests/virt/libvirt/test_libvirt_utils.py

@@ -13,12 +13,14 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import contextlib
 import functools
 import os
 
 import mock
 from oslo.config import cfg
 
+from nova import exception
 from nova.openstack.common import processutils
 from nova import test
 from nova import utils
@@ -230,6 +232,21 @@ blah BLAH: bb
         ])
         self.assertEqual(2, mock_execute.call_count)
 
+    def test_fail_remove_all_logical_volumes(self):
+
+        def fake_execute(*args, **kwargs):
+            if 'vol2' in args:
+                raise processutils.ProcessExecutionError('Error')
+
+        with contextlib.nested(
+             mock.patch.object(libvirt_utils, 'clear_logical_volume'),
+             mock.patch.object(libvirt_utils, 'execute',
+                  side_effect=fake_execute)) as (mock_clear, mock_execute):
+            self.assertRaises(exception.VolumesNotRemoved,
+                              libvirt_utils.remove_logical_volumes,
+                              ['vol1', 'vol2', 'vol3'])
+            self.assertEqual(3, mock_execute.call_count)
+
     @mock.patch('nova.utils.execute')
     def test_copy_image_scp(self, mock_execute):
         mock_execute.side_effect = [

nova/virt/libvirt/driver.py

@@ -1087,7 +1087,7 @@ class LibvirtDriver(driver.ComputeDriver):
         """Delete all LVM disks for given instance object."""
         disks = self._lvm_disks(instance)
         if disks:
-            libvirt_utils.remove_logical_volumes(*disks)
+            libvirt_utils.remove_logical_volumes(disks)
 
     def _lvm_disks(self, instance):
         """Returns all LVM disks for given instance object."""

nova/virt/libvirt/utils.py

@@ -416,15 +416,19 @@ def clear_logical_volume(path):
                                 % volume_clear)
 
 
-def remove_logical_volumes(*paths):
+def remove_logical_volumes(paths):
     """Remove one or more logical volume."""
 
+    errors = []
     for path in paths:
         clear_logical_volume(path)
-
-    if paths:
-        lvremove = ('lvremove', '-f') + paths
-        execute(*lvremove, attempts=3, run_as_root=True)
+        lvremove = ('lvremove', '-f', path)
+        try:
+            execute(*lvremove, attempts=3, run_as_root=True)
+        except processutils.ProcessExecutionError as exp:
+            errors.append(str(exp))
+    if errors:
+        raise exception.VolumesNotRemoved(reason=(', ').join(errors))
 
 
 def pick_disk_driver_name(hypervisor_version, is_block_dev=False):