From 3905a9902606871bbe92fe7c48480db3349ea316 Mon Sep 17 00:00:00 2001 From: Ronen Kat Date: Mon, 6 Aug 2012 10:11:10 +0300 Subject: [PATCH] Use volume driver specific execeptions. Change generic use of exception.Error in the cinder/volume directory to specific exceptions. The exceptions used in this patch are: exception.VolumeBackendAPIException exception.InvalidInput exception.InvalidVolume exceptio.VolumeAtatched Patch includes updates to the appropriate tests as well. Change-Id: I10407ff3f5babe64e88f445d3529269b7665ee16 --- cinder/exception.py | 6 ++- cinder/tests/test_HpSanISCSIDriver.py | 2 +- cinder/tests/test_storwize_svc.py | 2 +- cinder/tests/test_volume.py | 2 +- cinder/volume/driver.py | 22 ++++++---- cinder/volume/manager.py | 6 ++- cinder/volume/netapp.py | 43 +++++++++--------- cinder/volume/san.py | 27 +++++++----- cinder/volume/storwize_svc.py | 63 ++++++++++++++------------- cinder/volume/xensm.py | 34 ++++++++++----- 10 files changed, 118 insertions(+), 89 deletions(-) diff --git a/cinder/exception.py b/cinder/exception.py index 5cf10a15f..c9c5b079d 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -222,6 +222,10 @@ class VolumeUnattached(Invalid): message = _("Volume %(volume_id)s is not attached to anything") +class VolumeAttached(Invalid): + message = _("Volume %(volume_id)s is still attached, detach volume first.") + + class InvalidKeypair(Invalid): message = _("Keypair data is invalid") @@ -940,4 +944,4 @@ class CouldNotFetchImage(CinderException): class VolumeBackendAPIException(CinderException): message = _("Bad or unexpected response from the storage volume " - "backend API: data=%(data)s") + "backend API: %(data)s") diff --git a/cinder/tests/test_HpSanISCSIDriver.py b/cinder/tests/test_HpSanISCSIDriver.py index 5bde6032b..151f0a21a 100644 --- a/cinder/tests/test_HpSanISCSIDriver.py +++ b/cinder/tests/test_HpSanISCSIDriver.py @@ -208,5 +208,5 @@ class HpSanISCSITestCase(test.TestCase): def test_cliq_error(self): try: self.driver._cliq_run_xml("testError", {}) - except exception.Error: + except exception.VolumeBackendAPIException: pass diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index c7dfad79c..909985784 100644 --- a/cinder/tests/test_storwize_svc.py +++ b/cinder/tests/test_storwize_svc.py @@ -811,7 +811,7 @@ class StorwizeSVCDriverTestCase(test.TestCase): orig_pool = getattr(storwize_svc.FLAGS, "storwize_svc_volpool_name") no_exist_pool = "i-dont-exist-%s" % random.randint(10000, 99999) storwize_svc.FLAGS.storwize_svc_volpool_name = no_exist_pool - self.assertRaises(exception.InvalidParameterValue, + self.assertRaises(exception.InvalidInput, self.driver.check_for_setup_error) storwize_svc.FLAGS.storwize_svc_volpool_name = orig_pool diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 33271b8d9..dd5f0470a 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -187,7 +187,7 @@ class VolumeTestCase(test.TestCase): self.assertEqual(vol['mountpoint'], mountpoint) self.assertEqual(vol['instance_uuid'], instance_uuid) - self.assertRaises(exception.Error, + self.assertRaises(exception.VolumeAttached, self.volume.delete_volume, self.context, volume_id) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index f465af3a7..4ec73db75 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -103,8 +103,9 @@ class VolumeDriver(object): run_as_root=True) volume_groups = out.split() if not FLAGS.volume_group in volume_groups: - raise exception.Error(_("volume group %s doesn't exist") + exception_message = (_("volume group %s doesn't exist") % FLAGS.volume_group) + raise exception.VolumeBackendAPIException(data=exception_message) def _create_volume(self, volume_name, sizestr): self._try_execute('lvcreate', '-L', sizestr, '-n', @@ -379,9 +380,9 @@ class ISCSIDriver(VolumeDriver): location = self._do_iscsi_discovery(volume) if not location: - raise exception.Error(_("Could not find iSCSI export " - " for volume %s") % - (volume['name'])) + raise exception.InvalidVolume(_("Could not find iSCSI export " + " for volume %s") % + (volume['name'])) LOG.debug(_("ISCSI Discovery: Found %s") % (location)) properties['target_discovered'] = True @@ -500,8 +501,9 @@ class RBDDriver(VolumeDriver): (stdout, stderr) = self._execute('rados', 'lspools') pools = stdout.split("\n") if not FLAGS.rbd_pool in pools: - raise exception.Error(_("rbd has no pool %s") % - FLAGS.rbd_pool) + exception_message = (_("rbd has no pool %s") % + FLAGS.rbd_pool) + raise exception.VolumeBackendAPIException(data=exception_message) def create_volume(self, volume): """Creates a logical volume.""" @@ -574,9 +576,13 @@ class SheepdogDriver(VolumeDriver): # use it and just check if 'running' is in the output. (out, err) = self._execute('collie', 'cluster', 'info') if not 'running' in out.split(): - raise exception.Error(_("Sheepdog is not working: %s") % out) + exception_message = (_("Sheepdog is not working: %s") % out) + raise exception.VolumeBackendAPIException( + data=exception_message) + except exception.ProcessExecutionError: - raise exception.Error(_("Sheepdog is not working")) + exception_message = _("Sheepdog is not working") + raise exception.VolumeBackendAPIException(data=exception_message) def create_volume(self, volume): """Creates a sheepdog volume""" diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index dfd046461..28b55cf67 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -153,9 +153,11 @@ class VolumeManager(manager.SchedulerDependentManager): context = context.elevated() volume_ref = self.db.volume_get(context, volume_id) if volume_ref['attach_status'] == "attached": - raise exception.Error(_("Volume is still attached")) + # Volume is still attached, need to detach first + raise exception.VolumeAttached(volume_id=volume_id) if volume_ref['host'] != self.host: - raise exception.Error(_("Volume is not local to this node")) + raise exception.InvalidVolume( + reason=_("Volume is not local to this node")) self._reset_stats() try: diff --git a/cinder/volume/netapp.py b/cinder/volume/netapp.py index 31da652c2..e091e8760 100644 --- a/cinder/volume/netapp.py +++ b/cinder/volume/netapp.py @@ -76,8 +76,8 @@ class NetAppISCSIDriver(driver.ISCSIDriver): if 'failed' == response.Status: name = request.Name reason = response.Reason - msg = _('API %(name)sfailed: %(reason)s') - raise exception.Error(msg % locals()) + msg = _('API %(name)s failed: %(reason)s') + raise exception.VolumeBackendAPIException(data=msg % locals()) def _create_client(self, wsdl_url, login, password, hostname, port): """ @@ -106,7 +106,7 @@ class NetAppISCSIDriver(driver.ISCSIDriver): 'netapp_storage_service'] for flag in required_flags: if not getattr(FLAGS, flag, None): - raise exception.Error(_('%s is not set') % flag) + raise exception.InvalidInput(reason=_('%s is not set') % flag) def do_setup(self, context): """ @@ -156,8 +156,9 @@ class NetAppISCSIDriver(driver.ISCSIDriver): events = self._get_job_progress(job_id) for event in events: if event.EventStatus == 'error': - raise exception.Error(_('Job failed: %s') % - (event.ErrorMessage)) + msg = (_('Job failed: %s') % + (event.ErrorMessage)) + raise exception.VolumeBackendAPIException(data=msg) if event.EventType == 'job-end': return events time.sleep(5) @@ -237,7 +238,8 @@ class NetAppISCSIDriver(driver.ISCSIDriver): AssumeConfirmation=True) except (suds.WebFault, Exception): server.DatasetEditRollback(EditLockId=lock_id) - raise exception.Error(_('Failed to provision dataset member')) + msg = _('Failed to provision dataset member') + raise exception.VolumeBackendAPIException(data=msg) lun_id = None @@ -249,7 +251,8 @@ class NetAppISCSIDriver(driver.ISCSIDriver): lun_id = event.ProgressLunInfo.LunPathId if not lun_id: - raise exception.Error(_('No LUN was created by the provision job')) + msg = _('No LUN was created by the provision job') + raise exception.VolumeBackendAPIException(data=msg) def _remove_destroy(self, name, project): """ @@ -258,8 +261,8 @@ class NetAppISCSIDriver(driver.ISCSIDriver): """ lun_id = self._get_lun_id(name, project) if not lun_id: - raise exception.Error(_("Failed to find LUN ID for volume %s") % - (name)) + msg = (_("Failed to find LUN ID for volume %s") % (name)) + raise exception.VolumeBackendAPIException(data=msg) member = self.client.factory.create('DatasetMemberParameter') member.ObjectNameOrId = lun_id @@ -278,7 +281,7 @@ class NetAppISCSIDriver(driver.ISCSIDriver): except (suds.WebFault, Exception): server.DatasetEditRollback(EditLockId=lock_id) msg = _('Failed to remove and delete dataset member') - raise exception.Error(msg) + raise exception.VolumeBackendAPIException(data=msg) def create_volume(self, volume): """Driver entry point for creating a new volume""" @@ -431,7 +434,7 @@ class NetAppISCSIDriver(driver.ISCSIDriver): lun_id = self._get_lun_id(name, project) if not lun_id: msg = _("Failed to find LUN ID for volume %s") - raise exception.Error(msg % name) + raise exception.VolumeBackendAPIException(data=msg % name) return {'provider_location': lun_id} def ensure_export(self, context, volume): @@ -600,30 +603,30 @@ class NetAppISCSIDriver(driver.ISCSIDriver): initiator_name = connector['initiator'] lun_id = volume['provider_location'] if not lun_id: - msg = _("No LUN ID for volume %s") - raise exception.Error(msg % volume['name']) + msg = _("No LUN ID for volume %s") % volume['name'] + raise exception.VolumeBackendAPIException(data=msg) lun = self._get_lun_details(lun_id) if not lun: msg = _('Failed to get LUN details for LUN ID %s') - raise exception.Error(msg % lun_id) + raise exception.VolumeBackendAPIException(data=msg % lun_id) lun_num = self._ensure_initiator_mapped(lun.HostId, lun.LunPath, initiator_name) host = self._get_host_details(lun.HostId) if not host: msg = _('Failed to get host details for host ID %s') - raise exception.Error(msg % lun.HostId) + raise exception.VolumeBackendAPIException(data=msg % lun.HostId) portal = self._get_target_portal_for_host(host.HostId, host.HostAddress) if not portal: msg = _('Failed to get target portal for filer: %s') - raise exception.Error(msg % host.HostName) + raise exception.VolumeBackendAPIException(data=msg % host.HostName) iqn = self._get_iqn_for_host(host.HostId) if not iqn: msg = _('Failed to get target IQN for filer: %s') - raise exception.Error(msg % host.HostName) + raise exception.VolumeBackendAPIException(data=msg % host.HostName) properties = {} properties['target_discovered'] = False @@ -654,12 +657,12 @@ class NetAppISCSIDriver(driver.ISCSIDriver): initiator_name = connector['initiator'] lun_id = volume['provider_location'] if not lun_id: - msg = _('No LUN ID for volume %s') - raise exception.Error(msg % (volume['name'])) + msg = _('No LUN ID for volume %s') % volume['name'] + raise exception.VolumeBackendAPIException(data=msg) lun = self._get_lun_details(lun_id) if not lun: msg = _('Failed to get LUN details for LUN ID %s') - raise exception.Error(msg % (lun_id)) + raise exception.VolumeBackendAPIException(data=msg % (lun_id)) self._ensure_initiator_unmapped(lun.HostId, lun.LunPath, initiator_name) diff --git a/cinder/volume/san.py b/cinder/volume/san.py index 8df612caf..0448f8092 100644 --- a/cinder/volume/san.py +++ b/cinder/volume/san.py @@ -111,7 +111,8 @@ class SanISCSIDriver(cinder.volume.driver.ISCSIDriver): username=FLAGS.san_login, pkey=privatekey) else: - raise exception.Error(_("Specify san_password or san_private_key")) + msg = _("Specify san_password or san_private_key") + raise exception.InvalidInput(reason=msg) return ssh def _execute(self, *cmd, **kwargs): @@ -149,12 +150,12 @@ class SanISCSIDriver(cinder.volume.driver.ISCSIDriver): """Returns an error if prerequisites aren't met.""" if not self.run_local: if not (FLAGS.san_password or FLAGS.san_private_key): - raise exception.Error(_('Specify san_password or ' - 'san_private_key')) + raise exception.InvalidInput( + reason=_('Specify san_password or san_private_key')) # The san_ip must always be set, because we use it for the target if not (FLAGS.san_ip): - raise exception.Error(_("san_ip must be set")) + raise exception.InvalidInput(reason=_("san_ip must be set")) def _collect_lines(data): @@ -224,8 +225,8 @@ class SolarisISCSIDriver(SanISCSIDriver): if "View Entry:" in out: return True - - raise exception.Error("Cannot parse list-view output: %s" % (out)) + msg = _("Cannot parse list-view output: %s") % out + raise exception.VolumeBackendAPIException(data=msg) def _get_target_groups(self): """Gets list of target groups from host.""" @@ -318,8 +319,9 @@ class SolarisISCSIDriver(SanISCSIDriver): luid = items[0].strip() return luid - raise Exception(_('LUID not found for %(zfs_poolname)s. ' - 'Output=%(out)s') % locals()) + msg = _('LUID not found for %(zfs_poolname)s. ' + 'Output=%(out)s') % locals() + raise exception.VolumeBackendAPIException(data=msg) def _is_lu_created(self, volume): luid = self._get_luid(volume) @@ -459,7 +461,7 @@ class HpSanISCSIDriver(SanISCSIDriver): msg = (_("Malformed response to CLIQ command " "%(verb)s %(cliq_args)s. Result=%(out)s") % locals()) - raise exception.Error(msg) + raise exception.VolumeBackendAPIException(data=msg) result_code = response_node.attrib.get("result") @@ -467,7 +469,7 @@ class HpSanISCSIDriver(SanISCSIDriver): msg = (_("Error running CLIQ command %(verb)s %(cliq_args)s. " " Result=%(out)s") % locals()) - raise exception.Error(msg) + raise exception.VolumeBackendAPIException(data=msg) return result_xml @@ -497,7 +499,7 @@ class HpSanISCSIDriver(SanISCSIDriver): msg = (_("Unexpected number of virtual ips for cluster " " %(cluster_name)s. Result=%(_xml)s") % locals()) - raise exception.Error(msg) + raise exception.VolumeBackendAPIException(data=msg) def _cliq_get_volume_info(self, volume_name): """Gets the volume info, including IQN""" @@ -600,7 +602,8 @@ class HpSanISCSIDriver(SanISCSIDriver): def local_path(self, volume): # TODO(justinsb): Is this needed here? - raise exception.Error(_("local_path not supported")) + msg = _("local_path not supported") + raise exception.VolumeBackendAPIException(data=msg) def initialize_connection(self, volume, connector): """Assigns the volume to a server. diff --git a/cinder/volume/storwize_svc.py b/cinder/volume/storwize_svc.py index 4dbed4a1c..bb34d58d1 100644 --- a/cinder/volume/storwize_svc.py +++ b/cinder/volume/storwize_svc.py @@ -149,9 +149,9 @@ class StorwizeSVCDriver(san.SanISCSIDriver): 'err': str(err)}) search_text = '!%s!' % getattr(FLAGS, 'storwize_svc_volpool_name') if search_text not in out: - raise exception.InvalidParameterValue( - err=_('pool %s doesn\'t exist') - % getattr(FLAGS, 'storwize_svc_volpool_name')) + raise exception.InvalidInput( + reason=(_('pool %s doesn\'t exist') + % getattr(FLAGS, 'storwize_svc_volpool_name'))) storage_nodes = {} # Get the iSCSI names of the Storwize/SVC nodes @@ -320,63 +320,64 @@ class StorwizeSVCDriver(san.SanISCSIDriver): 'storwize_svc_volpool_name'] for flag in required_flags: if not getattr(FLAGS, flag, None): - raise exception.InvalidParameterValue( - err=_('%s is not set') % flag) + raise exception.InvalidInput( + reason=_('%s is not set') % flag) # Ensure that either password or keyfile were set if not (getattr(FLAGS, 'san_password', None) or getattr(FLAGS, 'san_private_key', None)): - raise exception.InvalidParameterValue( - err=_('Password or SSH private key is required for ' - 'authentication: set either san_password or ' - 'san_private_key option')) + raise exception.InvalidInput( + reason=_('Password or SSH private key is required for ' + 'authentication: set either san_password or ' + 'san_private_key option')) # vtype should either be 'striped' or 'seq' vtype = getattr(FLAGS, 'storwize_svc_vol_vtype') if vtype not in ['striped', 'seq']: - raise exception.InvalidParameterValue( - err=_('Illegal value specified for storwize_svc_vol_vtype: ' - 'set to either \'striped\' or \'seq\'')) + raise exception.InvalidInput( + reason=_('Illegal value specified for storwize_svc_vol_vtype: ' + 'set to either \'striped\' or \'seq\'')) # Check that rsize is a number or percentage rsize = getattr(FLAGS, 'storwize_svc_vol_rsize') if not self._check_num_perc(rsize) and (rsize not in ['auto', '-1']): - raise exception.InvalidParameterValue( - err=_('Illegal value specified for storwize_svc_vol_rsize: ' - 'set to either a number or a percentage')) + raise exception.InvalidInput( + reason=_('Illegal value specified for storwize_svc_vol_rsize: ' + 'set to either a number or a percentage')) # Check that warning is a number or percentage warning = getattr(FLAGS, 'storwize_svc_vol_warning') if not self._check_num_perc(warning): - raise exception.InvalidParameterValue( - err=_('Illegal value specified for storwize_svc_vol_warning: ' - 'set to either a number or a percentage')) + raise exception.InvalidInput( + reason=_('Illegal value specified for ' + 'storwize_svc_vol_warning: ' + 'set to either a number or a percentage')) # Check that autoexpand is a boolean autoexpand = getattr(FLAGS, 'storwize_svc_vol_autoexpand') if type(autoexpand) != type(True): - raise exception.InvalidParameterValue( - err=_('Illegal value specified for ' - 'storwize_svc_vol_autoexpand: set to either ' - 'True or False')) + raise exception.InvalidInput( + reason=_('Illegal value specified for ' + 'storwize_svc_vol_autoexpand: set to either ' + 'True or False')) # Check that grainsize is 32/64/128/256 grainsize = getattr(FLAGS, 'storwize_svc_vol_grainsize') if grainsize not in ['32', '64', '128', '256']: - raise exception.InvalidParameterValue( - err=_('Illegal value specified for ' - 'storwize_svc_vol_grainsize: set to either ' - '\'32\', \'64\', \'128\', or \'256\'')) + raise exception.InvalidInput( + reason=_('Illegal value specified for ' + 'storwize_svc_vol_grainsize: set to either ' + '\'32\', \'64\', \'128\', or \'256\'')) # Check that flashcopy_timeout is numeric and 32/64/128/256 flashcopy_timeout = getattr(FLAGS, 'storwize_svc_flashcopy_timeout') if not (flashcopy_timeout.isdigit() and int(flashcopy_timeout) > 0 and int(flashcopy_timeout) <= 600): - raise exception.InvalidParameterValue( - err=_('Illegal value %s specified for ' - 'storwize_svc_flashcopy_timeout: ' - 'valid values are between 0 and 600') - % flashcopy_timeout) + raise exception.InvalidInput( + reason=_('Illegal value %s specified for ' + 'storwize_svc_flashcopy_timeout: ' + 'valid values are between 0 and 600') + % flashcopy_timeout) def do_setup(self, context): """Validate the flags.""" diff --git a/cinder/volume/xensm.py b/cinder/volume/xensm.py index d9860f659..b7f324f1c 100644 --- a/cinder/volume/xensm.py +++ b/cinder/volume/xensm.py @@ -59,7 +59,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): except Exception as ex: LOG.debug(_("Failed to create sr %s...continuing") % str(backend_ref['id'])) - raise exception.Error(_('Create failed')) + msg = _('Create failed') + raise exception.VolumeBackendAPIException(data=msg) LOG.debug(_('SR UUID of new SR is: %s') % sr_uuid) try: @@ -68,7 +69,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): dict(sr_uuid=sr_uuid)) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to update db")) + msg = _("Failed to update db") + raise exception.VolumeBackendAPIException(data=msg) else: # sr introduce, if not already done @@ -88,8 +90,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): self._create_storage_repo(context, backend) except Exception as ex: LOG.exception(ex) - raise exception.Error(_('Failed to reach backend %d') - % backend['id']) + msg = _('Failed to reach backend %d') % backend['id'] + raise exception.VolumeBackendAPIException(data=msg) def __init__(self, *args, **kwargs): """Connect to the hypervisor.""" @@ -97,7 +99,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): # This driver leverages Xen storage manager, and hence requires # hypervisor to be Xen if FLAGS.connection_type != 'xenapi': - raise exception.Error(_('XenSMDriver requires xenapi connection')) + msg = _('XenSMDriver requires xenapi connection') + raise exception.VolumeBackendAPIException(data=msg) url = FLAGS.xenapi_connection_url username = FLAGS.xenapi_connection_username @@ -107,7 +110,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): self._volumeops = volumeops.VolumeOps(session) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to initiate session")) + msg = _("Failed to initiate session") + raise exception.VolumeBackendAPIException(data=msg) super(XenSMDriver, self).__init__(execute=utils.execute, sync_exec=utils.execute, @@ -151,10 +155,12 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): self.db.sm_volume_create(self.ctxt, sm_vol_rec) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to update volume in db")) + msg = _("Failed to update volume in db") + raise exception.VolumeBackendAPIException(data=msg) else: - raise exception.Error(_('Unable to create volume')) + msg = _('Unable to create volume') + raise exception.VolumeBackendAPIException(data=msg) def delete_volume(self, volume): @@ -168,13 +174,15 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): self._volumeops.delete_volume_for_sm(vol_rec['vdi_uuid']) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to delete vdi")) + msg = _("Failed to delete vdi") + raise exception.VolumeBackendAPIException(data=msg) try: self.db.sm_volume_delete(self.ctxt, volume['id']) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to delete volume in db")) + msg = _("Failed to delete volume in db") + raise exception.VolumeBackendAPIException(data=msg) def local_path(self, volume): return str(volume['id']) @@ -207,7 +215,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): volume['id'])) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to find volume in db")) + msg = _("Failed to find volume in db") + raise exception.VolumeBackendAPIException(data=msg) # Keep the volume id key consistent with what ISCSI driver calls it xensm_properties['volume_id'] = xensm_properties['id'] @@ -218,7 +227,8 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver): xensm_properties['backend_id']) except Exception as ex: LOG.exception(ex) - raise exception.Error(_("Failed to find backend in db")) + msg = _("Failed to find backend in db") + raise exception.VolumeBackendAPIException(data=msg) params = self._convert_config_params(backend_conf['config_params']) -- 2.45.2