From e845c1182dde5aa8858290c2719b017fb5ca5915 Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Sun, 7 Dec 2014 14:57:27 -0500 Subject: [PATCH] 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 --- cinder/tests/test_emc_vmax.py | 57 +++++++++++++++++++++ cinder/volume/drivers/emc/emc_vmax_utils.py | 36 +++++++------ 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/cinder/tests/test_emc_vmax.py b/cinder/tests/test_emc_vmax.py index 5646ff1e9..abd7aa7af 100644 --- 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 a8d9022c3..ad6131433 100644 --- 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() -- 2.45.2