Fixed wait for job completion in VMAX driver

The VMAX driver used non-thread-safe variables in the wait for job
completion routines, resulting in failures or timeouts during concurrent
operations. This patch fixed the problem.

Closes-Bug: #1393568
Change-Id: I444b961a6543886da35fb44127b1cf7c509ce8d8

diff --git a/cinder/tests/test_emc_vmax.py b/cinder/tests/test_emc_vmax.py
index 5646ff1e9261927a4b2d6757b780716267bb84c8..abd7aa7af4897f843445bad5e745ecb2f3b7646c 100644 (file)
--- a/cinder/tests/test_emc_vmax.py
+++ b/cinder/tests/test_emc_vmax.py
@@ -23,6 +23,7 @@ import mock
 
 from cinder import exception
 from cinder.openstack.common import log as logging
+from cinder.openstack.common import loopingcall
 from cinder import test
 from cinder.volume.drivers.emc.emc_vmax_common import EMCVMAXCommon
 from cinder.volume.drivers.emc.emc_vmax_fast import EMCVMAXFast
@@ -1084,6 +1085,62 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
     def fake_sleep(self, seconds):
         return
 
+    def test_wait_for_job_complete(self):
+        myjob = SE_ConcreteJob()
+        myjob.classname = 'SE_ConcreteJob'
+        myjob['InstanceID'] = '9999'
+        myjob['status'] = 'success'
+        myjob['type'] = 'type'
+        myjob['CreationClassName'] = 'SE_ConcreteJob'
+        myjob['Job'] = myjob
+        conn = self.fake_ecom_connection()
+
+        self.driver.utils._is_job_finished = mock.Mock(
+            return_value = True)
+        rc = self.driver.utils._wait_for_job_complete(conn, myjob)
+        self.assertIsNone(rc)
+        self.driver.utils._is_job_finished.assert_called_once_with(
+            conn, myjob)
+        self.assertEqual(
+            True,
+            self.driver.utils._is_job_finished.return_value)
+        self.driver.utils._is_job_finished.reset_mock()
+
+        # Save the original state and restore it after this test
+        loopingcall_orig = loopingcall.FixedIntervalLoopingCall
+        loopingcall.FixedIntervalLoopingCall = mock.Mock()
+        rc = self.driver.utils._wait_for_job_complete(conn, myjob)
+        self.assertIsNone(rc)
+        loopingcall.FixedIntervalLoopingCall.assert_called_once_with(
+            mock.ANY)
+        loopingcall.FixedIntervalLoopingCall.reset_mock()
+        loopingcall.FixedIntervalLoopingCall = loopingcall_orig
+
+    def test_wait_for_sync(self):
+        mysync = 'fakesync'
+        conn = self.fake_ecom_connection()
+
+        self.driver.utils._is_sync_complete = mock.Mock(
+            return_value = True)
+        rc = self.driver.utils.wait_for_sync(conn, mysync)
+        self.assertIsNone(rc)
+        self.driver.utils._is_sync_complete.assert_called_once_with(
+            conn, mysync)
+        self.assertEqual(
+            True,
+            self.driver.utils._is_sync_complete.return_value)
+        self.driver.utils._is_sync_complete.reset_mock()
+
+        # Save the original state and restore it after this test
+        loopingcall_orig = loopingcall.FixedIntervalLoopingCall
+        loopingcall.FixedIntervalLoopingCall = mock.Mock()
+        rc = self.driver.utils.wait_for_sync(conn, mysync)
+        self.assertIsNone(rc)
+        loopingcall.FixedIntervalLoopingCall.assert_called_once_with(
+            mock.ANY)
+        loopingcall.FixedIntervalLoopingCall.reset_mock()
+        loopingcall.FixedIntervalLoopingCall = loopingcall_orig
+
     def test_get_volume_stats_1364232(self):
         self.create_fake_config_file_1364232()
         self.assertEqual('000198700439',

diff --git a/cinder/volume/drivers/emc/emc_vmax_utils.py b/cinder/volume/drivers/emc/emc_vmax_utils.py
index a8d9022c34454b9a611bae173efd24f74f8aff86..ad61314337264aa4cb13ef99ec6ff617b7b0e020 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_utils.py
+++ b/cinder/volume/drivers/emc/emc_vmax_utils.py
@@ -287,27 +287,30 @@ class EMCVMAXUtils(object):
 
         def _wait_for_job_complete():
             """Called at an interval until the job is finished"""
+            retries = kwargs['retries']
+            wait_for_job_called = kwargs['wait_for_job_called']
             if self._is_job_finished(conn, job):
                 raise loopingcall.LoopingCallDone()
-            if self.retries > JOB_RETRIES:
+            if retries > JOB_RETRIES:
                 LOG.error(_LE("_wait_for_job_complete "
                               "failed after %(retries)d "
-                              "tries") % {'retries': self.retries})
+                              "tries."),
+                          {'retries': retries})
 
                 raise loopingcall.LoopingCallDone()
             try:
-                self.retries += 1
-                if not self.wait_for_job_called:
+                kwargs['retries'] = retries + 1
+                if not wait_for_job_called:
                     if self._is_job_finished(conn, job):
-                        self.wait_for_job_called = True
+                        kwargs['wait_for_job_called'] = True
             except Exception as e:
                 LOG.error(_LE("Exception: %s") % six.text_type(e))
                 exceptionMessage = (_("Issue encountered waiting for job."))
                 LOG.error(exceptionMessage)
                 raise exception.VolumeBackendAPIException(exceptionMessage)
 
-        self.retries = 0
-        self.wait_for_job_called = False
+        kwargs = {'retries': 0,
+                  'wait_for_job_called': False}
         timer = loopingcall.FixedIntervalLoopingCall(_wait_for_job_complete)
         timer.start(interval=INTERVAL_10_SEC).wait()
 
@@ -347,17 +350,20 @@ class EMCVMAXUtils(object):
 
         def _wait_for_sync():
             """Called at an interval until the synchronization is finished."""
+            retries = kwargs['retries']
+            wait_for_sync_called = kwargs['wait_for_sync_called']
             if self._is_sync_complete(conn, syncName):
                 raise loopingcall.LoopingCallDone()
-            if self.retries > JOB_RETRIES:
-                LOG.error(_LE("_wait_for_sync failed after %(retries)d tries.")
-                          % {'retries': self.retries})
+            if retries > JOB_RETRIES:
+                LOG.error(_LE("_wait_for_sync failed after %(retries)d "
+                              "tries."),
+                          {'retries': retries})
                 raise loopingcall.LoopingCallDone()
             try:
-                self.retries += 1
-                if not self.wait_for_sync_called:
+                kwargs['retries'] = retries + 1
+                if not wait_for_sync_called:
                     if self._is_sync_complete(conn, syncName):
-                        self.wait_for_sync_called = True
+                        kwargs['wait_for_sync_called'] = True
             except Exception as e:
                 LOG.error(_LE("Exception: %s") % six.text_type(e))
                 exceptionMessage = (_("Issue encountered waiting for "
@@ -365,8 +371,8 @@ class EMCVMAXUtils(object):
                 LOG.error(exceptionMessage)
                 raise exception.VolumeBackendAPIException(exceptionMessage)
 
-        self.retries = 0
-        self.wait_for_sync_called = False
+        kwargs = {'retries': 0,
+                  'wait_for_sync_called': False}
         timer = loopingcall.FixedIntervalLoopingCall(_wait_for_sync)
         timer.start(interval=INTERVAL_10_SEC).wait()