diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index bc4ca68a6ad..2dfb72939a9 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -347,14 +347,14 @@ class RBDTestCase(test.TestCase): self.context) @mock.patch.object(driver.RBDDriver, '_enable_replication', - return_value=mock.sentinel.volume_update) + return_value={'replication': 'enabled'}) 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._setup_volume(self.volume_a) - self.assertEqual(mock.sentinel.volume_update, res) + self.assertEqual('enabled', res['replication']) mock_enable.assert_called_once_with(self.volume_a) @ddt.data(False, True) @@ -365,7 +365,7 @@ class RBDTestCase(test.TestCase): if enabled: expect = {'replication_status': fields.ReplicationStatus.DISABLED} else: - expect = None + expect = {} self.assertEqual(expect, res) mock_enable.assert_not_called() @@ -458,7 +458,7 @@ class RBDTestCase(test.TestCase): res = self.driver.create_volume(self.volume_a) - self.assertIsNone(res) + self.assertEqual({}, res) chunk_size = self.cfg.rbd_store_chunk_size * units.Mi order = int(math.log(chunk_size, 2)) args = [client.ioctx, str(self.volume_a.name), @@ -1038,7 +1038,7 @@ class RBDTestCase(test.TestCase): res = self.driver.create_cloned_volume(self.volume_b, self.volume_a) - self.assertIsNone(res) + self.assertEqual({}, res) (self.mock_rbd.Image.return_value.create_snap .assert_called_once_with('.'.join( (self.volume_b.name, 'clone_snap')))) @@ -1104,7 +1104,7 @@ class RBDTestCase(test.TestCase): res = self.driver.create_cloned_volume(self.volume_b, self.volume_a) - self.assertIsNone(res) + self.assertEqual({}, res) (self.mock_rbd.Image.return_value.create_snap .assert_called_once_with('.'.join( (self.volume_b.name, 'clone_snap')))) @@ -1153,7 +1153,7 @@ class RBDTestCase(test.TestCase): res = self.driver.create_cloned_volume(self.volume_b, self.volume_a) - self.assertIsNone(res) + self.assertEqual({}, res) (self.mock_rbd.Image.return_value.create_snap .assert_called_once_with('.'.join( (self.volume_b.name, 'clone_snap')))) @@ -1706,7 +1706,7 @@ class RBDTestCase(test.TestCase): if enabled: expect = {'replication_status': fields.ReplicationStatus.DISABLED} else: - expect = None + expect = {} context = {} diff = {'encryption': {}, 'extra_specs': {}} @@ -1750,9 +1750,9 @@ class RBDTestCase(test.TestCase): @ddt.unpack @common_mocks @mock.patch.object(driver.RBDDriver, '_disable_replication', - return_value=mock.sentinel.disable_replication) + return_value={'replication': 'disabled'}) @mock.patch.object(driver.RBDDriver, '_enable_replication', - return_value=mock.sentinel.enable_replication) + return_value={'replication': 'enabled'}) def test_retype_replicated(self, mock_disable, mock_enable, old_replicated, new_replicated): """Test retyping a non replicated volume. @@ -1771,15 +1771,15 @@ class RBDTestCase(test.TestCase): if new_replicated: new_type = replicated_type if old_replicated: - update = None + update = {} else: - update = mock.sentinel.enable_replication + update = {'replication': 'enabled'} else: new_type = fake_volume.fake_volume_type_obj( self.context, id=fake.VOLUME_TYPE2_ID), if old_replicated: - update = mock.sentinel.disable_replication + update = {'replication': 'disabled'} else: update = {'replication_status': fields.ReplicationStatus.DISABLED} diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index 616ccfee02f..6ec4bb1f0d3 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -772,11 +772,37 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, ] vol_name = utils.convert_str(volume.name) + image_features = None with RBDVolumeProxy(self, vol_name) as image: + image_features = image.features() for feature in multipath_feature_exclusions: - if image.features() & feature: + if image_features & feature: image.update_features(feature, False) + return {'provider_location': + self._dumps({'saved_features': image_features})} + + def _disable_multiattach(self, volume): + multipath_feature_exclusions = [ + self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, + self.rbd.RBD_FEATURE_OBJECT_MAP, + self.rbd.RBD_FEATURE_FAST_DIFF, + self.rbd.RBD_FEATURE_JOURNALING, + ] + vol_name = utils.convert_str(volume.name) + with RBDVolumeProxy(self, vol_name) as image: + try: + provider_location = json.loads(volume.provider_location) + image_features = provider_location['saved_features'] + except Exception: + msg = _('Could not find saved image features.') + raise RBDDriverException(reason=msg) + for feature in multipath_feature_exclusions: + if image_features & feature: + image.update_features(feature, True) + + return {'provider_location': None} + 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', {}) @@ -787,27 +813,52 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, 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) + def _setup_volume(self, volume, volume_type=None): + + if volume_type: + had_replication = self._is_replicated_type(volume.volume_type) + had_multiattach = self._is_multiattach_type(volume.volume_type) + else: + had_replication = False + had_multiattach = False + volume_type = volume.volume_type + + want_replication = self._is_replicated_type(volume_type) + want_multiattach = self._is_multiattach_type(volume_type) if want_replication and want_multiattach: msg = _('Replication and Multiattach are mutually exclusive.') raise RBDDriverException(reason=msg) + volume_update = dict() + if want_replication: - return self._enable_replication(volume) - - update = None - - if self._is_replication_enabled: - update = {'replication_status': - fields.ReplicationStatus.DISABLED} + if had_multiattach: + volume_update.update(self._disable_multiattach(volume)) + if not had_replication: + try: + volume_update.update(self._enable_replication(volume)) + except Exception: + err_msg = (_('Failed to enable image replication')) + raise exception.ReplicationError(reason=err_msg, + volume_id=volume.id) + elif had_replication: + try: + volume_update.update(self._disable_replication(volume)) + except Exception: + err_msg = (_('Failed to disable image replication')) + raise exception.ReplicationError(reason=err_msg, + volume_id=volume.id) + elif self._is_replication_enabled: + volume_update.update({'replication_status': + fields.ReplicationStatus.DISABLED}) if want_multiattach: - self._enable_multiattach(volume) + volume_update.update(self._enable_multiattach(volume)) + elif had_multiattach: + volume_update.update(self._disable_multiattach(volume)) - return update + return volume_update def _check_encryption_provider(self, volume, context): """Check that this is a LUKS encryption provider. @@ -1197,40 +1248,7 @@ 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 RBDDriverException(reason=msg) - - old_vol_replicated = self._is_replicated_type(volume.volume_type) - new_vol_replicated = self._is_replicated_type(new_type) - - if old_vol_replicated and not new_vol_replicated: - try: - return True, self._disable_replication(volume) - except Exception: - err_msg = (_('Failed to disable image replication')) - raise exception.ReplicationError(reason=err_msg, - volume_id=volume.id) - elif not old_vol_replicated and new_vol_replicated: - try: - return True, self._enable_replication(volume) - except Exception: - err_msg = (_('Failed to enable image replication')) - raise exception.ReplicationError(reason=err_msg, - volume_id=volume.id) - - if not new_vol_replicated and self._is_replication_enabled: - update = {'replication_status': fields.ReplicationStatus.DISABLED} - else: - update = None - return True, update + return True, self._setup_volume(volume, new_type) def _dumps(self, obj): return json.dumps(obj, separators=(',', ':'), sort_keys=True)