diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index d4c8a2090921..768151e627e2 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 = """ @@ -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 = """ + + +""" + 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 = """ + + +""" + 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 = """ + + +""" + 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 678b85c72dcd..e70c9673a911 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):