From ce531dd1b763704b9043ddde8e80ac99cd193660 Mon Sep 17 00:00:00 2001
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
Date: Wed, 25 Oct 2017 05:57:11 -0400
Subject: [PATCH] libvirt: disconnect volume from host during detach

Under certain failure scenarios it may be that although the libvirt
definition for the volume has been removed for the instance that the
associated storage lun on the compute server may not have been fully
cleaned up yet.

In case users try an other attempt to detach volume we should not stop
the process whether the device is not found in domain definition but
try to disconnect the logical device from host.

This commit makes the process to attempt a disconnect volume even if
the device is not attached to the guest.

Closes-Bug: #1727260
Change-Id: I4182642aab3fd2ffb1c97d2de9bdca58982289d8
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
---
 nova/tests/unit/virt/libvirt/test_driver.py | 39 +++++++++++++++++++--
 nova/virt/libvirt/driver.py                 |  6 +++-
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 77cf1995528e..3cd945cbc85b 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -6913,8 +6913,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 mock_disconnect_volume.assert_called_with(
                     None, connection_info, instance, encryption=None)
 
+    @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>
@@ -6930,10 +6932,41 @@ 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(
+            None, connection_info, instance, encryption=None)
+
+    @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 = mock.MagicMock()
+
+        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_disconnect_volume.assert_called_once_with(
+            None, connection_info, instance, encryption=encryption)
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver')
     @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 751f82e0e032..ca9256392bc4 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1568,7 +1568,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