From d773e3b1469a76f129d2f7a4edab68d007f3f928 Mon Sep 17 00:00:00 2001
From: yenai <yenai@cmss.chinamobile.com>
Date: Wed, 23 Jan 2019 11:02:33 +0800
Subject: [PATCH] libvirt: disconnect volume when encryption fails

The compute node has left the attachment information after the
encryption fails. This patch fixes this by ensuring the volume is
disconnected when an exception occurs.

Change-Id: I9d652f182d83a2557ff6ed0b21c69a735c3f9840
Closes-Bug: #1812945
(cherry picked from commit 79bcb4e21b7909f3e989d4fbcf68ac268680b865)
---
 nova/tests/unit/virt/libvirt/test_driver.py | 60 +++++++++++++++++++++
 nova/virt/libvirt/driver.py                 | 11 +++-
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index abdd0943233e..369f3214ad8e 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -7450,6 +7450,66 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         _set_cache_mode.assert_called_once_with(config)
         self.assertEqual(config_guest_disk.to_xml(), config.to_xml())
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_driver')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_attach_encryptor')
+    def test_connect_volume_encryption_success(
+            self, mock_attach_encryptor, mock_get_volume_driver):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        connection_info = {'driver_volume_type': 'fake',
+                           'data': {'device_path': '/fake',
+                                    'access_mode': 'rw',
+                                    'volume_id': uuids.volume_id}}
+        encryption = {'provider': encryptors.LUKS,
+                      'encryption_key_id': uuids.encryption_key_id}
+        instance = mock.sentinel.instance
+
+        drvr._connect_volume(self.context, connection_info, instance,
+                             encryption=encryption)
+
+        mock_get_volume_driver.assert_called_once_with(connection_info)
+        mock_volume_driver.connect_volume.assert_called_once_with(
+            connection_info, instance)
+        mock_attach_encryptor.assert_called_once_with(
+            self.context, connection_info, encryption, True)
+        mock_volume_driver.disconnect_volume.assert_not_called()
+
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_driver')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_attach_encryptor')
+    def test_connect_volume_encryption_fail(
+            self, mock_attach_encryptor, mock_get_volume_driver):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        connection_info = {'driver_volume_type': 'fake',
+                           'data': {'device_path': '/fake',
+                                    'access_mode': 'rw',
+                                    'volume_id': uuids.volume_id}}
+        encryption = {'provider': encryptors.LUKS,
+                      'encryption_key_id': uuids.encryption_key_id}
+        instance = mock.sentinel.instance
+        mock_attach_encryptor.side_effect = processutils.ProcessExecutionError
+
+        self.assertRaises(processutils.ProcessExecutionError,
+                          drvr._connect_volume,
+                          self.context, connection_info, instance,
+                          encryption=encryption)
+
+        mock_get_volume_driver.assert_called_once_with(connection_info)
+        mock_volume_driver.connect_volume.assert_called_once_with(
+            connection_info, instance)
+        mock_attach_encryptor.assert_called_once_with(
+            self.context, connection_info, encryption, True)
+        mock_volume_driver.disconnect_volume.assert_called_once_with(
+            connection_info, instance)
+
     @mock.patch.object(key_manager, 'API')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_use_native_luks')
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 83f11ea933cf..f0cbeac56e5a 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1274,8 +1274,15 @@ class LibvirtDriver(driver.ComputeDriver):
                         encryption=None, allow_native_luks=True):
         vol_driver = self._get_volume_driver(connection_info)
         vol_driver.connect_volume(connection_info, instance)
-        self._attach_encryptor(context, connection_info, encryption,
-                               allow_native_luks)
+        try:
+            self._attach_encryptor(
+                context, connection_info, encryption, allow_native_luks)
+        except Exception:
+            # Encryption failed so rollback the volume connection.
+            with excutils.save_and_reraise_exception(logger=LOG):
+                LOG.exception("Failure attaching encryptor; rolling back "
+                              "volume connection", instance=instance)
+                vol_driver.disconnect_volume(connection_info, instance)
 
     def _should_disconnect_target(self, context, connection_info, instance):
         connection_count = 0