RBD: add support for multiattach

Change-Id: Ie3945427b54544a3b411c23bffdad1acb5e508e1

cinder/exception.py

@@ -971,6 +971,11 @@ class PureRetryableException(VolumeBackendAPIException):
     message = _("Retryable Pure Storage Exception encountered")
 
 
+# RBD
+class RBDDriverException(VolumeDriverException):
+    message = _("RBD Cinder driver failure: %(reason)s")
+
+
 # SolidFire
 class SolidFireAPIException(VolumeBackendAPIException):
     message = _("Bad response from SolidFire API")

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

@@ -348,21 +348,20 @@ class RBDTestCase(test.TestCase):
 
     @mock.patch.object(driver.RBDDriver, '_enable_replication',
                        return_value=mock.sentinel.volume_update)
-    def test_enable_replication_if_needed_replicated_volume(self, mock_enable):
+    def test_setup_volume_with_replication(self, mock_enable):
         self.volume_a.volume_type = fake_volume.fake_volume_type_obj(
             self.context,
             id=fake.VOLUME_TYPE_ID,
             extra_specs={'replication_enabled': '<is> True'})
-        res = self.driver._enable_replication_if_needed(self.volume_a)
+        res = self.driver._setup_volume(self.volume_a)
         self.assertEqual(mock.sentinel.volume_update, res)
         mock_enable.assert_called_once_with(self.volume_a)
 
     @ddt.data(False, True)
     @mock.patch.object(driver.RBDDriver, '_enable_replication')
-    def test_enable_replication_if_needed_non_replicated(self, enabled,
+    def test_setup_volume_without_replication(self, enabled, mock_enable):
-                                                         mock_enable):
         self.driver._is_replication_enabled = enabled
-        res = self.driver._enable_replication_if_needed(self.volume_a)
+        res = self.driver._setup_volume(self.volume_a)
         if enabled:
             expect = {'replication_status': fields.ReplicationStatus.DISABLED}
         else:
@@ -1322,7 +1321,7 @@ class RBDTestCase(test.TestCase):
             thin_provisioning_support=True,
             provisioned_capacity_gb=mock.sentinel.provisioned_capacity_gb,
             max_over_subscription_ratio=1.0,
-            multiattach=False,
+            multiattach=True,
             location_info=expected_location_info,
             backend_state='up')
 
@@ -1366,7 +1365,7 @@ class RBDTestCase(test.TestCase):
             reserved_percentage=0,
             thin_provisioning_support=True,
             max_over_subscription_ratio=1.0,
-            multiattach=False,
+            multiattach=True,
             location_info=expected_location_info,
             backend_state='up')
 
@@ -1401,7 +1400,7 @@ class RBDTestCase(test.TestCase):
                         total_capacity_gb='unknown',
                         free_capacity_gb='unknown',
                         reserved_percentage=0,
-                        multiattach=False,
+                        multiattach=True,
                         max_over_subscription_ratio=1.0,
                         thin_provisioning_support=True,
                         location_info=expected_location_info,

cinder/volume/drivers/rbd.py

@@ -131,6 +131,7 @@ CONF = cfg.CONF
 CONF.register_opts(RBD_OPTS, group=configuration.SHARED_CONF_GROUP)
 
 EXTRA_SPECS_REPL_ENABLED = "replication_enabled"
+EXTRA_SPECS_MULTIATTACH = "multiattach"
 
 
 class RBDVolumeProxy(object):
@@ -548,14 +549,7 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
             'free_capacity_gb': 'unknown',
             'reserved_percentage': (
                 self.configuration.safe_get('reserved_percentage')),
-            # NOTE(eharney): Do not enable multiattach for this driver.
+            'multiattach': True,
-            # For multiattach to work correctly, the exclusive-lock
-            # feature required by ceph journaling must be disabled.
-            # This has implications for replication and other Cinder
-            # operations.
-            # Multiattach support for this driver will be investigated
-            # as multi-attach support in Cinder matures.
-            'multiattach': False,
             'thin_provisioning_support': True,
             'max_over_subscription_ratio': (
                 self.configuration.safe_get('max_over_subscription_ratio')),
@@ -721,7 +715,7 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                     raise exception.VolumeBackendAPIException(data=msg)
 
             try:
-                volume_update = self._enable_replication_if_needed(volume)
+                volume_update = self._setup_volume(volume)
             except Exception:
                 self.RBDProxy().remove(client.ioctx, dest_name)
                 src_volume.unprotect_snap(clone_snap)
@@ -761,17 +755,50 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
         return {'replication_status': fields.ReplicationStatus.ENABLED,
                 'replication_driver_data': driver_data}
 
+    def _enable_multiattach(self, volume):
+        multipath_feature_exclusions = [
+            self.rbd.RBD_FEATURE_JOURNALING,
+            self.rbd.RBD_FEATURE_FAST_DIFF,
+            self.rbd.RBD_FEATURE_OBJECT_MAP,
+            self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK,
+        ]
+        vol_name = utils.convert_str(volume.name)
+        with RBDVolumeProxy(self, vol_name) as image:
+            for feature in multipath_feature_exclusions:
+                if image.features() & feature:
+                    image.update_features(feature, False)
+
     def _is_replicated_type(self, volume_type):
         # We do a safe attribute get because volume_type could be None
         specs = getattr(volume_type, 'extra_specs', {})
         return specs.get(EXTRA_SPECS_REPL_ENABLED) == "<is> True"
 
-    def _enable_replication_if_needed(self, volume):
+    def _is_multiattach_type(self, volume_type):
-        if self._is_replicated_type(volume.volume_type):
+        # We do a safe attribute get because volume_type could be None
+        specs = getattr(volume_type, 'extra_specs', {})
+        return specs.get(EXTRA_SPECS_MULTIATTACH) == "<is> True"
+
+    def _setup_volume(self, volume):
+        want_replication = self._is_replicated_type(volume.volume_type)
+        want_multiattach = self._is_multiattach_type(volume.volume_type)
+
+        if want_replication and want_multiattach:
+            msg = _('Replication and Multiattach are mutually exclusive.')
+            raise exception.RBDDriverException(reason=msg)
+
+        if want_replication:
             return self._enable_replication(volume)
+
+        update = None
+
         if self._is_replication_enabled:
-            return {'replication_status': fields.ReplicationStatus.DISABLED}
+            update = {'replication_status':
-        return None
+                      fields.ReplicationStatus.DISABLED}
+
+        if want_multiattach:
+            self._enable_multiattach(volume)
+
+        return update
 
     def _check_encryption_provider(self, volume, context):
         """Check that this is a LUKS encryption provider.
@@ -865,13 +892,14 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                                    old_format=False,
                                    features=client.features)
 
-            try:
+        try:
-                volume_update = self._enable_replication_if_needed(volume)
+            volume_update = self._setup_volume(volume)
-            except Exception:
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                LOG.error('Error creating rbd image %(vol)s.',
+                          {'vol': vol_name})
                 self.RBDProxy().remove(client.ioctx, vol_name)
-                err_msg = (_('Failed to enable image replication'))
+
-                raise exception.ReplicationError(reason=err_msg,
-                                                 volume_id=volume.id)
         return volume_update
 
     def _flatten(self, pool, volume_name):
@@ -900,7 +928,7 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                                       order=order)
 
             try:
-                volume_update = self._enable_replication_if_needed(volume)
+                volume_update = self._setup_volume(volume)
             except Exception:
                 self.RBDProxy().remove(dest_client.ioctx, vol_name)
                 err_msg = (_('Failed to enable image replication'))
@@ -1160,6 +1188,17 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
 
     def retype(self, context, volume, new_type, diff, host):
         """Retype from one volume type to another on the same backend."""
+
+        # NOTE: There is no mechanism to store prior image features when
+        # creating a multiattach volume.  So retyping to non-multiattach
+        # would result in an RBD image that lacks several popular
+        # features (object-map, fast-diff, etc).  Without saving prior
+        # state as we do for replication, it is impossible to know which
+        # feautures to restore.
+        if self._is_multiattach_type(volume.volume_type):
+            msg = _('Retyping from multiattach is not supported.')
+            raise exception.RBDDriverException(reason=msg)
+
         old_vol_replicated = self._is_replicated_type(volume.volume_type)
         new_vol_replicated = self._is_replicated_type(new_type)
 
@@ -1517,7 +1556,7 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
         # We may need to re-enable replication because we have deleted the
         # original image and created a new one using the command line import.
         try:
-            self._enable_replication_if_needed(volume)
+            self._setup_volume(volume)
         except Exception:
             err_msg = (_('Failed to enable image replication'))
             raise exception.ReplicationError(reason=err_msg,

releasenotes/notes/rbd-multiattach-support-2900ce0245af0239.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - RBD driver has added multiattach support.  It should be noted that
+    replication and multiattach are mutually exclusive, so a single RBD
+    volume can only be configured to support one of these features at a
+    time.  Additionally, RBD image features are not preserved which
+    prevents a volume being retyped from multiattach to another type.
+    This limitation is temporary and will be addressed soon.