From: Gerald McBrearty <gfm@us.ibm.com>
Date: Fri, 20 Nov 2015 19:23:21 +0000 (-0600)
Subject: Storwize: Add force flag on vdisk mapping call
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=cffad569f82bb5b6b52129015e37cfd62701622c;p=openstack-build%2Fcinder-build.git

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 ff0ab6e81..40187e71c 100644
--- 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 f7ab07d82..11929545d 100644
--- 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]