Correcting thin provisioning behavior

Currently, if thin provisioning is enabled and
max_over_subscription_ratio <= 1, the code will not go through the
thin-provisioning aware code, thereby ignoring any limits from
max_over_subscription_ratio. The intended behavior of setting
max_over_subscription_ratio = 1 is to disable over-provisioning.
Fixed help string to state minimum of 1.0 for
max_over_subscription_ratio.

Change-Id: I0ac265905e0613691238e7fcd60853c838118fcc
Closes-Bug: #1537162
Co-Authored-By: Siva Mullapudi <scmullap@us.ibm.com>

diff --git a/cinder/scheduler/filters/capacity_filter.py b/cinder/scheduler/filters/capacity_filter.py
index ed43e2eab28c4c21a49197f43f6728124774f82f..b0ba3597243f80acb41d9457c7668a2a96d28679 100644 (file)
--- a/cinder/scheduler/filters/capacity_filter.py
+++ b/cinder/scheduler/filters/capacity_filter.py
@@ -86,7 +86,7 @@ class CapacityFilter(filters.BaseHostFilter):
         # provisioned capacity over total capacity has exceeded over
         # subscription ratio.
         if (host_state.thin_provisioning_support and
-                host_state.max_over_subscription_ratio > 1):
+                host_state.max_over_subscription_ratio >= 1):
             provisioned_ratio = ((host_state.provisioned_capacity_gb +
                                   volume_size) / total)
             if provisioned_ratio > host_state.max_over_subscription_ratio:
@@ -110,6 +110,15 @@ class CapacityFilter(filters.BaseHostFilter):
                 adjusted_free_virtual = (
                     free * host_state.max_over_subscription_ratio)
                 return adjusted_free_virtual >= volume_size
+        elif host_state.thin_provisioning_support:
+            LOG.warning(_LW("Filtering out host %(host)s with an invalid "
+                            "maximum over subscription ratio of "
+                            "%(oversub_ratio).2f. The ratio should be a "
+                            "minimum of 1.0."),
+                        {"oversub_ratio":
+                            host_state.max_over_subscription_ratio,
+                         "host": host_state.host})
+            return False
 
         if free < volume_size:
             LOG.warning(_LW("Insufficient free space for volume creation "

diff --git a/cinder/tests/unit/scheduler/test_host_filters.py b/cinder/tests/unit/scheduler/test_host_filters.py
index f988c300e8981c66bdc629f5a657dd128d3e99f4..9effbc5241e24421c7b0d2f56b82349f824f9175 100644 (file)
--- a/cinder/tests/unit/scheduler/test_host_filters.py
+++ b/cinder/tests/unit/scheduler/test_host_filters.py
@@ -286,6 +286,28 @@ class CapacityFilterTestCase(HostFiltersTestCase):
                                     'service': service})
         self.assertFalse(filt_cls.host_passes(host, filter_properties))
 
+    @mock.patch('cinder.utils.service_is_up')
+    def test_filter_over_subscription_equal_to_1(self, _mock_serv_is_up):
+        _mock_serv_is_up.return_value = True
+        filt_cls = self.class_map['CapacityFilter']()
+        filter_properties = {'size': 150,
+                             'capabilities:thin_provisioning_support':
+                                 '<is> True',
+                             'capabilities:thick_provisioning_support':
+                                 '<is> False'}
+        service = {'disabled': False}
+        host = fakes.FakeHostState('host1',
+                                   {'total_capacity_gb': 500,
+                                    'free_capacity_gb': 200,
+                                    'provisioned_capacity_gb': 400,
+                                    'max_over_subscription_ratio': 1.0,
+                                    'reserved_percentage': 0,
+                                    'thin_provisioning_support': True,
+                                    'thick_provisioning_support': False,
+                                    'updated_at': None,
+                                    'service': service})
+        self.assertFalse(filt_cls.host_passes(host, filter_properties))
+
     @mock.patch('cinder.utils.service_is_up')
     def test_filter_over_subscription_fails(self, _mock_serv_is_up):
         _mock_serv_is_up.return_value = True

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index 4d1e2f9ee92a27f51df0a66b2b7a6bc4d76d4410..8eac6e29951e94ae20687ce8eb895b8fb1f6cf9b 100644 (file)
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -164,8 +164,7 @@ volume_opts = [
                       'means provisioned capacity can be 10.5 times of the '
                       'total physical capacity. A ratio of 1.0 means '
                       'provisioned capacity cannot exceed the total physical '
-                      'capacity. A ratio lower than 1.0 will be ignored and '
-                      'the default value will be used instead.'),
+                      'capacity. The ratio has to be a minimum of 1.0.'),
     cfg.StrOpt('scst_target_iqn_name',
                help='Certain ISCSI targets have predefined target names, '
                     'SCST target driver uses this name.'),