Clean Up EMC VNX Direct Driver in Cinder

This patch cleans up issues discovered during the review of
EMC VNX Direct Driver.

https://review.openstack.org/#/c/73672/

Implements blueprint emc-vnx-direct-driver
Closes-Bug: #1287944

Change-Id: I4002ef9ea14e2d843dd8cbccffa025997a54c738

diff --git a/cinder/tests/test_emc_vnxdirect.py b/cinder/tests/test_emc_vnxdirect.py
index f1a62ef306f8dd20bff30ecf524beed878858779..50000ffc9dbf9db8debdc1b6cce97d158200f963 100644 (file)
--- a/cinder/tests/test_emc_vnxdirect.py
+++ b/cinder/tests/test_emc_vnxdirect.py
@@ -279,6 +279,8 @@ class EMCVNXCLIDriverISCSITestCase(test.TestCase):
         elif cmd == ('connection', '-getport', '-sp', 'A'):
             return 'SP:  A\nPort ID:  5\nPort WWN:  iqn.1992-04.' + \
                    'com.emc:cx.fnm00124000215.a5\niSCSI Alias:  0215.a5\n', 0
+        else:
+            self.assertTrue(False)
 
     def setUp(self):
         # backup

diff --git a/cinder/volume/drivers/emc/emc_cli_iscsi.py b/cinder/volume/drivers/emc/emc_cli_iscsi.py
index bb7e9fb6feaa46ac760a777ba8508fe1916f8e73..d3e664f5b24b8665594dbd76a72859d643894085 100644 (file)
--- a/cinder/volume/drivers/emc/emc_cli_iscsi.py
+++ b/cinder/volume/drivers/emc/emc_cli_iscsi.py
@@ -86,7 +86,7 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
         """Initializes the connection and returns connection info.
 
         The iscsi driver returns a driver_volume_type of 'iscsi'.
-        the format of the driver data is defined in _get_iscsi_properties.
+        the format of the driver data is defined in vnx_get_iscsi_properties.
 
         :param volume: volume to be attached.
         :param connector: connector information.
@@ -98,7 +98,7 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
                     'target_discovered': True,
                     'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
                     'target_portal': '127.0.0.0.1:3260',
-                    'volume_id': 1,
+                    'volume_id': '12345678-1234-4321-1234-123456789012',
                 }
             }
         """
@@ -122,7 +122,7 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
 
     def _do_iscsi_discovery(self, volume):
 
-        LOG.warn(_("ISCSI provider_location not stored for volume %s, "
+        LOG.warn(_("iSCSI provider_location not stored for volume %s, "
                  "using discovery.") % (volume['name']))
 
         (out, _err) = self._execute('iscsiadm', '-m', 'discovery',
@@ -135,8 +135,6 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
 
         return targets
 
-    # Rename this function to vnx_get_iscsi_properties because it has
-    # different input parameters from _get_iscsi_properties in the base class
     def vnx_get_iscsi_properties(self, volume, connector):
         """Gets iscsi configuration.
 
@@ -152,7 +150,7 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
 
         :target_lun:    the lun of the iSCSI target
 
-        :volume_id:    the id of the volume (currently used by xen)
+        :volume_id:    the UUID of the volume
 
         :auth_method:, :auth_username:, :auth_password:
 
@@ -223,8 +221,6 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
             properties['auth_username'] = auth_username
             properties['auth_password'] = auth_secret
 
-        LOG.debug(_("ISCSI properties: %s") % (properties))
-
         return properties
 
     def terminate_connection(self, volume, connector, **kwargs):
@@ -241,12 +237,12 @@ class EMCCLIISCSIDriver(driver.ISCSIDriver):
         If 'refresh' is True, run update the stats first.
         """
         if refresh:
-            self.update_volume_status()
+            self.update_volume_stats()
             LOG.info(_("update_volume_status:%s"), self._stats)
 
         return self._stats
 
-    def update_volume_status(self):
+    def update_volume_stats(self):
         """Retrieve status info from volume group."""
         LOG.debug(_("Updating volume status"))
         # retrieving the volume update from the VNX

diff --git a/cinder/volume/drivers/emc/emc_vnx_cli.py b/cinder/volume/drivers/emc/emc_vnx_cli.py
index 4db39dabe80b736b19ab7392f397ebdb20cd5a02..bdccf5bcff645bb74d1188d74a4c909095769b1d 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vnx_cli.py
+++ b/cinder/volume/drivers/emc/emc_vnx_cli.py
@@ -51,7 +51,7 @@ loc_opts = [
 CONF.register_opts(loc_opts)
 
 
-class EMCVnxCli():
+class EMCVnxCli(object):
     """This class defines the functions to use the native CLI functionality."""
 
     stats = {'driver_version': VERSION,
@@ -108,7 +108,7 @@ class EMCVnxCli():
             raise exception.VolumeBackendAPIException(data=out)
 
     def _cli_execute(self, *cmd, **kwargv):
-        if "check_exit_code" not in kwargv.keys():
+        if "check_exit_code" not in kwargv:
             kwargv["check_exit_code"] = True
         rc = 0
         try:
@@ -157,20 +157,19 @@ class EMCVnxCli():
 
         # wait for up to a minute to verify that the LUN has progressed
         # to Ready state
-        def _wait_for_lun_ready(volumename):
+        def _wait_for_lun_ready(volumename, start_time):
             # executing cli command to check volume
             command_to_verify = ('lun', '-list', '-name', volumename)
             out, rc = self._cli_execute(*command_to_verify)
             if rc == 0 and out.find("Ready") > -1:
                 raise loopingcall.LoopingCallDone()
-            if int(time.time()) - self.start_lun_ready > self.timeout * 60:
+            if int(time.time()) - start_time > self.timeout * 60:
                 msg = (_('LUN %s failed to become Ready'), volumename)
                 LOG.error(msg)
                 raise exception.VolumeBackendAPIException(data=msg)
 
-        self.start_lun_ready = int(time.time())
         timer = loopingcall.FixedIntervalLoopingCall(
-            _wait_for_lun_ready, volumename)
+            _wait_for_lun_ready, volumename, int(time.time()))
         timer.start(interval=self.wait_interval).wait()
 
     def delete_volume(self, volume):
@@ -308,7 +307,7 @@ class EMCVnxCli():
                  % {'snapshot': snapshotname,
                     'volume': volumename})
 
-        def _wait_for_snap_delete(snapshot):
+        def _wait_for_snap_delete(snapshot, start_time):
             # defining CLI command
             snapshotname = snapshot['name']
             volumename = snapshot['volume_name']
@@ -324,7 +323,7 @@ class EMCVnxCli():
 
             if rc not in [0, 9, 5]:
                 if rc == 13:
-                    if int(time.time()) - self.start_snap_delete < \
+                    if int(time.time()) - start_time < \
                             self.timeout * 60:
                         LOG.info(_('Snapshot %s is in use'), snapshotname)
                     else:
@@ -339,9 +338,8 @@ class EMCVnxCli():
             else:
                 raise loopingcall.LoopingCallDone()
 
-        self.start_snap_delete = int(time.time())
         timer = loopingcall.FixedIntervalLoopingCall(
-            _wait_for_snap_delete, snapshot)
+            _wait_for_snap_delete, snapshot, int(time.time()))
         timer.start(interval=self.wait_interval).wait()
 
     def create_volume_from_snapshot(self, volume, snapshot):
@@ -440,9 +438,7 @@ class EMCVnxCli():
             LOG.error(msg)
             raise exception.VolumeBackendAPIException(data=msg)
 
-        self.sync_status = False
-
-        def _wait_for_sync_status(volumename):
+        def _wait_for_sync_status(volumename, start_time):
             lun_list = ('lun', '-list', '-name', volumename,
                         '-attachedSnapshot')
             out, rc = self._cli_execute(*lun_list)
@@ -450,24 +446,19 @@ class EMCVnxCli():
                 vol_details = out.split('\n')
                 snapshotname = vol_details[2].split(':')[1].strip()
             if (snapshotname == 'N/A'):
-                self.sync_status = True
                 raise loopingcall.LoopingCallDone()
             else:
-                LOG.info(_('Waiting for the update on Sync status of %s '),
+                LOG.info(_('Waiting for the update on Sync status of %s'),
                          volumename)
-                if int(time.time()) - self.start_status >= self.timeout * 60:
-                    raise loopingcall.LoopingCallDone()
+                if int(time.time()) - start_time >= self.timeout * 60:
+                    msg = (_('Failed to really migrate %s'), volumename)
+                    LOG.error(msg)
+                    raise exception.VolumeBackendAPIException(data=msg)
 
-        self.start_status = int(time.time())
         timer = loopingcall.FixedIntervalLoopingCall(
-            _wait_for_sync_status, volumename)
+            _wait_for_sync_status, volumename, int(time.time()))
         timer.start(interval=self.wait_interval).wait()
 
-        if not self.sync_status:
-            msg = (_('Failed to really migrate %s'), volumename)
-            LOG.error(msg)
-            raise exception.VolumeBackendAPIException(data=msg)
-
     def create_cloned_volume(self, volume, src_vref):
         """Creates a clone of the specified volume."""