From 63e54b80d0b3103621e248122c48b8bbb167580a Mon Sep 17 00:00:00 2001 From: Patrick East Date: Wed, 16 Dec 2015 16:59:51 -0800 Subject: [PATCH] Add config option to enable reporting discard MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The way for a client of Cinder (read: Nova) to know whether or not it can and should use the discard feature for the block device attachment is to report back discard=True in the connection info. Previously a driver would need to be updated to provide this functionality, which is kind of a pain with so many drivers in Cinder. This change adds a shared boolean config option called ‘report_discard_supported’ which can be set to enable reporting this functionality to the caller of initialize_connection. DocImpact Change-Id: I078e74583621316bcfe138096507e7d22af0d712 Implements: blueprint discard-config-option --- cinder/tests/unit/test_volume.py | 72 +++++++++++++++++++ cinder/volume/driver.py | 6 ++ cinder/volume/manager.py | 8 +++ ...iscard-config-option-711a7fbf20685834.yaml | 3 + 4 files changed, 89 insertions(+) create mode 100644 releasenotes/notes/discard-config-option-711a7fbf20685834.yaml diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 92b593f2c..6afc30221 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -7668,3 +7668,75 @@ class ImageVolumeCacheTestCase(BaseVolumeTestCase): entry = db.image_volume_cache_get_by_volume_id(self.context, volume['id']) self.assertIsNone(entry) + + +@ddt.ddt +class DiscardFlagTestCase(BaseVolumeTestCase): + + def setUp(self): + super(DiscardFlagTestCase, self).setUp() + self.volume.driver = mock.MagicMock() + self.mock_db = mock.MagicMock() + self.volume.db = self.mock_db + + @ddt.data(dict(config_discard_flag=True, + driver_discard_flag=None, + expected_flag=True), + dict(config_discard_flag=False, + driver_discard_flag=None, + expected_flag=None), + dict(config_discard_flag=True, + driver_discard_flag=True, + expected_flag=True), + dict(config_discard_flag=False, + driver_discard_flag=True, + expected_flag=True), + dict(config_discard_flag=False, + driver_discard_flag=False, + expected_flag=False), + dict(config_discard_flag=None, + driver_discard_flag=True, + expected_flag=True), + dict(config_discard_flag=None, + driver_discard_flag=False, + expected_flag=False)) + @ddt.unpack + def test_initialize_connection_discard_flag(self, + config_discard_flag, + driver_discard_flag, + expected_flag): + volume_properties = {'volume_type_id': None} + + def _get_item(key): + return volume_properties[key] + + mock_volume = mock.MagicMock() + mock_volume.__getitem__.side_effect = _get_item + self.mock_db.volume_get.return_value = mock_volume + self.mock_db.volume_update.return_value = mock_volume + self.volume.driver.create_export.return_value = None + connector = {'ip': 'IP', 'initiator': 'INITIATOR'} + + conn_info = { + 'driver_volume_type': 'iscsi', + 'data': {'access_mode': 'rw', + 'encrypted': False} + } + + if driver_discard_flag is not None: + conn_info['data']['discard'] = driver_discard_flag + + self.volume.driver.initialize_connection.return_value = conn_info + + def _safe_get(key): + if key is 'report_discard_supported': + return config_discard_flag + else: + return None + + self.volume.driver.configuration.safe_get.side_effect = _safe_get + + conn_info = self.volume.initialize_connection(self.context, 'id', + connector) + + self.assertEqual(expected_flag, conn_info['data'].get('discard')) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index ad4309913..a60feb2cd 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -243,6 +243,12 @@ volume_opts = [ default=0, help='Max number of entries allowed in the image volume cache. ' '0 => unlimited.'), + cfg.BoolOpt('report_discard_supported', + default=False, + help='Report to clients of Cinder that the backend supports ' + 'discard (aka. trim/unmap). This will not actually ' + 'change the behavior of the backend or the client ' + 'directly, it will only notify that it can be used.'), ] # for backward compatibility diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 180d20743..4ca1a358e 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1407,6 +1407,14 @@ class VolumeManager(manager.SchedulerDependentManager): encrypted = bool(volume.get('encryption_key_id')) conn_info['data']['encrypted'] = encrypted + # Add discard flag to connection_info if not set in the driver and + # configured to be reported. + if conn_info['data'].get('discard') is None: + discard_supported = (self.driver.configuration + .safe_get('report_discard_supported')) + if discard_supported: + conn_info['data']['discard'] = True + LOG.info(_LI("Initialize volume connection completed successfully."), resource=volume) return conn_info diff --git a/releasenotes/notes/discard-config-option-711a7fbf20685834.yaml b/releasenotes/notes/discard-config-option-711a7fbf20685834.yaml new file mode 100644 index 000000000..51d7493cc --- /dev/null +++ b/releasenotes/notes/discard-config-option-711a7fbf20685834.yaml @@ -0,0 +1,3 @@ +--- +features: + - New config option to enable discard (trim/unmap) support for any backend. -- 2.45.2