openstack/cinder
Code Issues Proposed changes

Fixed wait for job completion in VMAX driver

Browse Source 
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
This commit is contained in:
Xing Yang
2014-12-07 14:57:27 -05:00
parent d1b89778f9
commit e845c1182d
2 changed files with 78 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
cinder/tests/test_emc_vmax.py
View File

@@ -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',

36
cinder/volume/drivers/emc/emc_vmax_utils.py
View File

@@ -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()