From 91c7f040ada08be2eaf0bc59dced6d48b7cb6fe6 Mon Sep 17 00:00:00 2001 From: GirishChilukuri Date: Tue, 23 Mar 2021 09:58:46 +0000 Subject: [PATCH] [SVF]:Retype in-use hyperswap volume [Spectrum Virtualize Family] During HyperSwap volume retype, adding site and io group information to host if not already added to host. Closes-Bug: #1920890 Change-Id: I2f52cc8e42c487c04ddb54cd2d7afe4b35d20eea --- .../volume/drivers/ibm/test_storwize_svc.py | 115 +++++++++++++++- .../ibm/storwize_svc/storwize_svc_common.py | 73 ++++++++-- .../ibm/storwize_svc/storwize_svc_fc.py | 125 +++++++++++++++++- ...use-hyperswap-volume-95a6c033e493ee59.yaml | 7 + 4 files changed, 302 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/bug-1920890-ibm-svf-Retype-in-use-hyperswap-volume-95a6c033e493ee59.yaml diff --git a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py index 6b1b0713b5c..3d6ab415907 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -4574,6 +4574,8 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): configuration=config) self._driver = storwize_svc_iscsi.StorwizeSVCISCSIDriver( configuration=config) + self.fcdriver = StorwizeSVCFcFakeDriver( + configuration=config) wwpns = [ six.text_type(random.randint(0, 9999999999999999)).zfill(16), six.text_type(random.randint(0, 9999999999999999)).zfill(16)] @@ -4586,6 +4588,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): self.sim = StorwizeSVCManagementSimulator(SVC_POOLS) self.driver.set_fake_storage(self.sim) + self.fcdriver.set_fake_storage(self.sim) self.ctxt = context.get_admin_context() else: self._reset_flags() @@ -7695,6 +7698,28 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): snapshot_id=snapshot.id) self.driver.create_volume_from_snapshot(vol6, snapshot) + @ddt.data({'pool': 'openstack2', 'peer_pool': 'openstack3'}, + {'pool': 'openstack', 'peer_pool': None}, + {'pool': None, 'peer_pool': 'openstack1'}) + @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lsmdiskgrp') + def test_storwize_svc_get_hyperswap_pool_io_grp(self, pools, lsmdiskgrp): + lsmdiskgrp.side_effect = [{'site_id': '1'}, + {'site_id': '2'}] + + if pools['pool'] and pools['peer_pool']: + iogrp_list = self.driver._helpers.get_hyperswap_pool_io_grp( + self.driver._state, pools['pool'], pools['peer_pool']) + lsmdiskgrp.assert_called() + self.assertEqual(2, lsmdiskgrp.call_count) + self.assertEqual(['0', '1'], iogrp_list) + else: + self.assertRaises(exception.InvalidInput, + self.driver._helpers.get_hyperswap_pool_io_grp, + self.driver._state, pools['pool'], + pools['peer_pool']) + lsmdiskgrp.assert_not_called() + self.assertEqual(0, lsmdiskgrp.call_count) + @ mock.patch.object(storwize_svc_common.StorwizeSSH, 'lsmdiskgrp') def test_storwize_svc_select_iogrp_with_pool_site(self, lsmdiskgrp): opts = {} @@ -8286,6 +8311,91 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): volume3['volume_type'] = non_hyper_type self.driver.delete_volume(volume3) + @mock.patch.object(storwize_svc_common.StorwizeSVCCommonDriver, + '_update_replication_properties') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_hyperswap_pool_io_grp') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_system_info') + def test_retype_hyperswap_inuse_volume_fc(self, + get_system_info, + get_hyperswap_pool_io_grp, + update_rep_prop): + get_system_info.return_value = {'code_level': (7, 7, 0, 0), + 'topology': 'hyperswap', + 'system_name': 'storwize-svc-sim', + 'system_id': '0123456789ABCDEF'} + + self.fcdriver.do_setup(None) + + spec1 = {'drivers:iogrp': '0,1'} + non_hyper_type = self._create_volume_type(spec1, 'non_hyper_type') + + volume = testutils.create_volume(self.ctxt, + volume_type_id=non_hyper_type.id, + host='openstack@svc#hyperswap1') + self.fcdriver.create_volume(volume) + volume.previous_status = 'in-use' + host = {'host': 'openstack@svc#hyperswap1'} + + spec = {'drivers:volume_topology': 'hyperswap', + 'peer_pool': 'hyperswap2'} + + hyper_type = self._create_volume_type(spec, 'hypertype') + diff, _equal = volume_types.volume_types_diff(self.ctxt, + non_hyper_type['id'], + hyper_type['id']) + + self.fcdriver.retype(self.ctxt, volume, hyper_type, diff, host) + + self._assert_vol_exists(volume.name, True) + self._assert_vol_exists('site2' + volume.name, True) + self._assert_vol_exists('fcsite1' + volume.name, True) + self._assert_vol_exists('fcsite2' + volume.name, True) + + get_hyperswap_pool_io_grp.assert_called() + + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_hyperswap_pool_io_grp') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_system_info') + def test_retype_hyperswap_inuse_volume_iscsi(self, + get_system_info, + get_hyperswap_pool_io_grp): + get_system_info.return_value = {'code_level': (7, 7, 0, 0), + 'topology': 'hyperswap', + 'system_name': 'storwize-svc-sim', + 'system_id': '0123456789ABCDEF'} + + self.driver.do_setup(None) + + spec1 = {'drivers:iogrp': '0,1'} + non_hyper_type = self._create_volume_type(spec1, 'non_hyper_type') + + volume = testutils.create_volume(self.ctxt, + volume_type_id=non_hyper_type.id, + host='openstack@svc#hyperswap1') + self.driver.create_volume(volume) + volume.previous_status = 'in-use' + host = {'host': 'openstack@svc#hyperswap1'} + + spec = {'drivers:volume_topology': 'hyperswap', + 'peer_pool': 'hyperswap2'} + + hyper_type = self._create_volume_type(spec, 'hypertype') + diff, _equal = volume_types.volume_types_diff(self.ctxt, + non_hyper_type['id'], + hyper_type['id']) + + self.driver.retype(self.ctxt, volume, hyper_type, diff, host) + + self._assert_vol_exists(volume.name, True) + self._assert_vol_exists('site2' + volume.name, True) + self._assert_vol_exists('fcsite1' + volume.name, True) + self._assert_vol_exists('fcsite2' + volume.name, True) + + get_hyperswap_pool_io_grp.assert_not_called() + def test_retype_hyperswap_volume_failure_case(self): with mock.patch.object(storwize_svc_common.StorwizeHelpers, 'get_system_info') as get_system_info: @@ -8345,10 +8455,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): diff, _equal = volume_types.volume_types_diff( self.ctxt, hyperswap_vol_type['id'], inuse_type['id']) - self.assertRaises(exception.InvalidInput, - self.driver.retype, - self.ctxt, volume, inuse_type, diff, - host) + self.driver.retype(self.ctxt, volume, inuse_type, diff, host) # retype from hyperswap volume to replication volume spec3 = {'replication_enabled': ' 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 e878801f14a..e10af186450 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -271,6 +271,14 @@ class StorwizeSSH(object): ssh_cmd = ['svctask', 'addhostport', '-force'] + port + ['"%s"' % host] self.run_ssh_assert_no_output(ssh_cmd) + def addhostiogrp(self, host, iogrplist='all'): + ssh_cmd = ['svctask', 'addhostiogrp'] + if iogrplist == 'all': + ssh_cmd += ['-iogrpall', '"%s"' % host] + else: + ssh_cmd += ['-iogrp', ':'.join(iogrplist), '"%s"' % host] + self.run_ssh_assert_no_output(ssh_cmd) + def lshost(self, host=None): with_header = True ssh_cmd = ['svcinfo', 'lshost', '-delim', '!'] @@ -285,7 +293,11 @@ class StorwizeSSH(object): self.run_ssh_assert_no_output(ssh_cmd, log_cmd) def chhost(self, host, site): - ssh_cmd = ['svctask', 'chhost', '-site', '"%s"' % site, '"%s"' % host] + ssh_cmd = ['svctask', 'chhost'] + if site: + ssh_cmd += ['-site', '"%s"' % site, '"%s"' % host] + else: + ssh_cmd += ['-nosite', '"%s"' % host] self.run_ssh_assert_no_output(ssh_cmd) def lsiscsiauth(self): @@ -701,6 +713,10 @@ class StorwizeSSH(object): ssh_cmd = ['svctask', 'rmvdiskaccess', '-iogrp', iogrp, '"%s"' % vdisk] self.run_ssh_assert_no_output(ssh_cmd) + def lsvdiskaccess(self, vdisk): + ssh_cmd = ['svcinfo', 'lsvdiskaccess', '-delim', '!', '"%s"' % vdisk] + return self.run_ssh_info(ssh_cmd, with_header=True) + def lsportfc(self, node_id): ssh_cmd = ['svcinfo', 'lsportfc', '-delim', '!', '-filtervalue', 'node_id=%s' % node_id] @@ -891,6 +907,43 @@ class StorwizeHelpers(object): raise exception.VolumeBackendAPIException(data=msg) return res + def get_hyperswap_pool_io_grp(self, state, pool, peer_pool): + if not peer_pool or not pool: + raise exception.InvalidInput( + reason=_('The pool and peer pool is necessary for hyperswap ' + 'volume, please configure the pool and peer pool.')) + pool_data = None + peer_pool_data = None + for stat_pool in self.stats.get('pools', []): + if stat_pool['pool_name'] == pool: + pool_data = stat_pool + elif stat_pool['pool_name'] == peer_pool: + peer_pool_data = stat_pool + + if pool_data is None or pool_data.get("site_id") is None: + pool_data = self.get_pool_attrs(pool) + if not pool_data['site_id']: + raise exception.InvalidInput( + reason=_('The pool with site is necessary for hyperswap ' + 'volume, please configure the pool with site.')) + + if peer_pool_data is None or peer_pool_data.get("site_id") is None: + peer_pool_data = self.get_pool_attrs(peer_pool) + if not peer_pool_data['site_id']: + raise exception.InvalidInput( + reason=_('The peer pool with site is necessary for ' + 'hyperswap volume, please configure the peer ' + 'pool with site.')) + + iogrp_list = [] + for node in state['storage_nodes'].values(): + if ((pool_data['site_id'] == node['site_id']) or + (peer_pool_data['site_id'] == node['site_id'])): + if node['IO_group'] not in iogrp_list: + iogrp_list.append(node['IO_group']) + + return iogrp_list + def select_io_group(self, state, opts, pool): selected_iog = 0 iog_list = StorwizeHelpers._get_valid_requested_io_groups(state, opts) @@ -2114,7 +2167,7 @@ class StorwizeHelpers(object): attrs = self._get_flashcopy_mapping_attributes(map_id) copy_rate = attrs['copy_rate'] # update flashcopy rate for clone volume - if copy_rate != '0': + if copy_rate != '0' and attrs['rc_controlled'] != 'yes': self.ssh.chfcmap(map_id, copyrate=six.text_type(new_flashcopy_rate)) @@ -5073,11 +5126,6 @@ class StorwizeSVCCommonDriver(san.SanDriver, 'support pool change.') % volume.name) LOG.error(msg) raise exception.InvalidInput(message=msg) - if volume.previous_status == 'in-use': - msg = _('Retype an in-use volume to a hyperswap ' - 'volume is not allowed.') - LOG.error(msg) - raise exception.InvalidInput(message=msg) if not new_opts['easytier']: raise exception.InvalidInput( reason=_('The default easytier of hyperswap volume is ' @@ -5106,8 +5154,8 @@ class StorwizeSVCCommonDriver(san.SanDriver, LOG.error(msg) raise exception.InvalidInput(message=msg) - def _retype_hyperswap_volume(self, volume, host, old_opts, new_opts, - old_pool, new_pool, vdisk_changes, + def _retype_hyperswap_volume(self, ctxt, volume, host, old_opts, + new_opts, old_pool, new_pool, vdisk_changes, need_copy, new_type): if (old_opts['volume_topology'] != 'hyperswap' and new_opts['volume_topology'] == 'hyperswap'): @@ -5216,9 +5264,10 @@ class StorwizeSVCCommonDriver(san.SanDriver, change_mirror, new_rep_type, old_rep_type, old_pool, new_pool, old_io_grp) - self._retype_hyperswap_volume(volume, host, old_opts, new_opts, - old_pool, new_pool, vdisk_changes, - need_copy, new_type) + self._retype_hyperswap_volume(ctxt, volume, host, old_opts, + new_opts, old_pool, new_pool, + vdisk_changes, need_copy, + new_type) # Updating Hyperswap volume replication properties model_update = self._update_replication_properties(ctxt, volume, model_update) diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py index dc05d76e904..a626144d00b 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py @@ -171,9 +171,11 @@ class StorwizeSVCFCDriver(storwize_common.StorwizeSVCCommonDriver): volume_name, backend_helper, node_state = self._get_vol_sys_info( volume) - host_site = self._get_volume_host_site_from_conf(volume, - connector) + host_site = None is_hyper_volume = self.is_volume_hyperswap(volume) + if is_hyper_volume: + host_site = self._get_volume_host_site_from_conf(volume, + connector) # The host_site is necessary for hyperswap volume. if is_hyper_volume and host_site is None: msg = (_('There is no correct storwize_preferred_host_site ' @@ -365,3 +367,122 @@ class StorwizeSVCFCDriver(storwize_common.StorwizeSVCCommonDriver): 'conn': connector, 'info': info}) return info + + def _get_volume_connection_info(self, ctxt, volume, host_info, + iogrp_list): + connector = {'wwpns': []} + connection_info = {"driver_volume_type": "fibre_channel"} + data = {} + + for wwpn in host_info.select('WWPN'): + connector['wwpns'].append(wwpn) + + vol_name, backend_helper, node_state = self._get_vol_sys_info(volume) + + data['target_discovered'] = False + data['volume_id'] = volume.id + + conn_wwpns = [] + for node in node_state['storage_nodes'].values(): + if node['IO_group'] not in iogrp_list: + continue + + # The Storwize/svc release 7.7.0.0 introduced NPIV feature, + # Different commands be used to get the wwpns for host I/O + if node_state['code_level'] < (7, 7, 0, 0): + conn_wwpns.extend(node['WWPN']) + else: + npivwwpns = backend_helper.get_npiv_wwpns(node_id=node['id'], + host_io="yes") + conn_wwpns.extend(npivwwpns) + + i_t_map = self._make_initiator_target_map(connector['wwpns'], + conn_wwpns) + data["initiator_target_map"] = i_t_map + data["target_wwn"] = conn_wwpns + + connection_info['data'] = data + connection_info['connector'] = connector + + return connection_info + + def _retype_hyperswap_volume(self, ctxt, volume, host, old_opts, + new_opts, old_pool, new_pool, vdisk_changes, + need_copy, new_type): + if (old_opts['volume_topology'] != 'hyperswap' and + new_opts['volume_topology'] == 'hyperswap'): + LOG.debug('retype: Convert a normal volume %s to hyperswap ' + 'volume.', volume.name) + conn_info = {} + if volume.previous_status == 'in-use': + vdisk_info = self._helpers.ssh.lsvdiskhostmap(volume.name) + peer_pool = new_opts['peer_pool'] + iogrp_list = self._helpers.get_hyperswap_pool_io_grp( + self._state, new_pool, peer_pool) + for mapping_info in vdisk_info: + host = mapping_info['host_name'] + try: + host_info = self._helpers.ssh.lshost(host) + conn_info[host] = self._get_volume_connection_info( + ctxt, volume, host_info, iogrp_list) + host_site = self._get_volume_host_site_from_conf( + volume, conn_info[host].get('connector')) + self._update_host_site_for_hyperswap_volume( + host, host_site) + self._helpers.ssh.addhostiogrp(host, + iogrp_list) + except Exception as ex: + msg = _('Error updating host %(host)s due to %(ex)s', + {'host': host, 'ex': ex}) + raise exception.VolumeBackendAPIException(data=msg) + self._helpers.convert_volume_to_hyperswap(volume.name, + new_opts, + self._state) + if volume.previous_status == 'in-use': + for host, info in conn_info.items(): + try: + fczm_utils.add_fc_zone(info) + except Exception as ex: + self._helpers.convert_hyperswap_volume_to_normal( + volume.name, new_opts['peer_pool']) + msg = _('Zoning failed for volume %(vol)s and host ' + '%(host)s due to %(ex)s.', + {'vol': volume.name, 'host': host, 'ex': ex}) + raise exception.VolumeBackendAPIException(data=msg) + elif (old_opts['volume_topology'] == 'hyperswap' and + new_opts['volume_topology'] != 'hyperswap'): + LOG.debug('retype: Convert a hyperswap volume %s to normal ' + 'volume.', volume.name) + if new_pool == old_pool: + self._helpers.convert_hyperswap_volume_to_normal( + volume.name, + old_opts['peer_pool']) + elif new_pool == old_opts['peer_pool']: + self._helpers.convert_hyperswap_volume_to_normal( + volume.name, + old_pool) + if volume.previous_status == 'in-use': + vdisk_info = self._helpers.ssh.lsvdiskhostmap(volume.name) + for mapping_info in vdisk_info: + res = self._helpers.check_host_mapped_vols( + mapping_info['host_name']) + if len(res) == 1: + self._helpers.update_host(mapping_info['host_name'], + None) + else: + rel_info = self._helpers.get_relationship_info(volume.name) + aux_vdisk = rel_info['aux_vdisk_name'] + if need_copy: + self.add_vdisk_copy(aux_vdisk, old_opts['peer_pool'], new_type, + auto_delete=True) + elif vdisk_changes: + self._helpers.change_vdisk_options(aux_vdisk, + vdisk_changes, + new_opts, self._state) + if need_copy: + self.add_vdisk_copy(volume.name, old_pool, new_type, + auto_delete=True) + elif vdisk_changes: + self._helpers.change_vdisk_options(volume.name, + vdisk_changes, + new_opts, self._state) diff --git a/releasenotes/notes/bug-1920890-ibm-svf-Retype-in-use-hyperswap-volume-95a6c033e493ee59.yaml b/releasenotes/notes/bug-1920890-ibm-svf-Retype-in-use-hyperswap-volume-95a6c033e493ee59.yaml new file mode 100644 index 00000000000..9ed3228fc34 --- /dev/null +++ b/releasenotes/notes/bug-1920890-ibm-svf-Retype-in-use-hyperswap-volume-95a6c033e493ee59.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + IBM Spectrum Virtualize Family driver `Bug #1920890 + `_: Fixed issue in + retype_hyperswap_volume method to update site and iogrp information to + the host during a retype from a non-HyperSwap volume to a HyperSwap volume.