File 0001-Block-swap-volume-attempts-with-encrypted-volumes.patch of Package openstack-nova
From 77200eab3511339673933b762b2bffd1e0accb0c Mon Sep 17 00:00:00 2001
From: Lee Yarwood <lyarwood@redhat.com>
Date: Mon, 12 Feb 2018 18:07:14 +0000
Subject: libvirt: Block swap volume attempts with encrypted volumes
Prior to Queens any attempt to swap between encrypted volumes would
result in unencrypted data being written to the new volume. This
unencrypted data would then be overwritten the next time the volume was
reattached to an instance as the host no longer identifies it as
encrypted, resulting in the volume being reformatted by n-cpu.
This stable branch only change uses limited parts of the following
changes to block swap_volume with encrypted volumes prior to Queens.
Ica323b87fa85a454fca9d46ada3677f18fe50022
The request context is provided to swap_volume in order to look up the
encryption metadata of a volume.
Ibfa64f18bbd2fb70db7791330ed1a64fe61c1355
Attempts to swap encrypted volumes are blocked with NotImplementedError raised.
Ie02d298cd92d5b5ebcbbcd2b0e8be01f197bfafb
The serial of a volume is used as the id if connection_info for the
volume doesn't contain the volume_id key. Required to avoid bug #1746609.
Closes-bug: #1739593
---
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index d7b8b71383..db271ea175 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -4907,8 +4907,8 @@ class ComputeManager(manager.Manager):
"old: %(old_cinfo)s",
{'new_cinfo': new_cinfo, 'old_cinfo': old_cinfo},
contex=context, instance=instance)
- self.driver.swap_volume(old_cinfo, new_cinfo, instance, mountpoint,
- resize_to)
+ self.driver.swap_volume(context, old_cinfo, new_cinfo, instance,
+ mountpoint, resize_to)
LOG.debug("swap_volume: Driver volume swap returned, new "
"connection_info is now : %(new_cinfo)s",
{'new_cinfo': new_cinfo})
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 467c862d41..7c40121a82 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -14848,8 +14848,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
conf = mock.MagicMock(source_path='/fake-new-volume')
get_volume_config.return_value = conf
- conn.swap_volume(old_connection_info, new_connection_info, instance,
- '/dev/vdb', 1)
+ conn.swap_volume(self.context, old_connection_info,
+ new_connection_info, instance, '/dev/vdb', 1)
get_guest.assert_called_once_with(instance)
connect_volume.assert_called_once_with(new_connection_info, disk_info)
@@ -14866,6 +14866,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
def test_swap_volume_driver_source_is_snapshot(self):
self._test_swap_volume_driver(source_type='snapshot')
+ @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.rebase')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@@ -14875,7 +14876,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
@mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
write_config, get_guest, get_disk, get_volume_config,
- connect_volume, disconnect_volume, rebase):
+ connect_volume, disconnect_volume, rebase,
+ get_volume_encryption):
"""Assert that disconnect_volume is called for the new volume if an
error is encountered while rebasing
"""
@@ -14883,12 +14885,13 @@ class LibvirtConnTestCase(test.NoDBTestCase):
instance = objects.Instance(**self.test_instance)
guest = libvirt_guest.Guest(mock.MagicMock())
get_guest.return_value = guest
+ get_volume_encryption.return_value = {}
exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
rebase.side_effect = exc
self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
- mock.sentinel.old_connection_info,
+ self.context, mock.sentinel.old_connection_info,
mock.sentinel.new_connection_info,
instance, '/dev/vdb', 0)
connect_volume.assert_called_once_with(
@@ -14897,6 +14900,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
disconnect_volume.assert_called_once_with(
mock.sentinel.new_connection_info, 'vdb')
+ @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.abort_job')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@@ -14907,7 +14911,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
@mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
write_config, get_guest, get_disk, get_volume_config,
- connect_volume, disconnect_volume, abort_job, is_job_complete):
+ connect_volume, disconnect_volume, abort_job, is_job_complete,
+ get_volume_encryption):
"""Assert that disconnect_volume is called for the new volume if an
error is encountered while pivoting to the new volume
"""
@@ -14915,13 +14920,14 @@ class LibvirtConnTestCase(test.NoDBTestCase):
instance = objects.Instance(**self.test_instance)
guest = libvirt_guest.Guest(mock.MagicMock())
get_guest.return_value = guest
+ get_volume_encryption.return_value = {}
exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
is_job_complete.return_value = True
abort_job.side_effect = [None, exc]
self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
- mock.sentinel.old_connection_info,
+ self.context, mock.sentinel.old_connection_info,
mock.sentinel.new_connection_info,
instance, '/dev/vdb', 0)
connect_volume.assert_called_once_with(
diff --git a/nova/tests/unit/virt/test_virt_drivers.py b/nova/tests/unit/virt/test_virt_drivers.py
index 4d17e36d1f..33d6c46eda 100644
--- a/nova/tests/unit/virt/test_virt_drivers.py
+++ b/nova/tests/unit/virt/test_virt_drivers.py
@@ -495,7 +495,7 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
instance_ref,
'/dev/sda'))
self.assertIsNone(
- self.connection.swap_volume({'driver_volume_type': 'fake',
+ self.connection.swap_volume(None, {'driver_volume_type': 'fake',
'data': {}},
{'driver_volume_type': 'fake',
'data': {}},
diff --git a/nova/virt/block_device.py b/nova/virt/block_device.py
index f6ed6a30e9..c5a3427518 100644
--- a/nova/virt/block_device.py
+++ b/nova/virt/block_device.py
@@ -570,3 +570,13 @@ def is_block_device_mapping(bdm):
return (bdm.source_type in ('image', 'volume', 'snapshot', 'blank')
and bdm.destination_type == 'volume'
and is_implemented(bdm))
+
+
+def get_volume_id(connection_info):
+ if connection_info:
+ # Check for volume_id in 'data' and if not there, fallback to
+ # the 'serial' that the DriverVolumeBlockDevice adds during attach.
+ volume_id = connection_info.get('data', {}).get('volume_id')
+ if not volume_id:
+ volume_id = connection_info.get('serial')
+ return volume_id
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 7bd446451d..9d8307e33c 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -473,10 +473,11 @@ class ComputeDriver(object):
"""Detach the disk attached to the instance."""
raise NotImplementedError()
- def swap_volume(self, old_connection_info, new_connection_info,
+ def swap_volume(self, context, old_connection_info, new_connection_info,
instance, mountpoint, resize_to):
"""Replace the volume attached to the given `instance`.
+ :param context: The request context.
:param dict old_connection_info:
The volume for this connection gets detached from the given
`instance`.
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index 5b73789711..9c9aa30368 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -296,7 +296,7 @@ class FakeDriver(driver.ComputeDriver):
except KeyError:
pass
- def swap_volume(self, old_connection_info, new_connection_info,
+ def swap_volume(self, context, old_connection_info, new_connection_info,
instance, mountpoint, resize_to):
"""Replace the disk attached to the instance."""
instance_name = instance.name
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 3f4d7939de..952a07e892 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1114,6 +1114,16 @@ class LibvirtDriver(driver.ComputeDriver):
**encryption)
return encryptor
+ def _get_volume_encryption(self, context, connection_info):
+ """Get the encryption metadata dict if it is not provided
+ """
+ encryption = {}
+ volume_id = driver_block_device.get_volume_id(connection_info)
+ if volume_id:
+ encryption = encryptors.get_encryption_metadata(context,
+ self._volume_api, volume_id, connection_info)
+ return encryption
+
def _check_discard_for_attach_volume(self, conf, instance):
"""Perform some checks for volumes configured for discard support.
@@ -1250,9 +1260,19 @@ class LibvirtDriver(driver.ComputeDriver):
finally:
self._host.write_instance_config(xml)
- def swap_volume(self, old_connection_info,
+ def swap_volume(self, context, old_connection_info,
new_connection_info, instance, mountpoint, resize_to):
+ # NOTE(lyarwood): Bug #1739593 uncovered a nasty data corruption
+ # issue that was fixed in Queens by Ica323b87fa85a454fca9d46ada3677f18.
+ # Given the size of the bugfix it was agreed not to backport the change
+ # to earlier stable branches and to instead block swap volume attempts.
+ if self._get_volume_encryption(context, old_connection_info) or \
+ self._get_volume_encryption(context, new_connection_info):
+ raise NotImplementedError(_("Swap volume is not supported when"
+ "using encrypted volumes. For more details see "
+ "https://bugs.launchpad.net/nova/+bug/1739593."))
+
guest = self._host.get_guest(instance)
disk_dev = mountpoint.rpartition("/")[2]
