Fix Storwize terminate_connection with no host

Nova may pass a connector to Cinder with no 'host' field, which was
causing a KeyError in the Storwize driver. This patch resolves this case
by doing the following:
1. If the volume is mapped to only 1 host, unmap it
2. If the volume was not mapped or mapped to multiple hosts, print a
   warning but don't raise an exception

Change-Id: I0ec1c24adbdfbcf1c4868b4981a2e2618d4b411c
Closes-Bug: #1244257

diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py
index 24a219d211564552e2bd4b5bceafc52cf4286109..bf48525fecd14bf83f09903a7bcd053ee15065d3 100644 (file)
--- a/cinder/tests/test_storwize_svc.py
+++ b/cinder/tests/test_storwize_svc.py
@@ -975,7 +975,7 @@ port_speed!N/A
         del self._mappings_list[this_mapping]
         return ('', '')
 
-    # List information about vdisk-host mappings
+    # List information about host->vdisk mappings
     def _cmd_lshostvdiskmap(self, **kwargs):
         index = 1
         no_hdr = 0
@@ -998,6 +998,32 @@ port_speed!N/A
 
         return self._print_info_cmd(rows=rows, **kwargs)
 
+    # List information about vdisk->host mappings
+    def _cmd_lsvdiskhostmap(self, **kwargs):
+        mappings_found = 0
+        vdisk_name = kwargs['obj']
+
+        if vdisk_name not in self._volumes_list:
+            return self._errors['CMMVC5753E']
+
+        rows = []
+        rows.append(['id name', 'SCSI_id', 'host_id', 'host_name', 'vdisk_UID',
+                     'IO_group_id', 'IO_group_name'])
+
+        for k, mapping in self._mappings_list.iteritems():
+            if (mapping['vol'] == vdisk_name):
+                mappings_found += 1
+                volume = self._volumes_list[mapping['vol']]
+                host = self._hosts_list[mapping['host']]
+                rows.append([volume['id'], volume['name'], host['id'],
+                            host['host_name'], volume['uid'],
+                            volume['IO_group_id'], volume['IO_group_name']])
+
+        if mappings_found:
+            return self._print_info_cmd(rows=rows, **kwargs)
+        else:
+            return ('', '')
+
     # Create a FlashCopy mapping
     def _cmd_mkfcmap(self, **kwargs):
         source = ''
@@ -1369,6 +1395,8 @@ port_speed!N/A
             out, err = self._cmd_rmvdiskhostmap(**kwargs)
         elif command == 'lshostvdiskmap':
             out, err = self._cmd_lshostvdiskmap(**kwargs)
+        elif command == 'lsvdiskhostmap':
+            out, err = self._cmd_lsvdiskhostmap(**kwargs)
         elif command == 'mkfcmap':
             out, err = self._cmd_mkfcmap(**kwargs)
         elif command == 'prestartfcmap':
@@ -1998,8 +2026,10 @@ class StorwizeSVCDriverTestCase(test.TestCase):
 
         # Try to remove connection from volume that isn't mapped (should print
         # message but NOT fail)
-        vol_no_exist = {'name': 'i_dont_exist'}
-        self.driver.terminate_connection(vol_no_exist, self._connector)
+        unmapped_vol = self._generate_vol_info(None, None)
+        self.driver.create_volume(unmapped_vol)
+        self.driver.terminate_connection(unmapped_vol, self._connector)
+        self.driver.delete_volume(unmapped_vol)
 
         # Remove the mapping from the 1st volume and delete it
         self.driver.terminate_connection(volume1, self._connector)
@@ -2010,9 +2040,19 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         host_name = self.driver._get_host_from_connector(self._connector)
         self.assertIsNotNone(host_name)
 
-        # Remove the mapping from the 2nd volume and delete it. The host should
+        # Remove the mapping from the 2nd volume. The host should
         # be automatically removed because there are no more mappings.
         self.driver.terminate_connection(volume2, self._connector)
+
+        # Check if we successfully terminate connections when the host is not
+        # specified (see bug #1244257)
+        fake_conn = {'ip': '127.0.0.1', 'initiator': 'iqn.fake'}
+        self.driver.initialize_connection(volume2, self._connector)
+        host_name = self.driver._get_host_from_connector(self._connector)
+        self.assertIsNotNone(host_name)
+        self.driver.terminate_connection(volume2, fake_conn)
+        host_name = self.driver._get_host_from_connector(self._connector)
+        self.assertIsNone(host_name)
         self.driver.delete_volume(volume2)
         self._assert_vol_exists(volume2['name'], False)
 

diff --git a/cinder/volume/drivers/storwize_svc.py b/cinder/volume/drivers/storwize_svc.py
index d03da19c576f18947209f4166460abac2bfb80f0..cca77b9d573bafcbc3cadb48f0a9206c34dea784 100644 (file)
--- a/cinder/volume/drivers/storwize_svc.py
+++ b/cinder/volume/drivers/storwize_svc.py
@@ -551,8 +551,7 @@ class StorwizeSVCDriver(san.SanDriver):
 
         """
 
-        prefix = self._connector_to_hostname_prefix(connector)
-        LOG.debug(_('enter: _get_host_from_connector: prefix %s') % prefix)
+        LOG.debug(_('enter: _get_host_from_connector: %s') % str(connector))
 
         # Get list of host in the storage
         ssh_cmd = ['svcinfo', 'lshost', '-delim', '!']
@@ -627,6 +626,22 @@ class StorwizeSVCDriver(san.SanDriver):
                   {'host': connector['host'], 'host_name': host_name})
         return host_name
 
+    def _get_vdiskhost_mappings(self, vdisk_name):
+        """Return the defined storage mappings for a vdisk."""
+
+        return_data = {}
+        ssh_cmd = ['svcinfo', 'lsvdiskhostmap', '-delim', '!', vdisk_name]
+        out, err = self._run_ssh(ssh_cmd)
+
+        mappings = out.strip().split('\n')
+        if len(mappings):
+            header = mappings.pop(0)
+            for mapping_line in mappings:
+                mapping_data = self._get_hdr_dic(header, mapping_line, '!')
+                return_data[mapping_data['host_name']] = mapping_data
+
+        return return_data
+
     def _get_hostvdisk_mappings(self, host_name):
         """Return the defined storage mappings for a host."""
 
@@ -890,31 +905,49 @@ class StorwizeSVCDriver(san.SanDriver):
                                              'conn': str(connector)})
 
         vol_name = volume['name']
-        host_name = self._get_host_from_connector(connector)
-        # Verify that _get_host_from_connector returned the host.
-        # This should always succeed as we terminate an existing connection.
-        self._driver_assert(
-            host_name is not None,
-            _('_get_host_from_connector failed to return the host name '
-              'for connector'))
-
-        # Check if vdisk-host mapping exists, remove if it does
-        mapping_data = self._get_hostvdisk_mappings(host_name)
-        if vol_name in mapping_data:
-            ssh_cmd = ['svctask', 'rmvdiskhostmap', '-host', host_name,
-                       vol_name]
-            out, err = self._run_ssh(ssh_cmd)
-            # Verify CLI behaviour - no output is returned from
-            # rmvdiskhostmap
-            self._assert_ssh_return(len(out.strip()) == 0,
-                                    'terminate_connection', ssh_cmd, out, err)
-            del mapping_data[vol_name]
+        if 'host' in connector:
+            host_name = self._get_host_from_connector(connector)
+            self._driver_assert(
+                host_name is not None,
+                _('_get_host_from_connector failed to return the host name '
+                  'for connector'))
         else:
-            LOG.error(_('terminate_connection: No mapping of volume '
-                        '%(vol_name)s to host %(host_name)s found') %
-                      {'vol_name': vol_name, 'host_name': host_name})
+            # See bug #1244257
+            host_name = None
+
+        # Check if vdisk-host mapping exists, remove if it does. If no host
+        # name was given, but only one mapping exists, we can use that.
+        mapping_data = self._get_vdiskhost_mappings(vol_name)
+        if len(mapping_data) == 0:
+            LOG.warning(_('terminate_connection: No mapping of volume '
+                          '%(vol_name)s to any host found.') %
+                        {'vol_name': vol_name})
+            return
+        if host_name is None:
+            if len(mapping_data) > 1:
+                LOG.warning(_('terminate_connection: Multiple mappings of '
+                              'volume %(vol_name)s found, no host '
+                              'specified.') % {'vol_name': vol_name})
+                return
+            else:
+                host_name = mapping_data.keys()[0]
+        else:
+            if host_name not in mapping_data:
+                LOG.error(_('terminate_connection: No mapping of volume '
+                            '%(vol_name)s to host %(host_name)s found') %
+                          {'vol_name': vol_name, 'host_name': host_name})
+                return
+
+        # We have a valid host_name now
+        ssh_cmd = ['svctask', 'rmvdiskhostmap', '-host', host_name,
+                   vol_name]
+        out, err = self._run_ssh(ssh_cmd)
+        # Verify CLI behaviour - no output is returned from rmvdiskhostmap
+        self._assert_ssh_return(len(out.strip()) == 0,
+                                'terminate_connection', ssh_cmd, out, err)
 
         # If this host has no more mappings, delete it
+        mapping_data = self._get_hostvdisk_mappings(host_name)
         if not mapping_data:
             self._delete_host(host_name)