Merge "RBD: Fix upload volume with different format"

cinder/tests/unit/volume/drivers/test_rbd.py

@@ -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):

cinder/volume/drivers/rbd.py

@@ -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."""

releasenotes/notes/fix-rbd-upload-diff-format-38fc4ef24d7145ba.yaml

@@ -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.