From: Gorka Eguileor Date: Mon, 30 Nov 2015 11:04:58 +0000 (+0100) Subject: Take into consideration races in XtremIOClient3 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=89480f159231eae154119da6b46432849b9df20a;p=openstack-build%2Fcinder-build.git Take into consideration races in XtremIOClient3 When working with FC and xtremio is using XtremIOClient3 on FC's terminate_connection we can get VolumeBackendAPIException saying a resource could not be found when we shouldn't. The cause is that Client3 is open to race conditions on 2 methods, find_lunmap and num_of_mapped_volumes, where a list of mappings is first retrieved and then we iterate this list to retrieved additional info on the mappings. The race would happen if one of the mappings is removed from the backend in the time it takes to retrieve the additional info after we have retrieved the list. This patch fixes this issue by ignoring any mappings that have been removed and are now NotFound when retrieving additional information for a mapping in those 2 methods. This patch also fixes this kind of race problems on volume creation since it uses find_lumap method. Closes-Bug: #1521143 Change-Id: I40831e04093ff475395870a333211dd0cb60440f --- diff --git a/cinder/tests/unit/test_emc_xtremio.py b/cinder/tests/unit/test_emc_xtremio.py index 067652212..4aba0fcda 100644 --- a/cinder/tests/unit/test_emc_xtremio.py +++ b/cinder/tests/unit/test_emc_xtremio.py @@ -563,22 +563,20 @@ class EMCXIODriverFibreChannelTestCase(test.TestCase): super(EMCXIODriverFibreChannelTestCase, self).setUp() clean_xms_data() - config = mock.Mock() - config.san_login = '' - config.san_password = '' - config.san_ip = '' - config.xtremio_cluster_name = '' - config.xtremio_provisioning_factor = 20.0 + self.config = mock.Mock(san_login='', + san_password='', + san_ip='', + xtremio_cluster_name='', + xtremio_provisioning_factor=20.0) self.driver = xtremio.XtremIOFibreChannelDriver( - configuration=config) - self.driver.client = xtremio.XtremIOClient4(config, - config. - xtremio_cluster_name) - + configuration=self.config) self.data = CommonData() def test_initialize_terminate_connection(self, req): req.side_effect = xms_request + self.driver.client = xtremio.XtremIOClient4( + self.config, self.config.xtremio_cluster_name) + self.driver.create_volume(self.data.test_volume) map_data = self.driver.initialize_connection(self.data.test_volume, self.data.connector) @@ -586,3 +584,43 @@ class EMCXIODriverFibreChannelTestCase(test.TestCase): self.driver.terminate_connection(self.data.test_volume, self.data.connector) self.driver.delete_volume(self.data.test_volume) + + def test_race_on_terminate_connection(self, req): + """Test for race conditions on num_of_mapped_volumes. + + This test confirms that num_of_mapped_volumes won't break even if we + receive a NotFound exception when retrieving info on a specific + mapping, as that specific mapping could have been deleted between + the request to get the list of exiting mappings and the request to get + the info on one of them. + """ + req.side_effect = xms_request + self.driver.client = xtremio.XtremIOClient3( + self.config, self.config.xtremio_cluster_name) + # We'll wrap num_of_mapped_volumes, we'll store here original method + original_method = self.driver.client.num_of_mapped_volumes + + def fake_num_of_mapped_volumes(*args, **kwargs): + # Add a nonexistent mapping + mappings = [{'href': 'volumes/1'}, {'href': 'volumes/12'}] + + # Side effects will be: 1st call returns the list, then we return + # data for existing mappings, and on the nonexistent one we added + # we return NotFound + side_effect = [{'lun-maps': mappings}, + {'content': xms_data['lun-maps'][1]}, + exception.NotFound] + + with mock.patch.object(self.driver.client, 'req', + side_effect=side_effect): + return original_method(*args, **kwargs) + + self.driver.create_volume(self.data.test_volume) + map_data = self.driver.initialize_connection(self.data.test_volume, + self.data.connector) + self.assertEqual(1, map_data['data']['target_lun']) + with mock.patch.object(self.driver.client, 'num_of_mapped_volumes', + side_effect=fake_num_of_mapped_volumes): + self.driver.terminate_connection(self.data.test_volume, + self.data.connector) + self.driver.delete_volume(self.data.test_volume) diff --git a/cinder/volume/drivers/emc/xtremio.py b/cinder/volume/drivers/emc/xtremio.py index 6de8ca191..aacc6ad27 100644 --- a/cinder/volume/drivers/emc/xtremio.py +++ b/cinder/volume/drivers/emc/xtremio.py @@ -198,21 +198,35 @@ class XtremIOClient3(XtremIOClient): def find_lunmap(self, ig_name, vol_name): try: - for lm_link in self.req('lun-maps')['lun-maps']: - idx = lm_link['href'].split('/')[-1] - lm = self.req('lun-maps', idx=int(idx))['content'] - if lm['ig-name'] == ig_name and lm['vol-name'] == vol_name: - return lm + lun_mappings = self.req('lun-maps')['lun-maps'] except exception.NotFound: raise (exception.VolumeDriverException (_("can't find lun-map, ig:%(ig)s vol:%(vol)s") % {'ig': ig_name, 'vol': vol_name})) + for lm_link in lun_mappings: + idx = lm_link['href'].split('/')[-1] + # NOTE(geguileo): There can be races so mapped elements retrieved + # in the listing may no longer exist. + try: + lm = self.req('lun-maps', idx=int(idx))['content'] + except exception.NotFound: + continue + if lm['ig-name'] == ig_name and lm['vol-name'] == vol_name: + return lm + + return None + def num_of_mapped_volumes(self, initiator): cnt = 0 for lm_link in self.req('lun-maps')['lun-maps']: idx = lm_link['href'].split('/')[-1] - lm = self.req('lun-maps', idx=int(idx))['content'] + # NOTE(geguileo): There can be races so mapped elements retrieved + # in the listing may no longer exist. + try: + lm = self.req('lun-maps', idx=int(idx))['content'] + except exception.NotFound: + continue if lm['ig-name'] == initiator: cnt += 1 return cnt