Merge "RBD: use correct stripe unit in clone operation" into stable/victoria

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

@@ -888,13 +888,14 @@ 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)
+        with mock.patch.object(driver, 'LOG') as mock_log:
+            with mock.patch.object(self.driver.rbd.Image(), 'stripe_unit') as \
+                    mock_rbd_image_stripe_unit:
+                mock_rbd_image_stripe_unit.return_value = 4194304
+                self.driver.create_volume_from_snapshot(self.volume_a,
+                                                        self.snapshot)
 
             mock_log.info.assert_called_once()
             self.assertTrue(self.driver._clone_v2_api_checked)
@@ -909,13 +910,17 @@ 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:
+        with mock.patch.object(driver, 'LOG') as mock_log:
+            with mock.patch.object(self.driver.rbd.Image(), 'stripe_unit') as \
+                    mock_rbd_image_stripe_unit:
+                mock_rbd_image_stripe_unit.return_value = 4194304
+                self.driver.create_volume_from_snapshot(self.volume_a,
+                                                        self.snapshot)
 
-            self.driver.create_volume_from_snapshot(self.volume_a, snapshot)
+            self.assertTrue(any(m for m in mock_log.warning.call_args_list
+                                if 'Not using v2 clone API' in m[0][0]))
 
             mock_log.warning.assert_called_once()
             self.assertTrue(self.driver._clone_v2_api_checked)
@@ -1744,13 +1749,13 @@ class RBDTestCase(test.TestCase):
         self.assertEqual(cfg_file, self.driver.keyring_file)
         self.assertIsNone(self.driver.keyring_data)
 
-    @ddt.data({'rbd_chunk_size': 1, 'order': 20},
-              {'rbd_chunk_size': 8, 'order': 23},
-              {'rbd_chunk_size': 32, 'order': 25})
+    @ddt.data({'rbd_chunk_size': 1},
+              {'rbd_chunk_size': 8},
+              {'rbd_chunk_size': 32})
     @ddt.unpack
     @common_mocks
     @mock.patch.object(driver.RBDDriver, '_enable_replication')
-    def test_clone(self, mock_enable_repl, rbd_chunk_size, order):
+    def test_clone(self, mock_enable_repl, rbd_chunk_size):
         self.cfg.rbd_store_chunk_size = rbd_chunk_size
         src_pool = u'images'
         src_image = u'image-name'
@@ -1768,14 +1773,20 @@ class RBDTestCase(test.TestCase):
         # capture both rados client used to perform the clone
         client.__enter__.side_effect = mock__enter__(client)
 
-        res = self.driver._clone(self.volume_a, src_pool, src_image, src_snap)
+        with mock.patch.object(self.driver.rbd.Image(), 'stripe_unit') as \
+                mock_rbd_image_stripe_unit:
+            mock_rbd_image_stripe_unit.return_value = 4194304
+            res = self.driver._clone(self.volume_a, src_pool, src_image,
+                                     src_snap)
 
         self.assertEqual({}, res)
 
         args = [client_stack[0].ioctx, str(src_image), str(src_snap),
                 client_stack[1].ioctx, str(self.volume_a.name)]
+        stripe_unit = max(4194304, rbd_chunk_size * 1048576)
+        expected_order = int(math.log(stripe_unit, 2))
         kwargs = {'features': client.features,
-                  'order': order}
+                  'order': expected_order}
         self.mock_rbd.RBD.return_value.clone.assert_called_once_with(
             *args, **kwargs)
         self.assertEqual(2, client.__enter__.call_count)
@@ -1784,8 +1795,9 @@ class RBDTestCase(test.TestCase):
     @common_mocks
     @mock.patch.object(driver.RBDDriver, '_enable_replication')
     def test_clone_replicated(self, mock_enable_repl):
-        rbd_chunk_size = 1
         order = 20
+        rbd_chunk_size = 1
+        stripe_unit = 1048576
         self.volume_a.volume_type = fake_volume.fake_volume_type_obj(
             self.context,
             id=fake.VOLUME_TYPE_ID,
@@ -1814,7 +1826,11 @@ class RBDTestCase(test.TestCase):
         # capture both rados client used to perform the clone
         client.__enter__.side_effect = mock__enter__(client)
 
-        res = self.driver._clone(self.volume_a, src_pool, src_image, src_snap)
+        with mock.patch.object(self.driver.rbd.Image(), 'stripe_unit') as \
+                mock_rbd_image_stripe_unit:
+            mock_rbd_image_stripe_unit.return_value = stripe_unit
+            res = self.driver._clone(self.volume_a, src_pool, src_image,
+                                     src_snap)
 
         self.assertEqual(expected_update, res)
         mock_enable_repl.assert_called_once_with(self.volume_a)

cinder/volume/drivers/rbd.py

@@ -1004,16 +1004,35 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
         with RBDVolumeProxy(self, volume_name, pool) as vol:
             vol.flatten()
 
+    def _get_stripe_unit(self, ioctx, volume_name):
+        """Return the correct stripe unit for a cloned volume.
+
+        A cloned volume must be created with a stripe unit at least as large
+        as the source volume.  We compute the desired stripe width from
+        rbd_store_chunk_size and compare that to the incoming source volume's
+        stripe width, selecting the larger to avoid error.
+        """
+        default_stripe_unit = \
+            self.configuration.rbd_store_chunk_size * units.Mi
+
+        image = self.rbd.Image(ioctx, volume_name)
+        try:
+            image_stripe_unit = image.stripe_unit()
+        finally:
+            image.close()
+
+        return max(image_stripe_unit, default_stripe_unit)
+
     def _clone(self, volume, src_pool, src_image, src_snap):
         LOG.debug('cloning %(pool)s/%(img)s@%(snap)s to %(dst)s',
                   dict(pool=src_pool, img=src_image, snap=src_snap,
                        dst=volume.name))
 
-        chunk_size = self.configuration.rbd_store_chunk_size * units.Mi
-        order = int(math.log(chunk_size, 2))
         vol_name = utils.convert_str(volume.name)
 
         with RADOSClient(self, src_pool) as src_client:
+            stripe_unit = self._get_stripe_unit(src_client.ioctx, src_image)
+            order = int(math.log(stripe_unit, 2))
             with RADOSClient(self) as dest_client:
                 self.RBDProxy().clone(src_client.ioctx,
                                       utils.convert_str(src_image),
@@ -1022,7 +1041,6 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                                       vol_name,
                                       features=src_client.features,
                                       order=order)
-
             try:
                 volume_update = self._setup_volume(volume)
             except Exception:

releasenotes/notes/rbd-choose-correct-stripe-unit-9d317f4717533fb4.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    `Bug #1931004 <https://bugs.launchpad.net/cinder/+bug/1931004>`_: Fixed
+    use of incorrect stripe unit in RBD image clone causing volume-from-image
+    to fail when using raw images backed by Ceph.