Merge "libvirt: disconnect volume from host during detach" into stable/pike16.1.2

2 files changed, 135 insertions, 9 deletions

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index d4c8a20909..768151e627 100755
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -6972,8 +6972,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 mock_disconnect_volume.assert_called_with(
                     connection_info, 'vdc', instance)
 
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
     @mock.patch('nova.virt.libvirt.host.Host.get_domain')
-    def test_detach_volume_disk_not_found(self, mock_get_domain):
+    def test_detach_volume_disk_not_found(self, mock_get_domain,
+                                          mock_disconnect_volume):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
         instance = objects.Instance(**self.test_instance)
         mock_xml_without_disk = """<domain>
@@ -6989,10 +6991,115 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
                                       5678]
         mock_get_domain.return_value = mock_dom
-        self.assertRaises(exception.DiskNotFound, drvr.detach_volume,
-                          connection_info, instance, '/dev/vdc')
+
+        drvr.detach_volume(connection_info, instance, '/dev/vdc')
 
         mock_get_domain.assert_called_once_with(instance)
+        mock_disconnect_volume.assert_called_once_with(
+            connection_info, 'vdc', instance)
+
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
+    @mock.patch('nova.virt.libvirt.host.Host.get_domain')
+    def test_detach_volume_disk_not_found_encryption(self, mock_get_domain,
+                                                     mock_disconnect_volume,
+                                                     mock_get_encryptor):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instance = objects.Instance(**self.test_instance)
+        mock_xml_without_disk = """<domain>
+  <devices>
+  </devices>
+</domain>"""
+        mock_dom = mock.MagicMock(return_value=mock_xml_without_disk)
+        encryption = {"provider": "NoOpEncryptor"}
+        mock_encryptor = mock.MagicMock(spec=encryptors.nop.NoOpEncryptor)
+        mock_get_encryptor.return_value = mock_encryptor
+
+        connection_info = {"driver_volume_type": "fake",
+                           "data": {"device_path": "/fake",
+                                    "access_mode": "rw"}}
+
+        mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
+                                      5678]
+        mock_get_domain.return_value = mock_dom
+
+        drvr.detach_volume(connection_info, instance, '/dev/vdc',
+                           encryption)
+        mock_get_encryptor.assert_called_once_with(connection_info, encryption)
+        mock_encryptor.detach_volume.assert_called_once_with(**encryption)
+        mock_disconnect_volume.assert_called_once_with(
+            connection_info, 'vdc', instance)
+
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
+    @mock.patch('nova.virt.libvirt.host.Host.get_domain')
+    def test_detach_volume_disk_not_found_encryption_err(self, mock_get_domain,
+                                                     mock_disconnect_volume,
+                                                     mock_get_encryptor):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instance = objects.Instance(**self.test_instance)
+        mock_xml_without_disk = """<domain>
+  <devices>
+  </devices>
+</domain>"""
+        mock_dom = mock.MagicMock(return_value=mock_xml_without_disk)
+        encryption = {"provider": "NoOpEncryptor"}
+        mock_encryptor = mock.MagicMock(spec=encryptors.nop.NoOpEncryptor)
+        mock_encryptor.detach_volume = mock.MagicMock(
+            side_effect=processutils.ProcessExecutionError(exit_code=4)
+        )
+        mock_get_encryptor.return_value = mock_encryptor
+
+        connection_info = {"driver_volume_type": "fake",
+                           "data": {"device_path": "/fake",
+                                    "access_mode": "rw"}}
+
+        mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
+                                      5678]
+        mock_get_domain.return_value = mock_dom
+
+        drvr.detach_volume(connection_info, instance, '/dev/vdc',
+                           encryption)
+        mock_get_encryptor.assert_called_once_with(connection_info, encryption)
+        mock_encryptor.detach_volume.assert_called_once_with(**encryption)
+        mock_disconnect_volume.assert_called_once_with(
+            connection_info, 'vdc', instance)
+
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
+    @mock.patch('nova.virt.libvirt.host.Host.get_domain')
+    def test_detach_volume_disk_not_found_encryption_err_reraise(
+            self, mock_get_domain, mock_disconnect_volume,
+            mock_get_encryptor):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instance = objects.Instance(**self.test_instance)
+        mock_xml_without_disk = """<domain>
+  <devices>
+  </devices>
+</domain>"""
+        mock_dom = mock.MagicMock(return_value=mock_xml_without_disk)
+        encryption = {"provider": "NoOpEncryptor"}
+        mock_encryptor = mock.MagicMock(spec=encryptors.nop.NoOpEncryptor)
+        # Any nonzero exit code other than 4 would work here.
+        mock_encryptor.detach_volume = mock.MagicMock(
+            side_effect=processutils.ProcessExecutionError(exit_code=1)
+        )
+        mock_get_encryptor.return_value = mock_encryptor
+
+        connection_info = {"driver_volume_type": "fake",
+                           "data": {"device_path": "/fake",
+                                    "access_mode": "rw"}}
+
+        mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
+                                      5678]
+        mock_get_domain.return_value = mock_dom
+
+        self.assertRaises(processutils.ProcessExecutionError,
+                          drvr.detach_volume, connection_info, instance,
+                          '/dev/vdc', encryption)
+        mock_get_encryptor.assert_called_once_with(connection_info, encryption)
+        mock_encryptor.detach_volume.assert_called_once_with(**encryption)
+        mock_disconnect_volume.assert_not_called()
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 678b85c72d..e70c9673a9 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1442,11 +1442,6 @@ class LibvirtDriver(driver.ComputeDriver):
                                                              live=live)
             wait_for_detach()
 
-            if encryption:
-                encryptor = self._get_volume_encryptor(connection_info,
-                                                       encryption)
-                encryptor.detach_volume(**encryption)
-
         except exception.InstanceNotFound:
             # NOTE(zhaoqin): If the instance does not exist, _lookup_by_name()
             #                will throw InstanceNotFound exception. Need to
@@ -1454,7 +1449,11 @@ class LibvirtDriver(driver.ComputeDriver):
             LOG.warning("During detach_volume, instance disappeared.",
                         instance=instance)
         except exception.DeviceNotFound:
-            raise exception.DiskNotFound(location=disk_dev)
+            # We should still try to disconnect logical device from
+            # host, an error might have happened during a previous
+            # call.
+            LOG.info("Device %s not found in instance.",
+                     disk_dev, instance=instance)
         except libvirt.libvirtError as ex:
             # NOTE(vish): This is called to cleanup volumes after live
             #             migration, so we should still disconnect even if
@@ -1467,6 +1466,26 @@ class LibvirtDriver(driver.ComputeDriver):
             else:
                 raise
 
+        try:
+            if encryption:
+                encryptor = self._get_volume_encryptor(connection_info,
+                                                       encryption)
+                encryptor.detach_volume(**encryption)
+        except processutils.ProcessExecutionError as e:
+            # cryptsetup returns 4 when attempting to destroy a non-existent
+            # dm-crypt device. We assume here that the caller hasn't specified
+            # the wrong device, and that it doesn't exist because it has
+            # already been destroyed.
+            if e.exit_code == 4:
+                LOG.debug("Ignoring exit code 4, volume already destroyed")
+            else:
+                with excutils.save_and_reraise_exception():
+                    LOG.warning("Could not disconnect encrypted volume "
+                                "%(volume)s. If dm-crypt device is still "
+                                "active it will have to be destroyed manually "
+                                "for cleanup to succeed.",
+                                {'volume': disk_dev})
+
         self._disconnect_volume(connection_info, disk_dev, instance)
 
     def extend_volume(self, connection_info, instance):