From 4bafdb9425e915e97d501c4f1d7211e02b1afc0e Mon Sep 17 00:00:00 2001 From: Jon Bernard Date: Thu, 23 Aug 2018 12:32:59 -0400 Subject: [PATCH] RBD: add support for multiattach Change-Id: Ie3945427b54544a3b411c23bffdad1acb5e508e1 --- cinder/exception.py | 5 ++ cinder/tests/unit/volume/drivers/test_rbd.py | 15 ++-- cinder/volume/drivers/rbd.py | 81 ++++++++++++++----- ...-multiattach-support-2900ce0245af0239.yaml | 8 ++ 4 files changed, 80 insertions(+), 29 deletions(-) create mode 100644 releasenotes/notes/rbd-multiattach-support-2900ce0245af0239.yaml diff --git a/cinder/exception.py b/cinder/exception.py index fbade5102e6..deeb8c747cb 100644 --- a/cinder/exception.py +++ b/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") diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 4a3acfe6b78..bc4ca68a6ad 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/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': ' 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, - mock_enable): + def test_setup_volume_without_replication(self, enabled, 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, diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index ba10a99fde5..80ad764ac99 100644 --- a/cinder/volume/drivers/rbd.py +++ b/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. - # 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, + 'multiattach': True, '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) == " True" - def _enable_replication_if_needed(self, volume): - if self._is_replicated_type(volume.volume_type): + def _is_multiattach_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_MULTIATTACH) == " 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} - return None + update = {'replication_status': + 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: - volume_update = self._enable_replication_if_needed(volume) - except Exception: + try: + volume_update = self._setup_volume(volume) + 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, diff --git a/releasenotes/notes/rbd-multiattach-support-2900ce0245af0239.yaml b/releasenotes/notes/rbd-multiattach-support-2900ce0245af0239.yaml new file mode 100644 index 00000000000..6620cadcbed --- /dev/null +++ b/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.