Storwize: Add force flag on vdisk mapping call

If a vdisk is already mapped and multihostmap is True,
use the force flag on the first call to mkvdiskhostmap.
There is no functional change associated with this defect
for this method, a vdisk that is already mapped to a
host without the multihostmap flag will fail and it
will be successful if the multihostmap is True.

The code was waiting for the first call to svcinfo
mkvdiskhostmap to fail before trying a second time with
the force option if multihostmap is set to true. This fix
adds the force option before the call to svcinfo
mkvdiskhostmap to avoid this overhead. The svcinfo
mkvdiskhostmap call accepts the force option when
a volume is not already mapped, and mkvdiskhostmap()
already has a force parameter,
so the existing testcase is still valid.

Fixed a bug where mkvdiskhostmap() was returning
data, and add a testcase to check that it is not
returning data.

Change-Id: I2320aeec9422625ce2a01aa5d28801207e8671a5
Closes-Bug: 1511751

diff --git a/cinder/tests/unit/test_storwize_svc.py b/cinder/tests/unit/test_storwize_svc.py
index ff0ab6e81f5e0eacef98cc1083b9e170d8f1b1ff..40187e71c357983dc95197f298927cedf84e18e4 100644 (file)
--- a/cinder/tests/unit/test_storwize_svc.py
+++ b/cinder/tests/unit/test_storwize_svc.py
@@ -4284,3 +4284,25 @@ class StorwizeHelpersTestCase(test.TestCase):
         self.assertTrue(self.storwize_svc_common.compression_enabled())
 
         self.assertTrue(self.storwize_svc_common.compression_enabled())
+
+
+class StorwizeSSHTestCase(test.TestCase):
+    def setUp(self):
+        super(StorwizeSSHTestCase, self).setUp()
+        self.storwize_ssh = storwize_svc_common.StorwizeSSH(None)
+
+    def test_mkvdiskhostmap(self):
+        # mkvdiskhostmap should not be returning anything
+        with mock.patch.object(
+                storwize_svc_common.StorwizeSSH,
+                'run_ssh_check_created') as run_ssh_check_created:
+            run_ssh_check_created.return_value = None
+            ret = self.storwize_ssh.mkvdiskhostmap('HOST1', 9999, 511, False)
+            self.assertIsNone(ret)
+            ret = self.storwize_ssh.mkvdiskhostmap('HOST2', 9999, 511, True)
+            self.assertIsNone(ret)
+            ex = exception.VolumeBackendAPIException(data='CMMVC6071E')
+            run_ssh_check_created.side_effect = ex
+            self.assertRaises(exception.VolumeBackendAPIException,
+                              self.storwize_ssh.mkvdiskhostmap,
+                              'HOST3', 9999, 511, True)

diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py
index f7ab07d82c15b838bb357b5750ae124a3f5ac49e..11929545d9f0ceeb3b6649aab1c77452c4620603 100644 (file)
--- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py
+++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py
@@ -246,24 +246,21 @@ class StorwizeSSH(object):
         """
         ssh_cmd = ['svctask', 'mkvdiskhostmap', '-host', '"%s"' % host,
                    '-scsi', lun, vdisk]
-        out, err = self._ssh(ssh_cmd, check_exit_code=False)
-        if 'successfully created' in out:
-            return
-        if not err:
-            msg = (_('Did not find success message nor error for %(fun)s: '
-                     '%(out)s.') % {'out': out, 'fun': ssh_cmd})
-            raise exception.VolumeBackendAPIException(data=msg)
-        if err.startswith('CMMVC6071E'):
-            if not multihostmap:
+        if multihostmap:
+            ssh_cmd.insert(ssh_cmd.index('mkvdiskhostmap') + 1, '-force')
+        try:
+            self.run_ssh_check_created(ssh_cmd)
+        except Exception as ex:
+            if (not multihostmap and hasattr(ex, 'message') and
+                    'CMMVC6071E' in ex.message):
                 LOG.error(_LE('storwize_svc_multihostmap_enabled is set '
                               'to False, not allowing multi host mapping.'))
                 raise exception.VolumeDriverException(
                     message=_('CMMVC6071E The VDisk-to-host mapping was not '
                               'created because the VDisk is already mapped '
                               'to a host.\n"'))
-
-        ssh_cmd.insert(ssh_cmd.index('mkvdiskhostmap') + 1, '-force')
-        return self.run_ssh_check_created(ssh_cmd)
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE('Error mapping VDisk-to-host'))
 
     def rmvdiskhostmap(self, host, vdisk):
         ssh_cmd = ['svctask', 'rmvdiskhostmap', '-host', '"%s"' % host, vdisk]