openstack/cinder
Code Issues Proposed changes

Do not raise NEW exceptions

Browse Source 
Raising NEW exception is bad practice, because we lose TraceBack.
So all places like:

except SomeException as e:
    raise e

should be replaced by

except SomeException:
    raise

If we are doing some other actions before reraising we should
store information about exception then do all actions and then
reraise it. This is caused by eventlet bug. It lost information
about exception if it switch threads.

fixes bug 1191730
Change-Id: Ic2be96e9f03d2ca46d060caf6f6f7f713a1d6b82
This commit is contained in:
Sergey Vilgelm 2013-06-24 15:19:50 +04:00
parent 1bf8bfad29
commit 33f6d78c3a
12 changed files with 51 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
HACKING.rst
View File

@ -55,6 +55,15 @@ General
{'vol_name': vol_name,
'vol_size': vol_size}) # OKAY
- Use 'raise' instead of 'raise e' to preserve original traceback or exception being reraised::
except Exception as e:
...
raise e # BAD
except Exception:
...
raise # OKAY
Imports
-------

8
cinder/exception.py
View File

@ -24,6 +24,8 @@ SHOULD include dedicated exception logging.
"""
import sys
from oslo.config import cfg
import webob.exc
@ -105,15 +107,15 @@ class CinderException(Exception):
try:
message = self.message % kwargs
except Exception as e:
except Exception:
exc_info = sys.exc_info()
# kwargs doesn't match a variable in the message
# log the issue and the kwargs
LOG.exception(_('Exception in string format operation'))
for name, value in kwargs.iteritems():
LOG.error("%s: %s" % (name, value))
if CONF.fatal_exception_format_errors:
raise e
else:
raise exc_info[0], exc_info[1], exc_info[2]
# at least get the core message out if something happened
message = self.message

2
cinder/tests/test_netapp_nfs.py
View File

@ -386,6 +386,6 @@ class NetappDirect7modeNfsDriverTestCase(NetappDirectCmodeNfsDriverTestCase):
if isinstance(e, api.NaApiError):
pass
else:
raise e
raise
mox.VerifyAll()

4
cinder/tests/test_scality.py
View File

@ -61,7 +61,7 @@ class ScalityDriverTestCase(test.TestCase):
os.makedirs(path)
except OSError as e:
if e.errno != errno.EEXIST:
raise e
raise
def _create_fake_config(self):
open(self.TEST_CONFIG, "w+").close()
@ -75,7 +75,7 @@ class ScalityDriverTestCase(test.TestCase):
os.unlink(self.TEST_CONFIG)
except OSError as e:
if e.errno != errno.ENOENT:
raise e
raise
def _configure_driver(self):
scality.CONF.scality_sofs_config = self.TEST_CONFIG

3
cinder/tests/test_storwize_svc.py
View File

@ -24,6 +24,7 @@
Tests for the IBM Storwize family and SVC volume driver.
"""
import random
import re
import socket
@ -1610,7 +1611,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
self.assertEqual(attrs[k], v)
except exception.ProcessExecutionError as e:
if 'CMMVC7050E' not in e.stderr:
raise e
raise
def test_storwize_svc_unicode_host_and_volume_names(self):
# We'll check with iSCSI only - nothing protocol-dependednt here

5
cinder/transfer/api.py
View File

@ -27,6 +27,7 @@ from oslo.config import cfg
from cinder.db import base
from cinder import exception
from cinder.openstack.common import excutils
from cinder.openstack.common import log as logging
from cinder import quota
from cinder.volume import api as volume_api
@ -192,12 +193,12 @@ class API(base.Base):
if donor_reservations:
QUOTAS.commit(context, donor_reservations, project_id=donor_id)
LOG.info(_("Volume %s has been transferred.") % volume_id)
except Exception as exc:
except Exception:
with excutils.save_and_reraise_exception():
QUOTAS.rollback(context, reservations)
if donor_reservations:
QUOTAS.rollback(context, donor_reservations,
project_id=donor_id)
raise exc
vol_ref = self.db.volume_get(context, volume_id)
return {'id': transfer_id,

3
cinder/volume/drivers/hds/hds.py
View File

@ -24,6 +24,7 @@ from oslo.config import cfg
from xml.etree import ElementTree as ETree
from cinder import exception
from cinder.openstack.common import excutils
from cinder.openstack.common import log as logging
from cinder import utils
from cinder.volume import driver
@ -86,8 +87,8 @@ def _xml_read(root, element, check=None):
return None
except ETree.ParseError as e:
if check:
with excutils.save_and_reraise_exception():
LOG.error(_("XML exception reading parameter: %s") % element)
raise e
else:
LOG.info(_("XML exception reading parameter: %s") % element)
return None

7
cinder/volume/drivers/netapp/iscsi.py
View File

@ -22,6 +22,7 @@ This driver requires NetApp Clustered Data ONTAP or 7-mode
storage systems with installed iSCSI licenses.
"""
import sys
import time
import uuid
@ -402,12 +403,13 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
message = e.message
msg = _('Error mapping lun. Code :%(code)s, Message:%(message)s')
msg_fmt = {'code': code, 'message': message}
exc_info = sys.exc_info()
LOG.warn(msg % msg_fmt)
(igroup, lun_id) = self._find_mapped_lun_igroup(path, initiator)
if lun_id is not None:
return lun_id
else:
raise e
raise exc_info[0], exc_info[1], exc_info[2]
def _unmap_lun(self, path, initiator):
"""Unmaps a lun from given initiator."""
@ -422,12 +424,13 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
msg = _("Error unmapping lun. Code :%(code)s,"
" Message:%(message)s")
msg_fmt = {'code': e.code, 'message': e.message}
exc_info = sys.exc_info()
LOG.warn(msg % msg_fmt)
# if the lun is already unmapped
if e.code == '13115' or e.code == '9016':
pass
else:
raise e
raise exc_info[0], exc_info[1], exc_info[2]
def _find_mapped_lun_igroup(self, path, initiator, os=None):
"""Find the igroup for mapped lun with initiator."""

2
cinder/volume/drivers/netapp/nfs.py
View File

@ -397,7 +397,7 @@ class NetAppDirect7modeNfsDriver (NetAppDirectNfsDriver):
except NaApiError as e:
if e.code != 'UnknownCloneId':
self._clear_clone(clone_id)
raise e
raise
def _get_actual_path_for_export(self, export_path):
"""Gets the actual path on the filer for export path."""

8
cinder/volume/drivers/san/hp/hp_3par_common.py
View File

@ -36,6 +36,8 @@ hp3par_api_url, hp3par_username, hp3par_password
for credentials to talk to the REST service on the 3PAR
array.
"""
import base64
import json
import paramiko
@ -52,10 +54,12 @@ from oslo.config import cfg
from cinder import context
from cinder import exception
from cinder.openstack.common import excutils
from cinder.openstack.common import log as logging
from cinder import utils
from cinder.volume import volume_types
LOG = logging.getLogger(__name__)
hp3par_opts = [
@ -313,9 +317,9 @@ exit
"attempts : '%(command)s'") %
{'total_attempts': total_attempts, 'command': command})
raise paramiko.SSHException(msg)
except Exception as e:
except Exception:
with excutils.save_and_reraise_exception():
LOG.error(_("Error running ssh command: %s") % command)
raise e
def _delete_3par_host(self, hostname):
self._cli_run('removehost %s' % hostname, None)

5
cinder/volume/drivers/san/san.py
View File

@ -27,6 +27,7 @@ from eventlet import greenthread
from oslo.config import cfg
from cinder import exception
from cinder.openstack.common import excutils
from cinder.openstack.common import log as logging
from cinder import utils
from cinder.volume.driver import ISCSIDriver
@ -143,9 +144,9 @@ class SanISCSIDriver(ISCSIDriver):
stderr="Error running SSH command",
cmd=command)
except Exception as e:
except Exception:
with excutils.save_and_reraise_exception():
LOG.error(_("Error running SSH command: %s") % command)
raise e
def ensure_export(self, context, volume):
"""Synchronously recreates an export for a logical volume."""

4
cinder/volume/drivers/scality.py
View File

@ -16,6 +16,7 @@
Scality SOFS Volume Driver.
"""
import errno
import os
import urllib2
@ -28,6 +29,7 @@ from cinder.image import image_utils
from cinder.openstack.common import log as logging
from cinder.volume import driver
LOG = logging.getLogger(__name__)
volume_opts = [
@ -85,7 +87,7 @@ class ScalityDriver(driver.VolumeDriver):
os.makedirs(path)
except OSError as e:
if e.errno != errno.EEXIST:
raise e
raise
def _mount_sofs(self):
config = CONF.scality_sofs_config