[Libstoragemgmt-devel] [PATCH 1/6] PY: WS clean-up

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
python_binding/lsm/_data.py | 18 +++++++++---------
python_binding/lsm/_iplugin.py | 6 ++++--
tools/lsmcli/cmdline.py | 7 ++++---
3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 3d20557..8f9e94f 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -573,15 +573,15 @@ class Pool(IData):

def __init__(self, _id, _name, _element_type, _total_space, _free_space,
_status, _status_info, _system_id, _plugin_data=None):
- self._id = _id # Identifier
- self._name = _name # Human recognisable name
- self._element_type = _element_type # What pool can be used to create
- self._total_space = _total_space # Total size
- self._free_space = _free_space # Free space available
- self._status = _status # Status of pool.
- self._status_info = _status_info # Additional status text of pool
- self._system_id = _system_id # System id this pool belongs
- self._plugin_data = _plugin_data # Plugin private data
+ self._id = _id # Identifier
+ self._name = _name # Human recognisable name
+ self._element_type = _element_type # What pool can be used to create
+ self._total_space = _total_space # Total size
+ self._free_space = _free_space # Free space available
+ self._status = _status # Status of pool.
+ self._status_info = _status_info # Additional status text of pool
+ self._system_id = _system_id # System id this pool belongs
+ self._plugin_data = _plugin_data # Plugin private data


@default_property('id', doc="Unique identifier")
diff --git a/python_binding/lsm/_iplugin.py b/python_binding/lsm/_iplugin.py
index a1c59b5..da8e8d4 100644
--- a/python_binding/lsm/_iplugin.py
+++ b/python_binding/lsm/_iplugin.py
@@ -100,7 +100,8 @@ class IPlugin(object):
"""
Returns the description and version for plug-in, raises LsmError

- Note: Make sure plugin can handle this call before plugin_register is called.
+ Note: Make sure plugin can handle this call before plugin_register is
+ called.
"""
pass

@@ -351,6 +352,7 @@ class IStorageAreaNetwork(IPlugin):
def target_ports(self, search_key=None, search_value=None, flags=0):
raise LsmError(ErrorNumber.NO_SUPPORT, "Not supported")

+
class INetworkAttachedStorage(IPlugin):
"""
Class the represents Network attached storage (Common NFS/CIFS operations)
@@ -451,7 +453,7 @@ class INetworkAttachedStorage(IPlugin):
raise LsmError(ErrorNumber.NO_SUPPORT, "Not supported")

def fs_snapshot_restore(self, fs, snapshot, files, restore_files,
- all_files=False, flags=0):
+ all_files=False, flags=0):
"""
WARNING: Destructive!

diff --git a/tools/lsmcli/cmdline.py b/tools/lsmcli/cmdline.py
index 055ba65..abdac8b 100644
--- a/tools/lsmcli/cmdline.py
+++ b/tools/lsmcli/cmdline.py
@@ -101,7 +101,7 @@ def _get_item(l, the_id, friendly_name='item', raise_error=True):

list_choices = ['VOLUMES', 'POOLS', 'FS', 'SNAPSHOTS',
'EXPORTS', "NFS_CLIENT_AUTH", 'ACCESS_GROUPS',
- 'SYSTEMS', 'DISKS', 'PLUGINS', 'TARGET_PORTS',]
+ 'SYSTEMS', 'DISKS', 'PLUGINS', 'TARGET_PORTS']

init_types = ('WWPN', 'WWNN', 'ISCSI', 'HOSTNAME', 'SAS')
init_id_help = "Access Group Initiator type: " + \
@@ -915,7 +915,7 @@ class CmdLine:
search_key = 'id'
if search_key == 'volume_id':
lsm_vol = _get_item(self.c.volumes(), args.vol,
- "Volume ID", raise_error=False)
+ "Volume ID", raise_error=False)
if lsm_vol:
return self.display_data(
self.c.access_groups_granted_to_volume(lsm_vol))
@@ -1051,7 +1051,8 @@ class CmdLine:
ss = None

self._wait_for_it(
- "fs_file_clone", self.c.fs_file_clone(fs, args.src, args.dst, ss), None)
+ "fs_file_clone", self.c.fs_file_clone(fs, args.src, args.dst, ss),
+ None)

##Converts a size parameter into the appropriate number of bytes
# @param s Size to convert to bytes handles B, K, M, G, T, P postfix

[Tony Asleson]

When you have:

try:
foo()
except Exception as e:
# Want to raise original exception
raise # 1. preferred

# raise e # 1a. Bad

try:
bar()
except Exception as e:
import sys
exception_info = sys.exec_info()

try:
bar()
except Exception as e_inner:
pass

raise exception_info[1], None, exception_info[2]

It's better to do #1 instead of #1a as #1 will preserve the
stack trace to where the actual error occurred instead of
leading back to #1a where it was simply re-thrown.

However, in some cases you need to do a bit of work that may throw
an exception itself when handling an exception, so you need to
preserve and rethrow. In this case follow #2.

See:
http://nedbatchelder.com/blog/200711/rethrowing_exceptions_in_python.html

Signed-off-by: Tony Asleson <***@redhat.com>
---
plugin/ontap/na.py | 16 ++++++++++------
plugin/ontap/ontap.py | 8 ++++++--
plugin/smispy/smis.py | 12 ++++++------
python_binding/lsm/_pluginrunner.py | 4 +++-
4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index 98ad03e..c1a2555 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
@@ -17,6 +17,7 @@

import urllib2
import socket
+import sys
from xml.etree import ElementTree
import time
from binascii import hexlify
@@ -104,12 +105,12 @@ def netapp_filer(host, username, password, timeout, command, parameters=None,
if handler.getcode() == 200:
rc = netapp_filer_parse_response(handler.read())
except urllib2.HTTPError as he:
- raise he
+ raise
except urllib2.URLError as ue:
if isinstance(ue.reason, socket.timeout):
raise FilerError(Filer.ETIMEOUT, "Connection timeout")
else:
- raise ue
+ raise
except socket.timeout:
raise FilerError(Filer.ETIMEOUT, "Connection timeout")
except SSLError as sse:
@@ -387,12 +388,15 @@ class Filer(object):
self._invoke('volume-destroy', {'name': vol_name})
except FilerError as f_error:
#If the volume was online, we will return it to same status
+ # Store the original exception information
+ exception_info = sys.exc_info()
+
if online:
try:
self._invoke('volume-online', {'name': vol_name})
except FilerError:
pass
- raise f_error
+ raise exception_info[1], None, exception_info[2]

def volume_names(self):
"""
@@ -759,7 +763,7 @@ class Filer(object):
adapter=f['adapter']))
except FilerError as na:
if na.errno != Filer.EAPILICENSE:
- raise na
+ raise

return fcp_list

@@ -770,7 +774,7 @@ class Filer(object):
return rc['node-name']
except FilerError as na:
if na.errno != Filer.EAPILICENSE:
- raise na
+ raise
return None

def interface_get_infos(self):
@@ -806,7 +810,7 @@ class Filer(object):
mac=i_info[p['interface-name']]['mac-address']))
except FilerError as na:
if na.errno != Filer.EAPILICENSE:
- raise na
+ raise

return i_list

diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 11cbf58..fba6660 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -546,10 +546,12 @@ class Ontap(IStorageAreaNetwork, INfs):
try:
self.f.lun_create(lun_name, size_bytes)
except Exception as e:
+ exception_info = sys.exc_info()
+
if flag_resize:
self._na_resize_recovery(na_vol_name,
-Ontap._size_kb_padded(size_bytes))
- raise e
+ raise exception_info[1], None, exception_info[2]

#Get the information about the newly created LUN
return None, self._get_volume(lun_name, pool.id)
@@ -596,9 +598,11 @@ class Ontap(IStorageAreaNetwork, INfs):
#if this raises an exception we need to restore the volume
self.f.lun_resize(volume.name, new_size_bytes)
except Exception as e:
+ exception_info = sys.exc_info()
+
#Put the volume back to previous size
self._na_resize_recovery(na_vol, -diff)
- raise e
+ raise exception_info[1], None, exception_info[2]
else:
self.f.lun_resize(volume.name, new_size_bytes)
self.f.volume_resize(na_vol, diff)
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 2a1e0fb..1c5764a 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -66,7 +66,7 @@ def handle_cim_errors(method):
try:
return method(*args, **kwargs)
except LsmError as lsm:
- raise lsm
+ raise
except CIMError as ce:
error_code, desc = ce

@@ -545,7 +545,7 @@ class Smis(IStorageAreaNetwork):
raise_error is False:
return None
else:
- raise ce
+ raise

for cim_xxx in cim_xxxs:
if prop_name in cim_xxx and cim_xxx[prop_name] == prop_value:
@@ -663,7 +663,7 @@ class Smis(IStorageAreaNetwork):
e[0] == pywbem.CIM_ERR_INVALID_CLASS:
pass
else:
- raise e
+ raise
if len(self.cim_rps) != 0:
break

@@ -1684,7 +1684,7 @@ class Smis(IStorageAreaNetwork):
if e[0] == pywbem.CIM_ERR_INVALID_CLASS:
return
else:
- raise e
+ raise

if len(ss):
for s in ss:
@@ -2499,7 +2499,7 @@ class Smis(IStorageAreaNetwork):
ResultClass='CIM_CompositeExtent',
PropertyList=pros_list)
else:
- raise ce
+ raise
if cim_pool_path.classname == 'LSIESG_StoragePool':
# LSI does not report error on CIM_AssociatedComponentExtent
# But they don't support it.
@@ -3312,7 +3312,7 @@ class Smis(IStorageAreaNetwork):
PropertyList=property_list,
LocalOnly=False)
else:
- raise e
+ raise

if not cim_syss:
for cim_scs_path in cim_scss_path:
diff --git a/python_binding/lsm/_pluginrunner.py b/python_binding/lsm/_pluginrunner.py
index d6b27ff..8882b53 100644
--- a/python_binding/lsm/_pluginrunner.py
+++ b/python_binding/lsm/_pluginrunner.py
@@ -65,9 +65,11 @@ class PluginRunner(object):
try:
self.plugin = plugin()
except Exception as e:
+ exception_info = sys.exc_info()
+
self.tp.send_error(0, -32099,
'Error instantiating plug-in ' + str(e))
- raise e
+ raise exception_info[1], None, exception_info[2]

except Exception:
Error(traceback.format_exc())

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
test/plugin_test.py | 17 +++++++++++++++++
test/webtest/test_results.py | 2 +-
2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/test/plugin_test.py b/test/plugin_test.py
index c3fc5a1..aaa4082 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -552,6 +552,23 @@ class TestPlugin(unittest.TestCase):
if supported(cap, [lsm.Capabilities.FS_SNAPSHOT_DELETE]):
self._fs_snapshot_delete(fs, ss)

+ def test_target_ports(self):
+ for s in self.systems:
+ cap = self.c.capabilities(s)
+
+ if supported(cap, [lsm.Capabilities.TARGET_PORTS]):
+ ports = self.c.targetports()
+
+ for p in ports:
+ self.assertTrue(p.id is not None)
+ self.assertTrue(p.port_type is not None)
+ self.assertTrue(p.service_address is not None)
+ self.assertTrue(p.network_address is not None)
+ self.assertTrue(p.physical_address is not None)
+ self.assertTrue(p.physical_name is not None)
+ self.assertTrue(p.system_id is not None)
+
+

def dump_results():
"""
diff --git a/test/webtest/test_results.py b/test/webtest/test_results.py
index 01554c1..eb8348e 100755
--- a/test/webtest/test_results.py
+++ b/test/webtest/test_results.py
@@ -113,7 +113,7 @@ def to_html(results):
methods = ['capabilities',
'systems', 'plugin_info', 'pools', 'job_status', 'job_free',
'volumes', 'volume_create', 'volume_delete', 'volume_resize',
- 'volume_replicate', 'disks']
+ 'volume_replicate', 'disks', 'target_ports']

ch = []
row_data = []

[Tony Asleson]

It wasn't very useful to just print the exception text without
the stack trace, so we are adding that too.

Note: The reason this is not in a common function is it's
possible that the common function could not be found in
the namespace and thus no debug information would be
available. Try and keep it simple and always available.

Signed-off-by: Tony Asleson <***@redhat.com>
---
plugin/nstor/nstor_lsmplugin | 5 +++--
plugin/ontap/ontap_lsmplugin | 7 ++++---
plugin/sim/sim_lsmplugin | 7 ++++---
plugin/smispy/smispy_lsmplugin | 7 ++++---
plugin/targetd/targetd_lsmplugin | 7 ++++---
plugin/v7k/v7k_lsmplugin | 7 ++++---
6 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/plugin/nstor/nstor_lsmplugin b/plugin/nstor/nstor_lsmplugin
index ce88d54..ee85f48 100755
--- a/plugin/nstor/nstor_lsmplugin
+++ b/plugin/nstor/nstor_lsmplugin
@@ -21,6 +21,7 @@
# Author: legkodymov
import sys
import syslog
+import traceback

try:
from lsm.plugin.nstor.nstor import NexentaStor
@@ -28,11 +29,11 @@ try:

if __name__ == '__main__':
PluginRunner(NexentaStor, sys.argv).run()
-except Exception as e:
+except:
#This should be quite rare, but when it does happen this is pretty
#key in understanding what happened, especially when it happens when
#running from the daemon.
- msg = "Exception: %s\n" % str(e)
+ msg = str(traceback.format_exc())
syslog.syslog(syslog.LOG_ERR, msg)
sys.stderr.write(msg)
sys.exit(1)
diff --git a/plugin/ontap/ontap_lsmplugin b/plugin/ontap/ontap_lsmplugin
index c6f5967..c2131d8 100755
--- a/plugin/ontap/ontap_lsmplugin
+++ b/plugin/ontap/ontap_lsmplugin
@@ -19,6 +19,7 @@

import sys
import syslog
+import traceback

try:
from lsm.plugin.ontap.ontap import Ontap
@@ -26,11 +27,11 @@ try:

if __name__ == '__main__':
PluginRunner(Ontap, sys.argv).run()
-except Exception as e:
+except:
#This should be quite rare, but when it does happen this is pretty
#key in understanding what happened, especially when it happens when
#running from the daemon.
- msg = "Exception: %s\n" % str(e)
+ msg = str(traceback.format_exc())
syslog.syslog(syslog.LOG_ERR, msg)
sys.stderr.write(msg)
- sys.exit(1)
+ sys.exit(1)
\ No newline at end of file
diff --git a/plugin/sim/sim_lsmplugin b/plugin/sim/sim_lsmplugin
index 9fce142..ef9e477 100755
--- a/plugin/sim/sim_lsmplugin
+++ b/plugin/sim/sim_lsmplugin
@@ -19,6 +19,7 @@

import sys
import syslog
+import traceback

try:
from lsm import PluginRunner
@@ -26,11 +27,11 @@ try:

if __name__ == '__main__':
PluginRunner(SimPlugin, sys.argv).run()
-except Exception as e:
+except Exception:
#This should be quite rare, but when it does happen this is pretty
#key in understanding what happened, especially when it happens when
#running from the daemon.
- msg = "Exception: %s\n" % str(e)
+ msg = str(traceback.format_exc())
syslog.syslog(syslog.LOG_ERR, msg)
sys.stderr.write(msg)
- sys.exit(1)
+ sys.exit(1)
\ No newline at end of file
diff --git a/plugin/smispy/smispy_lsmplugin b/plugin/smispy/smispy_lsmplugin
index 1d8bdb9..95d7084 100755
--- a/plugin/smispy/smispy_lsmplugin
+++ b/plugin/smispy/smispy_lsmplugin
@@ -19,6 +19,7 @@

import sys
import syslog
+import traceback

try:
from lsm import PluginRunner
@@ -26,11 +27,11 @@ try:

if __name__ == '__main__':
PluginRunner(SmisProxy, sys.argv).run()
-except Exception as e:
+except:
#This should be quite rare, but when it does happen this is pretty
#key in understanding what happened, especially when it happens when
#running from the daemon.
- msg = "Exception: %s\n" % str(e)
+ msg = str(traceback.format_exc())
syslog.syslog(syslog.LOG_ERR, msg)
sys.stderr.write(msg)
- sys.exit(1)
+ sys.exit(1)
\ No newline at end of file
diff --git a/plugin/targetd/targetd_lsmplugin b/plugin/targetd/targetd_lsmplugin
index 43db5ab..948f3ae 100755
--- a/plugin/targetd/targetd_lsmplugin
+++ b/plugin/targetd/targetd_lsmplugin
@@ -19,6 +19,7 @@

import sys
import syslog
+import traceback

try:
from lsm import PluginRunner
@@ -26,11 +27,11 @@ try:

if __name__ == '__main__':
PluginRunner(TargetdStorage, sys.argv).run()
-except Exception as e:
+except:
#This should be quite rare, but when it does happen this is pretty
#key in understanding what happened, especially when it happens when
#running from the daemon.
- msg = "Exception: %s\n" % str(e)
+ msg = str(traceback.format_exc())
syslog.syslog(syslog.LOG_ERR, msg)
sys.stderr.write(msg)
- sys.exit(1)
+ sys.exit(1)
\ No newline at end of file
diff --git a/plugin/v7k/v7k_lsmplugin b/plugin/v7k/v7k_lsmplugin
index 34e32dc..d67cf26 100755
--- a/plugin/v7k/v7k_lsmplugin
+++ b/plugin/v7k/v7k_lsmplugin
@@ -18,6 +18,7 @@
# Author: Deepak C Shetty (***@linux.vnet.ibm.com)
import sys
import syslog
+import traceback

try:
from lsm import PluginRunner
@@ -25,11 +26,11 @@ try:

if __name__ == '__main__':
PluginRunner(IbmV7k, sys.argv).run()
-except Exception as e:
+except:
#This should be quite rare, but when it does happen this is pretty
#key in understanding what happened, especially when it happens when
#running from the daemon.
- msg = "Exception: %s\n" % str(e)
+ msg = str(traceback.format_exc())
syslog.syslog(syslog.LOG_ERR, msg)
sys.stderr.write(msg)
- sys.exit(1)
+ sys.exit(1)
\ No newline at end of file

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
python_binding/lsm/_pluginrunner.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/python_binding/lsm/_pluginrunner.py b/python_binding/lsm/_pluginrunner.py
index e4a2552..d6b27ff 100644
--- a/python_binding/lsm/_pluginrunner.py
+++ b/python_binding/lsm/_pluginrunner.py
@@ -41,7 +41,8 @@ class PluginRunner(object):
work.
"""

- def _is_number(self, val):
+ @staticmethod
+ def _is_number(val):
"""
Returns True if val is an integer.
"""
@@ -53,7 +54,7 @@ class PluginRunner(object):

def __init__(self, plugin, args):
self.cmdline = False
- if len(args) == 2 and self._is_number(args[1]):
+ if len(args) == 2 and PluginRunner._is_number(args[1]):
try:
fd = int(args[1])
self.tp = _transport.TransPort(

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
test/plugin_test.py | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 695ac45..c3fc5a1 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -175,15 +175,13 @@ class TestProxy(object):
rc = getattr(self.o, _proxy_method_name)(*args, **kwargs)
TestProxy.log_result(_proxy_method_name,
dict(rc=True, stack_trace=None, msg=None))
- except Exception as e:
-
- if isinstance(e, lsm.LsmError) and \
- e.code != lsm.ErrorNumber.NO_SUPPORT:
+ except lsm.LsmError as le:
+ if le.code != lsm.ErrorNumber.NO_SUPPORT:
TestProxy.log_result(
_proxy_method_name,
dict(rc=False,
stack_trace=traceback.format_exc(),
- msg=str(e)))
+ msg=str(le)))
raise

# If the job can do async, we will block looping on it.