openstack/cinder
Code Issues Proposed changes

[rbd] Fix create encrypted volume from snapshot

Browse Source 
Usually the source volume would be the same size or smaller
than the destination volume and they must share the same
volume-type. In particular, when the destination volume is
same size as the source volume, creating an encrypted volume
from a snapshot of an encrypted volume truncates the data in
the new volume.

In order to fix this the RBD workflow would be something
like this:
A source luks volume would be 1026M, we write some data
and create a snap from it. We like to create a new luks
volume from a snapshot so the create_volume_from_snapshot()
method performs a RBD clone first and then a resize if needed.

In addition the _clone() method creates a clone
(copy-on-write child) of the parent snapshot. Object size
will be identical to that of the parent image unless specified
(we don't in cinder) so size will be the same as the parent
snapshot.

If the desired size of the destination luks volume is 1G the
create_volume_from_snapshot() won't perform any resize and
will be 1026M as the parent. This solves bug #1922408 because
we don't force it to resize and because of that we don't
truncate the data anymore.

The second case scenario is when we would like to increase
the size of the destination volume. As far as I can tell this
won't face the encryption header problem but we still need to
calculate the difference size to provide the size that the
user is expecting.

That's why the fix proposed calculate the new_size based on:
size difference = desired size - size of source volume
new size = current size + size difference

Closes-Bug: #1922408
Co-Authored-By: Sofia Enriquez <lsofia.enriquez@gmail.com>
Change-Id: I220b5e3b01d115262a8b1dd45758f0531aea0edf
This commit is contained in:
haixin 2021-04-03 12:36:14 +08:00 committed by Sofia Enriquez
parent 393c2e4ad9
commit cf1c525296
3 changed files with 104 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
cinder/tests/unit/volume/drivers/test_rbd.py
View File

@ -888,13 +888,13 @@ class RBDTestCase(test.TestCase):
self.mock_proxy().__enter__().volume.op_features.return_value = 1
self.mock_rbd.RBD_OPERATION_FEATURE_CLONE_PARENT = 1
snapshot = mock.Mock()
self.cfg.rbd_flatten_volume_from_snapshot = False
with mock.patch.object(driver, 'LOG') as \
mock_log:
self.driver.create_volume_from_snapshot(self.volume_a, snapshot)
self.driver.create_volume_from_snapshot(self.volume_a,
self.snapshot)
mock_log.info.assert_called_with('Using v2 Clone API')
@ -910,13 +910,13 @@ class RBDTestCase(test.TestCase):
self.mock_proxy().__enter__().volume.op_features.return_value = 0
self.mock_rbd.RBD_OPERATION_FEATURE_CLONE_PARENT = 1
snapshot = mock.Mock()
self.cfg.rbd_flatten_volume_from_snapshot = False
with mock.patch.object(driver, 'LOG') as \
mock_log:
self.driver.create_volume_from_snapshot(self.volume_a, snapshot)
self.driver.create_volume_from_snapshot(self.volume_a,
self.snapshot)
self.assertTrue(any(m for m in mock_log.warning.call_args_list
if 'Not using v2 clone API' in m[0][0]))
@ -1859,7 +1859,7 @@ class RBDTestCase(test.TestCase):
@mock.patch.object(driver.RBDDriver, '_resize', mock.Mock())
def test_create_vol_from_snap_replication(self, mock_clone):
self.cfg.rbd_flatten_volume_from_snapshot = False
snapshot = mock.Mock()
snapshot = self.snapshot_b
res = self.driver.create_volume_from_snapshot(self.volume_a, snapshot)
@ -1869,6 +1869,75 @@ class RBDTestCase(test.TestCase):
snapshot.volume_name,
snapshot.name)
@common_mocks
@mock.patch.object(driver.RBDDriver, '_clone',
return_value=mock.sentinel.volume_update)
def test_create_encrypted_vol_from_snap_same_size(self, mock_clone):
"""Test create encrypted volume from encrypted snapshot.
When creating an encrypted volume from encrypted snapshot
the new volume is same size than the snapshot.
"""
self.cfg.rbd_flatten_volume_from_snapshot = False
volume_size = self.volume_c.size
self.snapshot_b.volume_size = volume_size
mock_resize = self.mock_object(self.driver, '_resize')
mock_new_size = self.mock_object(self.driver,
'_calculate_new_size')
res = self.driver.create_volume_from_snapshot(self.volume_c,
self.snapshot_b)
self.assertEqual(mock.sentinel.volume_update, res)
mock_resize.assert_not_called()
mock_new_size.assert_not_called()
@common_mocks
@mock.patch.object(driver.RBDDriver, '_clone',
return_value=mock.sentinel.volume_update)
def test_create_encrypted_vol_from_snap(self, mock_clone):
"""Test create encrypted volume from encrypted snapshot.
When creating an encrypted volume from encrypted snapshot
the new volume is larger than the snapshot (12GB vs 11GB).
"""
self.cfg.rbd_flatten_volume_from_snapshot = False
new_size_bytes = 12288
diff_size = 1
volume_size = 11
self.snapshot_b.volume_size = volume_size
mock_resize = self.mock_object(self.driver, '_resize')
mock_new_size = self.mock_object(self.driver,
'_calculate_new_size')
mock_new_size.return_value = new_size_bytes
res = self.driver.create_volume_from_snapshot(self.volume_c,
self.snapshot_b)
self.assertEqual(mock.sentinel.volume_update, res)
mock_resize.assert_called_once_with(self.volume_c,
size=new_size_bytes)
volume_name = self.volume_c.name
mock_new_size.assert_called_once_with(diff_size,
volume_name)
@common_mocks
@mock.patch.object(driver.RBDDriver, '_clone',
return_value=mock.sentinel.volume_update)
def test_create_unencrypted_vol_from_snap(self, mock_clone):
"""Test create regular volume from regular snapshot"""
self.cfg.rbd_flatten_volume_from_snapshot = False
self.snapshot_b.volume.size = 9
mock_resize = self.mock_object(self.driver, '_resize')
mock_new_size = self.mock_object(self.driver,
'_calculate_new_size')
res = self.driver.create_volume_from_snapshot(self.volume_b,
self.snapshot_b)
self.assertEqual(mock.sentinel.volume_update, res)
mock_resize.assert_called_once_with(self.volume_b, size=None)
mock_new_size.assert_not_called()
@common_mocks
def test_extend_volume(self):
fake_size = '20'

26
cinder/volume/drivers/rbd.py
View File

@ -1041,14 +1041,36 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
with RBDVolumeProxy(self, volume.name) as vol:
vol.resize(size)
def _calculate_new_size(self, size_diff, volume_name):
with RBDVolumeProxy(self, volume_name) as vol:
current_size_bytes = vol.volume.size()
size_diff_bytes = size_diff * units.Gi
new_size_bytes = current_size_bytes + size_diff_bytes
return new_size_bytes
def create_volume_from_snapshot(self, volume, snapshot):
"""Creates a volume from a snapshot."""
volume_update = self._clone(volume, self.configuration.rbd_pool,
snapshot.volume_name, snapshot.name)
if self.configuration.rbd_flatten_volume_from_snapshot:
self._flatten(self.configuration.rbd_pool, volume.name)
if int(volume.size):
self._resize(volume)
snap_vol_size = snapshot.volume_size
# In case the destination size is bigger than the snapshot size
# we should resize. In particular when the destination volume
# is encrypted we should consider the encryption header size.
# Because of this, we need to calculate the difference size to
# provide the size that the user is expecting.
# Otherwise if the destination volume size is equal to the
# source volume size we don't perform a resize.
if volume.size > snap_vol_size:
new_size = None
# In case the volume is encrypted we need to consider the
# size of the encryption header when resizing the volume
if volume.encryption_key_id:
size_diff = volume.size - snap_vol_size
new_size = self._calculate_new_size(size_diff, volume.name)
self._resize(volume, size=new_size)
self._show_msg_check_clone_v2_api(snapshot.volume_name)
return volume_update

6
releasenotes/notes/bug-1922408-create-encryption-volume-from-snapshot-skip-resize-bb5d77c5e912b5c1.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
RBD driver `Bug #1922408
<https://bugs.launchpad.net/cinder/+bug/1922408>`_:
Fixed create encrypted volume from encrypted snapshot.