]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Handle the volume not found case in the VMAX driver
authorXing Yang <xing.yang@emc.com>
Fri, 19 Dec 2014 17:45:45 +0000 (12:45 -0500)
committerXing Yang <xing.yang@emc.com>
Mon, 22 Dec 2014 21:28:10 +0000 (16:28 -0500)
When a volume cannot be found on the array, VMAX driver throws
an exception. This brings a volume into error_deleting state during
the volume deletion operation. This patch fixed it by returning None
when a volume cannot be found.

Closes-Bug: #1395830
Change-Id: Iad714ff6e577bc9d7cf69b8654a7eb726969279e

cinder/tests/test_emc_vmax.py
cinder/volume/drivers/emc/emc_vmax_common.py
cinder/volume/drivers/emc/emc_vmax_utils.py

index abd7aa7af4897f843445bad5e745ecb2f3b7646c..b6c40e911cbb2ee839f3d7c1e65559e6286b1c56 100644 (file)
@@ -20,6 +20,7 @@ import time
 from xml.dom.minidom import Document
 
 import mock
+import six
 
 from cinder import exception
 from cinder.openstack.common import log as logging
@@ -1141,6 +1142,63 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         loopingcall.FixedIntervalLoopingCall.reset_mock()
         loopingcall.FixedIntervalLoopingCall = loopingcall_orig
 
+    # Bug 1395830: _find_lun throws exception when lun is not found.
+    def test_find_lun(self):
+        keybindings = {'CreationClassName': u'Symm_StorageVolume',
+                       'SystemName': u'SYMMETRIX+000195900551',
+                       'DeviceID': u'1',
+                       'SystemCreationClassName': u'Symm_StorageSystem'}
+        provider_location = {'classname': 'Symm_StorageVolume',
+                             'keybindings': keybindings}
+        volume = EMC_StorageVolume()
+        volume['name'] = 'vol1'
+        volume['provider_location'] = six.text_type(provider_location)
+
+        self.driver.common.conn = self.driver.common._get_ecom_connection()
+        findlun = self.driver.common._find_lun(volume)
+        getinstance = self.driver.common.conn._getinstance_storagevolume(
+            keybindings)
+        # Found lun.
+        self.assertEqual(getinstance, findlun)
+
+        keybindings2 = {'CreationClassName': u'Symm_StorageVolume',
+                        'SystemName': u'SYMMETRIX+000195900551',
+                        'DeviceID': u'9',
+                        'SystemCreationClassName': u'Symm_StorageSystem'}
+        provider_location2 = {'classname': 'Symm_StorageVolume',
+                              'keybindings': keybindings2}
+        volume2 = EMC_StorageVolume()
+        volume2['name'] = 'myVol'
+        volume2['provider_location'] = six.text_type(provider_location2)
+        verify_orig = self.driver.common.utils.get_existing_instance
+        self.driver.common.utils.get_existing_instance = mock.Mock(
+            return_value=None)
+        findlun2 = self.driver.common._find_lun(volume2)
+        # Not found.
+        self.assertIsNone(findlun2)
+        instancename2 = self.driver.utils.get_instance_name(
+            provider_location2['classname'],
+            keybindings2)
+        self.driver.common.utils.get_existing_instance.assert_called_once_with(
+            self.driver.common.conn, instancename2)
+        self.driver.common.utils.get_existing_instance.reset_mock()
+        self.driver.common.utils.get_existing_instance = verify_orig
+
+        keybindings3 = {'CreationClassName': u'Symm_StorageVolume',
+                        'SystemName': u'SYMMETRIX+000195900551',
+                        'DeviceID': u'9999',
+                        'SystemCreationClassName': u'Symm_StorageSystem'}
+        provider_location3 = {'classname': 'Symm_StorageVolume',
+                              'keybindings': keybindings3}
+        instancename3 = self.driver.utils.get_instance_name(
+            provider_location3['classname'],
+            keybindings3)
+        # Error other than not found.
+        arg = 9999, "test_error"
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.common.utils.process_exception_args,
+                          arg, instancename3)
+
     def test_get_volume_stats_1364232(self):
         self.create_fake_config_file_1364232()
         self.assertEqual('000198700439',
index 101894ee517c808774d52554e9a963f426ea8e07..628527f07ef05d190ec9eae196d672648cd77112 100644 (file)
@@ -1196,7 +1196,9 @@ class EMCVMAXCommon(object):
             instancename = self.utils.get_instance_name(
                 name['classname'], name['keybindings'])
 
-            foundVolumeinstance = self.conn.GetInstance(instancename)
+            # Handle the case where volume cannot be found.
+            foundVolumeinstance = self.utils.get_existing_instance(
+                self.conn, instancename)
 
         if foundVolumeinstance is None:
             LOG.debug("Volume %(volumename)s not found on the array."
index ad61314337264aa4cb13ef99ec6ff617b7b0e020..82fc6b07fb73e69ce123e82f03f274997a4a34e1 100644 (file)
@@ -46,6 +46,7 @@ ISCSI = 'iscsi'
 FC = 'fc'
 JOB_RETRIES = 60
 INTERVAL_10_SEC = 10
+CIM_ERR_NOT_FOUND = 6
 
 
 class EMCVMAXUtils(object):
@@ -1225,3 +1226,43 @@ class EMCVMAXUtils(object):
             volumeSizeInbits = numOfBlocks * blockSize
             capacitiesInBit.append(volumeSizeInbits)
         return capacitiesInBit
+
+    def get_existing_instance(self, conn, instanceName):
+        """Check that the instance name still exists and return the instance.
+
+        :param conn: the connection to the ecom server
+        :param instanceName: the instanceName to be checked
+        :returns: instance or None
+        """
+        instance = None
+        try:
+            instance = conn.GetInstance(instanceName, LocalOnly=False)
+        except pywbem.cim_operations.CIMError as arg:
+            instance = self.process_exception_args(arg, instanceName)
+
+        return instance
+
+    def process_exception_args(self, arg, instanceName):
+        """Process exception arguments.
+
+        :param arg: the arg list
+        :param instanceName: the instance name
+        :returns: None
+        :raises: VolumeBackendAPIException
+        """
+        instance = None
+        code, desc = arg[0], arg[1]
+        if code == CIM_ERR_NOT_FOUND:
+            # Object doesn't exist any more
+            instance = None
+        else:
+            # Something else that we cannot recover from has happened
+            LOG.error(_LE("Exception: %s"), six.text_type(desc))
+            exceptionMessage = (_(
+                "Cannot verify the existance of object:"
+                "%(instanceName)s.")
+                % {'instanceName': instanceName})
+            LOG.error(exceptionMessage)
+            raise exception.VolumeBackendAPIException(
+                data=exceptionMessage)
+        return instance