VMAX driver - failover error fix

Issuing the failover command when the backend is already
failed-over, or the failback command (denoted by specifying
"--backend_id default") when the backend is not failed over
does not stop immediately, and attempts to fulfill the
command in the incorrect state. This could result in
inconsistencies, and also leads to confusing error messages
for the end user. This patch rectifies the issue and logs
appropriate errors.

Change-Id: I58b2e030cd5a2e3a89a030da0643a61cb7264d64
Closes-Bug: #1660383

cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py

@@ -9235,6 +9235,7 @@ class VMAXCommonTest(test.TestCase):
         self.assertEqual(verify_update_fo, volume_update)
         # Path 2: Failback non replicated volume
         # Path 2a: Volume still available on primary
+        common.failover = True
         verify_update_fb1 = [{'volume_id': volumes[0]['id'],
                               'updates': {'status': 'available'}}]
         secondary_id, volume_update_1 = (
@@ -9243,6 +9244,7 @@ class VMAXCommonTest(test.TestCase):
         # Path 2a: Volume not still available on primary
         with mock.patch.object(common, '_find_lun',
                                return_value=None):
+            common.failover = True
             secondary_id, volume_update_2 = (
                 common.failover_host('context', volumes, 'default'))
             self.assertEqual(verify_update_fo, volume_update_2)
@@ -9632,6 +9634,7 @@ class EMCV3ReplicationTest(test.TestCase):
                   {'replication_status': fields.ReplicationStatus.ENABLED,
                    'provider_location': loc,
                    'replication_driver_data': rep_data}}])
+        self.driver.common.failover = True
         secondary_id, volume_update_list = (
             self.driver.failover_host('context', volumes, 'default'))
         self.assertEqual(check_update_list, volume_update_list)
@@ -9677,6 +9680,7 @@ class EMCV3ReplicationTest(test.TestCase):
                   {'replication_status': fields.ReplicationStatus.ENABLED,
                    'replication_driver_data': rep_data,
                    'provider_location': loc}}])
+        self.driver.common.failover = True
         secondary_id, volume_update_list = (
             self.driver.failover_host('context', volumes, 'default'))
         self.assertEqual(check_update_list, volume_update_list)
@@ -9698,6 +9702,7 @@ class EMCV3ReplicationTest(test.TestCase):
                       fields.ReplicationStatus.FAILOVER_ERROR),
                       'provider_location': fake_location,
                       'replication_driver_data': 'fake_data'}}])
+        self.driver.common.failover = True
         secondary_id, volume_update_list = (
             self.driver.failover_host('context', fake_volumes, 'default'))
         self.assertEqual(check_update_list, volume_update_list)
@@ -10016,3 +10021,17 @@ class EMCV3ReplicationTest(test.TestCase):
         self.driver.delete_volume(volume)
         common.cleanup_lun_replication.assert_called_once_with(
             common.conn, volume, volumeName, volumeInstance, extraSpecs)
+
+    def test_failback_failover_wrong_state(self):
+        common = self.driver.common
+        volumes = [self.data.test_volume_re]
+        # failover command, backend already failed over
+        common.failover = True
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.failover_host,
+                          'context', volumes, None)
+        # failback command, backend not failed over
+        common.failover = False
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.failover_host,
+                          'context', volumes, 'default')

cinder/volume/drivers/dell_emc/vmax/common.py

@@ -5319,12 +5319,34 @@ class VMAXCommon(object):
         if not self.conn:
             self.conn = self._get_ecom_connection()
         if secondary_id != 'default':
-            self.failover = True
-            if self.rep_config:
-                secondary_id = self.rep_config['array']
+            if not self.failover:
+                self.failover = True
+                if self.rep_config:
+                    secondary_id = self.rep_config['array']
+            else:
+                exception_message = (_(
+                    "Backend %(backend)s is already failed over. "
+                    "If you wish to failback, please append "
+                    "'--backend_id default' to your command.")
+                    % {'backend': self.configuration.safe_get(
+                       'volume_backend_name')})
+                LOG.error(exception_message)
+                raise exception.VolumeBackendAPIException(
+                    data=exception_message)
         else:
-            self.failover = False
-            secondary_id = None
+            if self.failover:
+                self.failover = False
+                secondary_id = None
+            else:
+                exception_message = (_(
+                    "Cannot failback backend %(backend)s- backend not "
+                    "in failed over state. If you meant to failover, please "
+                    "omit the '--backend_id default' from the command")
+                    % {'backend': self.configuration.safe_get(
+                       'volume_backend_name')})
+                LOG.error(exception_message)
+                raise exception.VolumeBackendAPIException(
+                    data=exception_message)
 
         def failover_volume(vol, failover):
             loc = vol['provider_location']