Handle config file with newlines and whitespaces

EMC VMAX driver does not handle newlines and whitespaces
consistently in the XML config file. This patch addressed
this issue.

Change-Id: I45f61f167feee9174ba2bc3229695b680f8e946d
Closes-Bug: #1364232

diff --git a/cinder/tests/test_emc_vmax.py b/cinder/tests/test_emc_vmax.py
index 536d5a874eeeee750d0c4bc6b7f9980329a96b09..eee0025ee1e91cffa40e94082d3b7cce5c3a4198 100644 (file)
--- a/cinder/tests/test_emc_vmax.py
+++ b/cinder/tests/test_emc_vmax.py
@@ -925,7 +925,9 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         self.tempdir = tempfile.mkdtemp()
         super(EMCVMAXISCSIDriverNoFastTestCase, self).setUp()
         self.config_file_path = None
+        self.config_file_1364232 = None
         self.create_fake_config_file_no_fast()
+        self.addCleanup(self._cleanup)
 
         configuration = mock.Mock()
         configuration.safe_get.return_value = 'ISCSINoFAST'
@@ -945,6 +947,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         driver = EMCVMAXISCSIDriver(configuration=configuration)
         driver.db = FakeDB()
         self.driver = driver
+        self.driver.utils = EMCVMAXUtils(object)
 
     def create_fake_config_file_no_fast(self):
 
@@ -1008,6 +1011,28 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         doc.writexml(f)
         f.close()
 
+    # Create XML config file with newlines and whitespaces
+    # Bug #1364232
+    def create_fake_config_file_1364232(self):
+        filename = 'cinder_emc_config_1364232.xml'
+        self.config_file_1364232 = self.tempdir + '/' + filename
+        text_file = open(self.config_file_1364232, "w")
+        text_file.write("<?xml version='1.0' encoding='UTF-8'?>\n<EMC>\n"
+                        "<EcomServerIp>10.108.246.202</EcomServerIp>\n"
+                        "<EcomServerPort>5988</EcomServerPort>\n"
+                        "<EcomUserName>admin\t</EcomUserName>\n"
+                        "<EcomPassword>#1Password</EcomPassword>\n"
+                        "<PortGroups><PortGroup>OS-PORTGROUP1-PG"
+                        "</PortGroup><PortGroup>OS-PORTGROUP2-PG"
+                        "                </PortGroup>\n"
+                        "<PortGroup>OS-PORTGROUP3-PG</PortGroup>"
+                        "<PortGroup>OS-PORTGROUP4-PG</PortGroup>"
+                        "</PortGroups>\n<Array>000198700439"
+                        "              \n</Array>\n<Pool>FC_SLVR1\n"
+                        "</Pool>\n<FastPolicy>SILVER1</FastPolicy>\n"
+                        "</EMC>")
+        text_file.close()
+
     def fake_ecom_connection(self):
         conn = FakeEcomConnection()
         return conn
@@ -1021,6 +1046,24 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
     def fake_sleep(self, seconds):
         return
 
+    def test_get_volume_stats_1364232(self):
+        self.create_fake_config_file_1364232()
+        self.assertEqual('000198700439',
+                         self.driver.utils.parse_array_name_from_file(
+                             self.config_file_1364232))
+        self.assertEqual('FC_SLVR1',
+                         self.driver.utils.parse_pool_name_from_file(
+                             self.config_file_1364232))
+        self.assertEqual('SILVER1',
+                         self.driver.utils.parse_fast_policy_name_from_file(
+                             self.config_file_1364232))
+        self.assertIn('OS-PORTGROUP',
+                      self.driver.utils.parse_file_to_get_port_group_name(
+                          self.config_file_1364232))
+        bExists = os.path.exists(self.config_file_1364232)
+        if bExists:
+            os.remove(self.config_file_1364232)
+
     @mock.patch.object(
         EMCVMAXCommon,
         '_find_storageSystem',
@@ -1325,10 +1368,6 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
             os.remove(self.config_file_path)
         shutil.rmtree(self.tempdir)
 
-    def tearDown(self):
-        self._cleanup()
-        super(EMCVMAXISCSIDriverNoFastTestCase, self).tearDown()
-
 
 class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
 
@@ -1340,6 +1379,7 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
         super(EMCVMAXISCSIDriverFastTestCase, self).setUp()
         self.config_file_path = None
         self.create_fake_config_file_fast()
+        self.addCleanup(self._cleanup)
 
         configuration = mock.Mock()
         configuration.cinder_emc_config_file = self.config_file_path
@@ -1853,10 +1893,6 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
             os.remove(self.config_file_path)
         shutil.rmtree(self.tempdir)
 
-    def tearDown(self):
-        self._cleanup()
-        super(EMCVMAXISCSIDriverFastTestCase, self).tearDown()
-
 
 class EMCVMAXFCDriverNoFastTestCase(test.TestCase):
     def setUp(self):
@@ -1867,6 +1903,7 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase):
         super(EMCVMAXFCDriverNoFastTestCase, self).setUp()
         self.config_file_path = None
         self.create_fake_config_file_no_fast()
+        self.addCleanup(self._cleanup)
 
         configuration = mock.Mock()
         configuration.cinder_emc_config_file = self.config_file_path
@@ -2259,10 +2296,6 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase):
             os.remove(self.config_file_path)
         shutil.rmtree(self.tempdir)
 
-    def tearDown(self):
-        self._cleanup()
-        super(EMCVMAXFCDriverNoFastTestCase, self).tearDown()
-
 
 class EMCVMAXFCDriverFastTestCase(test.TestCase):
 
@@ -2274,6 +2307,7 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
         super(EMCVMAXFCDriverFastTestCase, self).setUp()
         self.config_file_path = None
         self.create_fake_config_file_fast()
+        self.addCleanup(self._cleanup)
 
         configuration = mock.Mock()
         configuration.cinder_emc_config_file = self.config_file_path
@@ -2777,7 +2811,3 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
         if bExists:
             os.remove(self.config_file_path)
         shutil.rmtree(self.tempdir)
-
-    def tearDown(self):
-        self._cleanup()
-        super(EMCVMAXFCDriverFastTestCase, self).tearDown()

diff --git a/cinder/volume/drivers/emc/emc_vmax_common.py b/cinder/volume/drivers/emc/emc_vmax_common.py
index b7d7cc99d1999330a61153439212e8c99bb3ecd9..08553305e947e9e4bd9b6e608ee7baf308e389ec 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_common.py
+++ b/cinder/volume/drivers/emc/emc_vmax_common.py
@@ -652,7 +652,7 @@ class EMCVMAXCommon(object):
                 'QoS_support': False,
                 'volume_backend_name': backendName or self.__class__.__name__,
                 'vendor_name': "EMC",
-                'driver_version': '1.0',
+                'driver_version': '2.0',
                 'storage_protocol': 'unknown',
                 'location_info': location_info}
 

diff --git a/cinder/volume/drivers/emc/emc_vmax_utils.py b/cinder/volume/drivers/emc/emc_vmax_utils.py
index 005213afdd338533f833505133ea50868ec26cd8..ab7e1c4101d6b02736c52ecd8a3fb9685749f95d 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_utils.py
+++ b/cinder/volume/drivers/emc/emc_vmax_utils.py
@@ -480,7 +480,6 @@ class EMCVMAXUtils(object):
     def find_storage_system_name_from_service(self, configService):
         """Given any service get the storage system name from it.
 
-        :param conn: connection to the ecom server
         :param configService: the configuration service
         :returns: configService['SystemName'] - storage system name (String)
         """
@@ -535,7 +534,7 @@ class EMCVMAXUtils(object):
         NOTE:  This exists in common too...will be moving it to other file
         where both common and masking can access it
 
-        :param classname: connection to the ecom server
+        :param classname: class name for the volume instance
         :param bindings: volume created from job
         :returns: foundVolumeInstance - the volume instance
 
@@ -558,18 +557,9 @@ class EMCVMAXUtils(object):
         :returns: ecomIp - the ecom IP address
         :returns: ecomPort - the ecom port
         """
-        myFile = open(filename, 'r')
-        data = myFile.read()
-        myFile.close()
-        dom = parseString(data)
-        ecomIps = dom.getElementsByTagName('EcomServerIp')
-        if ecomIps is not None and len(ecomIps) > 0:
-            ecomIp = ecomIps[0].toxml().replace('<EcomServerIp>', '')
-            ecomIp = ecomIp.replace('</EcomServerIp>', '')
-        ecomPorts = dom.getElementsByTagName('EcomServerPort')
-        if ecomPorts is not None and len(ecomPorts) > 0:
-            ecomPort = ecomPorts[0].toxml().replace('<EcomServerPort>', '')
-            ecomPort = ecomPort.replace('</EcomServerPort>', '')
+
+        ecomIp = self._parse_from_file(filename, 'EcomServerIp')
+        ecomPort = self._parse_from_file(filename, 'EcomServerPort')
         if ecomIp is not None and ecomPort is not None:
             LOG.debug("Ecom IP: %(ecomIp)s Port: %(ecomPort)s",
                       {'ecomIp': ecomIp, 'ecomPort': ecomPort})
@@ -585,18 +575,9 @@ class EMCVMAXUtils(object):
         :returns: ecomUser - the ecom user
         :returns: ecomPasswd - the ecom password
         """
-        myFile = open(filename, 'r')
-        data = myFile.read()
-        myFile.close()
-        dom = parseString(data)
-        ecomUsers = dom.getElementsByTagName('EcomUserName')
-        if ecomUsers is not None and len(ecomUsers) > 0:
-            ecomUser = ecomUsers[0].toxml().replace('<EcomUserName>', '')
-            ecomUser = ecomUser.replace('</EcomUserName>', '')
-        ecomPasswds = dom.getElementsByTagName('EcomPassword')
-        if ecomPasswds is not None and len(ecomPasswds) > 0:
-            ecomPasswd = ecomPasswds[0].toxml().replace('<EcomPassword>', '')
-            ecomPasswd = ecomPasswd.replace('</EcomPassword>', '')
+
+        ecomUser = self._parse_from_file(filename, 'EcomUserName')
+        ecomPasswd = self._parse_from_file(filename, 'EcomPassword')
         if ecomUser is not None and ecomPasswd is not None:
             return ecomUser, ecomPasswd
         else:
@@ -622,11 +603,14 @@ class EMCVMAXUtils(object):
             portGroupsXml = portGroups[0].toxml()
             portGroupsXml = portGroupsXml.replace('<PortGroups>', '')
             portGroupsXml = portGroupsXml.replace('</PortGroups>', '')
-            portGroupsXml = portGroupsXml.replace('<PortGroup>', '')
+            portGroupsXml = portGroupsXml.replace('<PortGroup>', '|')
             portGroupsXml = portGroupsXml.replace('</PortGroup>', '')
-            # convert the newline separated string to a list
+            portGroupsXml = portGroupsXml.replace('\n', '')
+            portGroupsXml = portGroupsXml.replace('\t', '')
+            portGroupsXml = portGroupsXml[1:]
+            # convert the | separated string to a list
             portGroupNames = (
-                [s.strip() for s in portGroupsXml.split('\n') if s])
+                [s.strip() for s in portGroupsXml.split('|') if s])
 
             numPortGroups = len(portGroupNames)
 
@@ -639,30 +623,46 @@ class EMCVMAXUtils(object):
             LOG.error(exception_message)
             raise exception.VolumeBackendAPIException(data=exception_message)
 
-    def parse_fast_policy_name_from_file(self, fileName):
-        """Parse the fast policy name from config file.  If it is not there
-        then NON FAST is assumed
+    def _parse_from_file(self, fileName, stringToParse):
+        """Parse the string from XML.
+
+        Remove newlines, tabs, and trailing spaces.
 
         :param fileName: the path and name of the file
-        :returns: fastPolicyName - the fast policy name
+        :returns: retString - the returned string
         """
-        fastPolicyName = None
+        retString = None
         myFile = open(fileName, 'r')
         data = myFile.read()
         myFile.close()
         dom = parseString(data)
-        fastPolicy = dom.getElementsByTagName('FastPolicy')
-        if fastPolicy is not None and len(fastPolicy) > 0:
-            fastPolicyXml = fastPolicy[0].toxml()
-            fastPolicyXml = fastPolicyXml.replace('<FastPolicy>', '')
-            fastPolicyName = fastPolicyXml.replace('</FastPolicy>', '')
+        tag = dom.getElementsByTagName(stringToParse)
+        if tag is not None and len(tag) > 0:
+            strXml = tag[0].toxml()
+            strXml = strXml.replace('<%s>' % stringToParse, '')
+            strXml = strXml.replace('\n', '')
+            strXml = strXml.replace('\t', '')
+            retString = strXml.replace('</%s>' % stringToParse, '')
+            retString = retString.strip()
+        return retString
+
+    def parse_fast_policy_name_from_file(self, fileName):
+        """Parse the fast policy name from config file.
+
+        If it is not there, then NON FAST is assumed.
+
+        :param fileName: the path and name of the file
+        :returns: fastPolicyName - the fast policy name
+        """
+
+        fastPolicyName = self._parse_from_file(fileName, 'FastPolicy')
+        if fastPolicyName:
             LOG.debug("File %(fileName)s: Fast Policy is %(fastPolicyName)s"
                       % {'fileName': fileName,
                          'fastPolicyName': fastPolicyName})
-            return fastPolicyName
         else:
             LOG.info(_("Fast Policy not found."))
-            return None
+        return fastPolicyName
 
     def parse_array_name_from_file(self, fileName):
         """Parse the array name from config file.
@@ -673,20 +673,10 @@ class EMCVMAXUtils(object):
         :param fileName: the path and name of the file
         :returns: arrayName - the array name
         """
-        arrayName = None
-        myFile = open(fileName, 'r')
-        data = myFile.read()
-        myFile.close()
-        dom = parseString(data)
-        array = dom.getElementsByTagName('Array')
-        if array is not None and len(array) > 0:
-            arrayXml = array[0].toxml()
-            arrayXml = arrayXml.replace('<Array>', '')
-            arrayName = arrayXml.replace('</Array>', '')
-            return arrayName
-        else:
+        arrayName = self._parse_from_file(fileName, 'Array')
+        if not arrayName:
             LOG.debug("Array not found from config file.")
-            return None
+        return arrayName
 
     def parse_pool_name_from_file(self, fileName):
         """Parse the pool name from config file.
@@ -696,18 +686,8 @@ class EMCVMAXUtils(object):
         :param fileName: the path and name of the file
         :returns: poolName - the pool name
         """
-        poolName = None
-        myFile = open(fileName, 'r')
-        data = myFile.read()
-        myFile.close()
-        dom = parseString(data)
-        pool = dom.getElementsByTagName('Pool')
-        if pool is not None and len(pool) > 0:
-            poolXml = pool[0].toxml()
-            poolXml = poolXml.replace('<Pool>', '')
-            poolName = poolXml.replace('</Pool>', '')
-            return poolName
-        else:
+        poolName = self._parse_from_file(fileName, 'Pool')
+        if not poolName:
             LOG.debug("Pool not found from config file.")
 
         return poolName
@@ -1148,7 +1128,7 @@ class EMCVMAXUtils(object):
             self, conn, hardwareIdManagementService):
         """Get all the hardware ids from an array.
 
-        :param conn: connection to the ecom
+        :param conn: connection to the ecom server
         :param: hardwareIdManagementService - hardware id management service
         :returns: hardwareIdInstanceNames - the list of hardware
                                             id instance names
@@ -1177,11 +1157,15 @@ class EMCVMAXUtils(object):
                 propertiesList = (
                     ipProtocolEndpointInstance.properties.items())
                 for properties in propertiesList:
-                    if properties[0] == 'ElementName':
+                    if properties[0] == 'IPv4Address':
                         cimProperties = properties[1]
                         foundIpAddress = cimProperties.value
-                        break
-                if foundIpAddress is not None:
-                    break
+                        if (foundIpAddress == '127.0.0.1'
+                                or foundIpAddress == '0.0.0.0'):
+                            foundIpAddress = None
+                        else:
+                            break
+            if foundIpAddress is not None:
+                break
 
         return foundIpAddress