From: nikeshm Date: Thu, 27 Aug 2015 09:59:02 +0000 (+0530) Subject: Fix volume copy for 'virtual' volumes in DotHill X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2899d4624e2b07933bbe1237cd25ab73f27f53f6;p=openstack-build%2Fcinder-build.git Fix volume copy for 'virtual' volumes in DotHill Dot Hill volumes allocated from virtual storage pools require different handling than volumes allocated from vdisks when cloning volumes or snapshots. This change ensures that the correct API calls are used based on the dothill_backend_type property declared in the cinder.conf. Change-Id: If2ac15d1d6426948ff439815e85411fbbb95215b Closes-Bug: #1489303 Co-Authored-By: Chris Maio Co-Authored-By: Lakshman --- diff --git a/cinder/tests/unit/test_dothill.py b/cinder/tests/unit/test_dothill.py index 6b55fa0af..cb9e70721 100644 --- a/cinder/tests/unit/test_dothill.py +++ b/cinder/tests/unit/test_dothill.py @@ -315,7 +315,8 @@ class TestFCDotHillCommon(test.TestCase): self.assertEqual(None, self.common.do_setup(None)) mock_backend_exists.assert_called_with(self.common.backend_name, self.common.backend_type) - mock_owner_info.assert_called_with(self.common.backend_name) + mock_owner_info.assert_called_with(self.common.backend_name, + self.common.backend_type) def test_vol_name(self): self.assertEqual(encoded_volid, self.common._get_vol_name(vol_id)) @@ -413,7 +414,8 @@ class TestFCDotHillCommon(test.TestCase): mock_copy.assert_called_with(encoded_volid, 'vqqqqqqqqqqqqqqqqqqq', - 0, self.common.backend_name) + self.common.backend_name, + self.common.backend_type) @mock.patch.object(dothill.DotHillClient, 'copy_volume') @mock.patch.object(dothill.DotHillClient, 'backend_stats') @@ -434,7 +436,8 @@ class TestFCDotHillCommon(test.TestCase): self.assertEqual(None, ret) mock_copy.assert_called_with('sqqqqqqqqqqqqqqqqqqq', 'vqqqqqqqqqqqqqqqqqqq', - 0, self.common.backend_name) + self.common.backend_name, + self.common.backend_type) @mock.patch.object(dothill.DotHillClient, 'extend_volume') def test_extend_volume(self, mock_extend): diff --git a/cinder/volume/drivers/dothill/dothill_client.py b/cinder/volume/drivers/dothill/dothill_client.py index c1da83132..8f62e7ac8 100644 --- a/cinder/volume/drivers/dothill/dothill_client.py +++ b/cinder/volume/drivers/dothill/dothill_client.py @@ -66,15 +66,25 @@ class DotHillClient(object): def _assert_response_ok(self, tree): """Parses the XML returned by the device to check the return code. - Raises a DotHillRequestError error if the return code is not 0. + Raises a DotHillRequestError error if the return code is not 0 + or if the return code is None. """ + # Get the return code for the operation, raising an exception + # if it is not present. return_code = tree.findtext(".//PROPERTY[@name='return-code']") - if return_code and return_code != '0': - raise exception.DotHillRequestError( - message=tree.findtext(".//PROPERTY[@name='response']")) - elif not return_code: + if not return_code: raise exception.DotHillRequestError(message="No status found") + # If no error occurred, just return. + if return_code == '0': + return + + # Format a message for the status code. + msg = "%s (%s)" % (tree.findtext(".//PROPERTY[@name='response']"), + return_code) + + raise exception.DotHillRequestError(message=msg) + def _build_request_url(self, path, *args, **kargs): url = self._base_url + path if kargs: @@ -95,6 +105,7 @@ class DotHillClient(object): """ url = self._build_request_url(path, *args, **kargs) + LOG.debug("DotHill Request URL: %s", url) headers = {'dataType': 'api', 'sessionKey': self._session_key} try: xml = requests.get(url, headers=headers, verify=self.ssl_verify) @@ -233,16 +244,16 @@ class DotHillClient(object): return [port['target-id'] for port in self.get_active_target_ports() if port['port-type'] == "iSCSI"] - def copy_volume(self, src_name, dest_name, same_bknd, dest_bknd_name): + def linear_copy_volume(self, src_name, dest_name, dest_bknd_name): + """Copy a linear volume.""" + self._request("/volumecopy", dest_name, dest_vdisk=dest_bknd_name, source_volume=src_name, prompt='yes') - if same_bknd == 0: - return - + # The copy has started; now monitor until the operation completes. count = 0 while True: tree = self._request("/show/volumecopy-status") @@ -267,6 +278,47 @@ class DotHillClient(object): time.sleep(5) + def copy_volume(self, src_name, dest_name, dest_bknd_name, + backend_type='virtual'): + """Copy a linear or virtual volume.""" + + if backend_type == 'linear': + return self.linear_copy_volume(src_name, dest_name, dest_bknd_name) + # Copy a virtual volume to another in the same pool. + self._request("/copy/volume", src_name, name=dest_name) + LOG.debug("Volume copy of source_volume: %(src_name)s to " + "destination_volume: %(dest_name)s started.", + {'src_name': src_name, 'dest_name': dest_name, }) + + # Loop until this volume copy is no longer in progress. + while self.volume_copy_in_progress(src_name): + time.sleep(5) + + # Once the copy operation is finished, check to ensure that + # the volume was not deleted because of a subsequent error. An + # exception will be raised if the named volume is not present. + self._request("/show/volumes", dest_name) + LOG.debug("Volume copy of source_volume: %(src_name)s to " + "destination_volume: %(dest_name)s completed.", + {'src_name': src_name, 'dest_name': dest_name, }) + + def volume_copy_in_progress(self, src_name): + """Check if a volume copy is in progress for the named volume.""" + + # 'show volume-copies' always succeeds, even if none in progress. + tree = self._request("/show/volume-copies") + + # Find 0 or 1 job(s) with source volume we're interested in + q = "OBJECT[PROPERTY[@name='source-volume']/text()='%s']" % src_name + joblist = tree.xpath(q) + if len(joblist) == 0: + return False + LOG.debug("Volume copy of volume: %(src_name)s is " + "%(pc)s percent completed.", + {'src_name': src_name, + 'pc': joblist[0].findtext("PROPERTY[@name='progress']"), }) + return True + def _check_host(self, host): host_status = -1 tree = self._request("/show/hosts") @@ -325,8 +377,12 @@ class DotHillClient(object): tree = self._request("/show/system") return tree.findtext(".//PROPERTY[@name='midplane-serial-number']") - def get_owner_info(self, backend_name): - tree = self._request("/show/vdisks", backend_name) + def get_owner_info(self, backend_name, backend_type): + if backend_type == 'linear': + tree = self._request("/show/vdisks", backend_name) + else: + tree = self._request("/show/pools", backend_name) + return tree.findtext(".//PROPERTY[@name='owner']") def modify_volume_name(self, old_name, new_name): diff --git a/cinder/volume/drivers/dothill/dothill_common.py b/cinder/volume/drivers/dothill/dothill_common.py index 2c395137d..96b098e70 100644 --- a/cinder/volume/drivers/dothill/dothill_common.py +++ b/cinder/volume/drivers/dothill/dothill_common.py @@ -88,11 +88,8 @@ class DotHillCommon(object): def do_setup(self, context): self.client_login() self._validate_backend() - if (self.backend_type == "linear" or - (self.backend_type == "virtual" and - self.backend_name not in ['A', 'B'])): - self._get_owner_info(self.backend_name) - self._get_serial_number() + self._get_owner_info() + self._get_serial_number() self.client_logout() def client_login(self): @@ -115,8 +112,9 @@ class DotHillCommon(object): def _get_serial_number(self): self.serialNumber = self.client.get_serial_number() - def _get_owner_info(self, backend_name): - self.owner = self.client.get_owner_info(backend_name) + def _get_owner_info(self): + self.owner = self.client.get_owner_info(self.backend_name, + self.backend_type) def _validate_backend(self): if not self.client.backend_exists(self.backend_name, @@ -208,11 +206,6 @@ class DotHillCommon(object): raise exception.VolumeAttached(volume_id=volume['id']) def create_cloned_volume(self, volume, src_vref): - if self.backend_type == "virtual" and self.backend_name in ["A", "B"]: - msg = _("Create volume from volume(clone) does not have support " - "for virtual pool A and B.") - LOG.error(msg) - raise exception.InvalidInput(reason=msg) self.get_volume_stats(True) self._assert_enough_space_for_copy(volume['size']) self._assert_source_detached(src_vref) @@ -228,7 +221,8 @@ class DotHillCommon(object): self.client_login() try: - self.client.copy_volume(orig_name, dest_name, 0, self.backend_name) + self.client.copy_volume(orig_name, dest_name, + self.backend_name, self.backend_type) except exception.DotHillRequestError as ex: LOG.exception(_LE("Cloning of volume %s failed."), volume['source_volid']) @@ -237,11 +231,6 @@ class DotHillCommon(object): self.client_logout() def create_volume_from_snapshot(self, volume, snapshot): - if self.backend_type == "virtual" and self.backend_name in ["A", "B"]: - msg = _('Create volume from snapshot does not have support ' - 'for virtual pool A and B.') - LOG.error(msg) - raise exception.InvalidInput(reason=msg) self.get_volume_stats(True) self._assert_enough_space_for_copy(volume['size']) LOG.debug("Creating Volume from snapshot %(source_id)s to " @@ -252,7 +241,8 @@ class DotHillCommon(object): dest_name = self._get_vol_name(volume['id']) self.client_login() try: - self.client.copy_volume(orig_name, dest_name, 0, self.backend_name) + self.client.copy_volume(orig_name, dest_name, + self.backend_name, self.backend_type) except exception.DotHillRequestError as ex: LOG.exception(_LE("Create volume failed from snapshot: %s"), snapshot['id']) @@ -303,14 +293,11 @@ class DotHillCommon(object): backend_stats = self.client.backend_stats(self.backend_name, self.backend_type) pool.update(backend_stats) - if (self.backend_type == "linear" or - (self.backend_type == "virtual" and - self.backend_name not in ['A', 'B'])): - pool['location_info'] = ('%s:%s:%s:%s' % - (src_type, - self.serialNumber, - self.backend_name, - self.owner)) + pool['location_info'] = ('%s:%s:%s:%s' % + (src_type, + self.serialNumber, + self.backend_name, + self.owner)) pool['pool_name'] = self.backend_name except exception.DotHillRequestError: err = (_("Unable to get stats for backend_name: %s") % @@ -489,7 +476,8 @@ class DotHillCommon(object): self.client_login() try: - self.client.copy_volume(source_name, dest_name, 1, dest_back_name) + self.client.copy_volume(source_name, dest_name, + dest_back_name, self.backend_type) self.client.delete_volume(source_name) self.client.modify_volume_name(dest_name, source_name) return (True, None)