From: Patrick East <patrick.east@purestorage.com>
Date: Thu, 17 Dec 2015 00:59:51 +0000 (-0800)
Subject: Add config option to enable reporting discard
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=63e54b80d0b3103621e248122c48b8bbb167580a;p=openstack-build%2Fcinder-build.git

Add config option to enable reporting discard

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
---

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.