openstack/cinder
Code Issues Proposed changes

Optimize rbd upload volume to image

Browse Source 
Open the source volume directly rather than using a
temporary file.

Closes-Bug: #2080060
Change-Id: I5415be9ad70a0d622dcb6fcb292e7897a3195a6c
(cherry picked from commit 0fdf2e141c)
This commit is contained in:
Eric Harney 2024-09-04 14:53:10 +00:00
parent 530376bc5d
commit c4c587f3bb
9 changed files with 91 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cinder/image/image_utils.py
View File

@ -1088,6 +1088,7 @@ def upload_volume(context: context.RequestContext,
image_service: glance.GlanceImageService,
image_meta: dict,
volume_path: str,
volume_fd = None,
volume_format: str = 'raw',
run_as_root: bool = True,
compress: bool = True,
@ -1104,6 +1105,12 @@ def upload_volume(context: context.RequestContext,
if (image_meta['disk_format'] == volume_format):
LOG.debug("%s was %s, no need to convert to %s",
image_id, volume_format, image_meta['disk_format'])
if volume_fd is not None:
image_service.update(context, image_id, {},
volume_fd,
store_id=store_id,
base_image_ref=base_image_ref)
else:
with chown_if_needed(volume_path):
with open(volume_path, 'rb') as image_file:
image_service.update(context, image_id, {},

31
cinder/tests/unit/test_image_utils.py
View File

@ -937,6 +937,37 @@ class TestUploadVolume(test.TestCase):
ctxt, image_meta['id'], {}, mock_proxy.return_value,
store_id=None, base_image_ref=None)
@mock.patch('eventlet.tpool.Proxy')
@mock.patch('cinder.image.image_utils.utils.temporary_chown')
@mock.patch('cinder.image.image_utils.open', new_callable=mock.mock_open)
@mock.patch('cinder.image.image_utils.qemu_img_info')
@mock.patch('cinder.image.image_utils.convert_image')
@mock.patch('cinder.image.image_utils.temporary_file')
@mock.patch('cinder.image.image_utils.os')
def test_same_format_fd(self, mock_os, mock_temp, mock_convert, mock_info,
mock_open, mock_chown, mock_proxy):
ctxt = mock.sentinel.context
image_service = mock.Mock()
image_meta = {'id': 'test_id',
'disk_format': 'raw',
'container_format': mock.sentinel.container_format}
mock_os.name = 'posix'
mock_os.access.return_value = False
output = image_utils.upload_volume(ctxt, image_service, image_meta,
None,
volume_fd=mock.sentinel.volume_fd)
self.assertIsNone(output)
self.assertFalse(mock_convert.called)
self.assertFalse(mock_info.called)
mock_chown.assert_not_called()
mock_open.assert_not_called()
mock_proxy.assert_not_called()
image_service.update.assert_called_once_with(
ctxt, image_meta['id'], {}, mock.sentinel.volume_fd,
store_id=None, base_image_ref=None)
@mock.patch('cinder.image.accelerator.ImageAccel._get_engine')
@mock.patch('cinder.image.accelerator.ImageAccel.is_engine_ready',
return_value = True)

7
cinder/tests/unit/volume/drivers/test_quobyte.py
View File

@ -1335,7 +1335,7 @@ class QuobyteDriverTestCase(test.TestCase):
mock_upload_volume.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False,
store_id=None, base_image_ref=None, compress=True,
volume_format='raw')
volume_format='raw', volume_fd=None)
self.assertTrue(mock_create_temporary_file.called)
@mock.patch('cinder.db.volume_glance_metadata_get', return_value={})
@ -1392,7 +1392,7 @@ class QuobyteDriverTestCase(test.TestCase):
mock_upload_volume.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False,
store_id=None, base_image_ref=None, compress=True,
volume_format='raw')
volume_format='raw', volume_fd=None)
self.assertTrue(mock_create_temporary_file.called)
@mock.patch('cinder.db.volume_glance_metadata_get', return_value={})
@ -1449,7 +1449,8 @@ class QuobyteDriverTestCase(test.TestCase):
mock_convert_image.assert_called_once_with(
volume_path, upload_path, 'raw', run_as_root=False)
mock_upload_volume.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False,
mock.ANY, mock.ANY, mock.ANY, upload_path, volume_fd=None,
run_as_root=False,
store_id=None, base_image_ref=None, compress=True,
volume_format='raw')
self.assertTrue(mock_create_temporary_file.called)

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

@ -3475,6 +3475,25 @@ class RBDTestCase(test.TestCase):
self.assertEqual({'provider_location': None}, ret)
@common_mocks
def test_copy_volume_to_image(self):
mock_uv = self.mock_object(cinder.volume.volume_utils, 'upload_volume')
mock_get_rbd_handle = self.mock_object(
self.driver, '_get_rbd_handle',
return_value=mock.sentinel.rbd_handle)
self.driver.copy_volume_to_image(mock.sentinel.context,
mock.sentinel.volume,
mock.sentinel.image_service,
mock.sentinel.image_meta)
mock_get_rbd_handle.assert_called_once_with(mock.sentinel.volume)
mock_uv.assert_called_once_with(mock.sentinel.context,
mock.sentinel.image_service,
mock.sentinel.image_meta,
None,
mock.sentinel.volume,
volume_fd=mock.sentinel.rbd_handle)
class ManagedRBDTestCase(test_driver.BaseDriverTestCase):
driver_name = "cinder.volume.drivers.rbd.RBDDriver"

3
cinder/tests/unit/windows/test_iscsi.py
View File

@ -416,7 +416,8 @@ class TestWindowsISCSIDriver(test.TestCase):
expected_tmp_vhd_path)
mock_upload_volume.assert_called_once_with(
mock.sentinel.context, mock.sentinel.image_service,
fake_image_meta, expected_tmp_vhd_path, volume_format='vhd',
fake_image_meta, expected_tmp_vhd_path, volume_fd=None,
volume_format='vhd',
store_id='fake-store', base_image_ref=None,
compress=True, run_as_root=True)
mock_delete_if_exists.assert_called_once_with(

3
cinder/tests/unit/windows/test_smbfs.py
View File

@ -789,7 +789,8 @@ class WindowsSmbFsTestCase(test.TestCase):
fake_upload_volume.assert_called_once_with(
mock.sentinel.context, mock.sentinel.image_service,
fake_image_meta, upload_path, volume_format=fake_img_format,
fake_image_meta, upload_path, volume_fd=None,
volume_format=fake_img_format,
store_id='fake-store', base_image_ref=None, compress=True,
run_as_root=True)

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

@ -2083,20 +2083,18 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
raise exception.ReplicationError(reason=err_msg,
volume_id=volume.id)
def _get_rbd_handle(self, volume: Volume):
conn = self.initialize_connection(volume, {})
connector = volume_utils.brick_get_connector('rbd')
return connector._get_rbd_handle(conn['data'])
def copy_volume_to_image(self, context, volume, image_service, image_meta):
tmp_dir = volume_utils.image_conversion_dir()
tmp_file = os.path.join(tmp_dir,
volume.name + '-' + image_meta['id'])
with fileutils.remove_path_on_error(tmp_file):
args = ['rbd', 'export',
'--pool', self.configuration.rbd_pool,
volume.name, tmp_file]
args.extend(self._ceph_args())
self._try_execute(*args)
volume_utils.upload_volume(context, image_service,
image_meta, tmp_file,
volume)
os.unlink(tmp_file)
source_handle = self._get_rbd_handle(volume)
volume_utils.upload_volume(context, image_service, image_meta, None,
volume, volume_fd=source_handle)
def extend_volume(self, volume: Volume, new_size: str) -> None:
"""Extend an existing volume."""

6
cinder/volume/volume_utils.py
View File

@ -1340,7 +1340,8 @@ def upload_volume(context: context.RequestContext,
volume: 'objects.Volume',
volume_format: str = 'raw',
run_as_root: bool = True,
compress: bool = True) -> None:
compress: bool = True,
volume_fd = None) -> None:
# retrieve store information from extra-specs
store_id = volume.volume_type.extra_specs.get('image_service:store_id')
@ -1353,7 +1354,8 @@ def upload_volume(context: context.RequestContext,
volume_format=volume_format,
run_as_root=run_as_root,
compress=compress, store_id=store_id,
base_image_ref=base_image_ref)
base_image_ref=base_image_ref,
volume_fd=volume_fd)
def get_backend_configuration(backend_name, backend_opts=None):

5
releasenotes/notes/rbd-optimize-image-upload-836c9df06674a665.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
RBD driver: No longer copy the RBD source volume image to a temporary
file when uploading a volume to an image.