RBD: Fix upload volume with different format
With change[1], we introduced an optimized path to upload an RBD
volume to image. However, this does not work well when the image
format is not 'raw' and conversion is required.
Currently there are 2 places where we need handling of the
RBDVolumeIOWrapper object:
1. format inspector
2. qemu-img commands
While testing, I found out the following issues that needs to be
addressed to fix the upload path with conversion:
1. Passing RBDVolumeIOWrapper to format inspector
We fail when calling privsep since RPC cannot serialize the
RBDVolumeIOWrapper object and fails with the following error
ERROR oslo_messaging.rpc.server TypeError: can not serialize 'RBDVolumeIOWrapper' object
2. Handling in format inspector
Either we need to pass the volume_fd var to several methods in
format inspector code or introduce a condition to find out if
we get volume path or RBDVolumeIOWrapper and open it if it's a path
3. Handling in qemu-img commands
We need to use rbd:{pool-name}/{image-name} format instead of file path when
executing qemu-img commands like convert[2] otherwise we will fail with the
below error.
Stderr: "qemu-img: Could not open '<os_brick.initiator.linuxrbd.RBDVolumeIOWrapper object at 0x781f747a6aa0>':
Could not open '<os_brick.initiator.linuxrbd.RBDVolumeIOWrapper object at 0x781f747a6aa0>': No such file or directory\n"
Not to mention passing the volume_fd object to all the qemu related methods
and handling cases when volume_path is none and volume_fd contains the
RBD IO object.
Even after fixing all the issues, it's still not sure that the optimized
path offers better performance compared to the traditional workflow as
qemu-img will use librbd to read the volume file and convert it to a local
path.
Due to the above reasons, we only use the optimized path when the image
format is same as volume format (i.e. raw) and container format is not
'compressed'.
[1] https://review.opendev.org/c/openstack/cinder/+/928024
[2] https://docs.ceph.com/en/latest/rbd/qemu-rbd/#running-qemu-with-rbd
Closes-Bug: #2092534
Change-Id: I32b505aa69c71b62e7e3a52d65d38165d34e97d8
This commit is contained in:
Rajat Dhasmana
parent
bff6a6dfec
commit
7d32076498
@ -26,6 +26,7 @@ import uuid
|
||||
|
||||
import castellan
|
||||
import ddt
|
||||
from oslo_utils import fileutils
|
||||
from oslo_utils import imageutils
|
||||
from oslo_utils import units
|
||||
|
||||
@ -3501,24 +3502,56 @@ class RBDTestCase(test.TestCase):
|
||||
|
||||
self.assertEqual({'provider_location': None}, ret)
|
||||
|
||||
@ddt.data(('bare', 'raw'),
|
||||
('bare', 'qcow2'),
|
||||
('compressed', 'raw'),
|
||||
('compressed', 'qcow2'))
|
||||
@ddt.unpack
|
||||
@common_mocks
|
||||
def test_copy_volume_to_image(self):
|
||||
def test_copy_volume_to_image(self, container_format, disk_format):
|
||||
fake_image_meta = {
|
||||
'id': 'e105244f-4cb8-447b-8452-6f1da459e3ab',
|
||||
'container_format': container_format,
|
||||
'disk_format': disk_format,
|
||||
}
|
||||
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)
|
||||
if container_format != 'compressed' and disk_format == 'raw':
|
||||
self.driver.copy_volume_to_image(mock.sentinel.context,
|
||||
mock.sentinel.volume,
|
||||
mock.sentinel.image_service,
|
||||
fake_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,
|
||||
fake_image_meta,
|
||||
None,
|
||||
mock.sentinel.volume,
|
||||
volume_fd=
|
||||
mock.sentinel.rbd_handle)
|
||||
else:
|
||||
with mock.patch.object(self.driver, '_execute'), \
|
||||
mock.patch.object(fileutils, 'remove_path_on_error'), \
|
||||
mock.patch.object(os, 'unlink'), \
|
||||
mock.patch.object(
|
||||
volume_utils, 'image_conversion_dir') as fake_dir:
|
||||
fake_path = 'fake_path'
|
||||
fake_vol = 'volume-' + fake_image_meta['id']
|
||||
fake_dir.return_value = fake_path
|
||||
fake_vol_path = os.path.join(fake_path, fake_vol)
|
||||
self.driver.copy_volume_to_image(mock.sentinel.context,
|
||||
mock.sentinel.volume,
|
||||
mock.sentinel.image_service,
|
||||
fake_image_meta)
|
||||
mock_get_rbd_handle.assert_not_called()
|
||||
mock_uv.assert_called_once_with(mock.sentinel.context,
|
||||
mock.sentinel.image_service,
|
||||
fake_image_meta,
|
||||
fake_vol_path,
|
||||
mock.sentinel.volume)
|
||||
|
||||
|
||||
class ManagedRBDTestCase(test_driver.BaseDriverTestCase):
|
||||
|
||||
@ -2089,10 +2089,41 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
|
||||
return connector._get_rbd_handle(conn['data'])
|
||||
|
||||
def copy_volume_to_image(self, context, volume, image_service, image_meta):
|
||||
source_handle = self._get_rbd_handle(volume)
|
||||
if image_meta.get('container_format') != 'compressed' and (
|
||||
image_meta['disk_format'] == 'raw'):
|
||||
source_handle = self._get_rbd_handle(volume)
|
||||
|
||||
volume_utils.upload_volume(context, image_service, image_meta, None,
|
||||
volume, volume_fd=source_handle)
|
||||
volume_utils.upload_volume(context, image_service, image_meta,
|
||||
None, volume, volume_fd=source_handle)
|
||||
else:
|
||||
# When the image format is different from volume format, we will
|
||||
# fallback to the old workflow because of the following issues:
|
||||
# 1. Passing RBDVolumeIOWrapper to format inspector
|
||||
# We fail when calling privsep since RPC cannot serialize the
|
||||
# RBDVolumeIOWrapper object
|
||||
# 2. Handling in format inspector
|
||||
# Determine if it's RBD file descriptor and only open the volume
|
||||
# file if it's not
|
||||
# 3. Handling in qemu-img commands
|
||||
# Use rbd:{pool-name}/{image-name} format instead of file path
|
||||
# https://docs.ceph.com/en/latest/rbd/qemu-rbd/#running-qemu-with-rbd # noqa
|
||||
#
|
||||
# Even if above issues are addressed, qemu-img convert will create
|
||||
# a local copy of converted volume file so will need to determine
|
||||
# the performance vs this workflow.
|
||||
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)
|
||||
|
||||
def extend_volume(self, volume: Volume, new_size: str) -> None:
|
||||
"""Extend an existing volume."""
|
||||
|
||||
@ -0,0 +1,7 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
RBD driver `bug #2092534
|
||||
<https://bugs.launchpad.net/cinder/+bug/2092534>`_: Fixed
|
||||
uploading a volume to image when image has different format
|
||||
than volume.
|
||||
Loading…
x
Reference in New Issue
Block a user