openstack/cinder
Code Issues Proposed changes

Use volume driver specific execeptions.

Browse Source 
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
This commit is contained in:
Ronen Kat
2012-08-06 10:11:10 +03:00
parent e6e1e98369
commit 3905a99026
10 changed files with 118 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cinder/exception.py
View File

@@ -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")

2
cinder/tests/test_HpSanISCSIDriver.py
View File

@@ -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

2
cinder/tests/test_storwize_svc.py
View File

@@ -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

2
cinder/tests/test_volume.py
View File

@@ -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)

22
cinder/volume/driver.py
View File

@@ -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"""

6
cinder/volume/manager.py
View File

@@ -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:

43
cinder/volume/netapp.py
View File

@@ -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)

27
cinder/volume/san.py
View File

@@ -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.

63
cinder/volume/storwize_svc.py
View File

@@ -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."""

34
cinder/volume/xensm.py
View File

@@ -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'])