From 37c42c97e22b891e7c2d99d4452aa5b5e9e6d6a7 Mon Sep 17 00:00:00 2001
From: Takashi NATSUME <natsume.takashi@lab.ntt.co.jp>
Date: Thu, 28 Mar 2019 11:13:38 +0900
Subject: [PATCH] Add minimum value in max_concurrent_live_migrations

Add minimum value 0 in the max_concurrent_live_migrations option.

Change-Id: I52ead0154e311a0ebdfd6d7413704fa350020587
---
 nova/compute/manager.py                       |  8 ++------
 nova/conf/compute.py                          |  3 +--
 nova/tests/unit/compute/test_compute_mgr.py   | 19 ++++---------------
 ...-force-minimum-value-c0455a1b97d54bf1.yaml |  5 +++++
 4 files changed, 12 insertions(+), 23 deletions(-)
 create mode 100644 releasenotes/notes/max_concurrent_live_migrations-option-force-minimum-value-c0455a1b97d54bf1.yaml

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index b7fa83960aab..d68ab7308d0e 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -518,15 +518,11 @@ class ComputeManager(manager.Manager):
                 CONF.max_concurrent_builds)
         else:
             self._build_semaphore = compute_utils.UnlimitedSemaphore()
-        if max(CONF.max_concurrent_live_migrations, 0) != 0:
+        if CONF.max_concurrent_live_migrations > 0:
             self._live_migration_executor = futurist.GreenThreadPoolExecutor(
                 max_workers=CONF.max_concurrent_live_migrations)
         else:
-            if CONF.max_concurrent_live_migrations < 0:
-                LOG.warning('The value of the max_concurrent_live_migrations '
-                            'config option is less than 0. '
-                            'It is treated as 0 and will raise ValueError '
-                            'in a future release.')
+            # CONF.max_concurrent_live_migrations is 0 (unlimited)
             self._live_migration_executor = futurist.GreenThreadPoolExecutor()
         # This is a dict, keyed by instance uuid, to a two-item tuple of
         # migration object and Future for the queued live migration.
diff --git a/nova/conf/compute.py b/nova/conf/compute.py
index 95bc65294af6..cf83090fdd16 100644
--- a/nova/conf/compute.py
+++ b/nova/conf/compute.py
@@ -621,9 +621,9 @@ Possible Values:
 * 0 : treated as unlimited.
 * Any positive integer representing maximum concurrent builds.
 """),
-    # TODO(sfinucan): Add min parameter
     cfg.IntOpt('max_concurrent_live_migrations',
         default=1,
+        min=0,
         help="""
 Maximum number of live migrations to run concurrently. This limit is enforced
 to avoid outbound live migrations overwhelming the host/network and causing
@@ -633,7 +633,6 @@ that doing so is safe and stable in your environment.
 Possible values:
 
 * 0 : treated as unlimited.
-* Negative value defaults to 0.
 * Any positive integer representing maximum number of live migrations
   to run concurrently.
 """),
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index 462621750f32..0808d78c73d7 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -7437,22 +7437,11 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
         manager.ComputeManager()
         mock_executor.assert_called_once_with(max_workers=123)
 
-    @ddt.data(0, -2)
-    def test_max_concurrent_live_semaphore_unlimited(self, max_concurrent):
-        self.flags(max_concurrent_live_migrations=max_concurrent)
-        with test.nested(
-            mock.patch('futurist.GreenThreadPoolExecutor'),
-            mock.patch('nova.compute.manager.LOG'),
-        ) as (mock_executor, mock_log):
-            manager.ComputeManager()
+    @mock.patch('futurist.GreenThreadPoolExecutor')
+    def test_max_concurrent_live_semaphore_unlimited(self, mock_executor):
+        self.flags(max_concurrent_live_migrations=0)
+        manager.ComputeManager()
         mock_executor.assert_called_once_with()
-        if max_concurrent < 0:
-            mock_log.warning.assert_called_once_with(
-                'The value of the max_concurrent_live_migrations config '
-                'option is less than 0. It is treated as 0 and will raise '
-                'ValueError in a future release.')
-        else:
-            mock_log.warning.assert_not_called()
 
     def test_pre_live_migration_cinder_v3_api(self):
         # This tests that pre_live_migration with a bdm with an
diff --git a/releasenotes/notes/max_concurrent_live_migrations-option-force-minimum-value-c0455a1b97d54bf1.yaml b/releasenotes/notes/max_concurrent_live_migrations-option-force-minimum-value-c0455a1b97d54bf1.yaml
new file mode 100644
index 000000000000..d16588350b93
--- /dev/null
+++ b/releasenotes/notes/max_concurrent_live_migrations-option-force-minimum-value-c0455a1b97d54bf1.yaml
@@ -0,0 +1,5 @@
+---
+upgrade:
+  - The ``max_concurrent_live_migrations`` configuration option has been
+    restricted by the minimum value and now raises a ValueError
+    if the value is less than 0.