From d26108dcf150752aaf2c07bfb446ddedeabfb12b Mon Sep 17 00:00:00 2001 From: Kostas Ladopoulos Date: Fri, 19 Dec 2014 10:27:32 +0000 Subject: [PATCH 1/2] CA-128095: Unhandled exception in LVMoISCSISR.load() There is a big try/except block in load() that does not handle all possible exceptions. If an exception is not handled in place, it is logged and the function tries to continue immediately after the try/except block, almost certainly resulting in an "IndexError" exception, as the self.iscsiSRs list is empty. Now, all such exceptions are reraised. The fix for CA-129230 should be present as well, to ensure that no exception escapes SRCommand.run(). Signed-off-by: Kostas Ladopoulos --- drivers/LVHDoISCSISR.py | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/LVHDoISCSISR.py b/drivers/LVHDoISCSISR.py index c09244c91..76625744f 100755 --- a/drivers/LVHDoISCSISR.py +++ b/drivers/LVHDoISCSISR.py @@ -153,6 +153,7 @@ def load(self, sr_uuid): self.session.xenapi.PBD.set_device_config(pbd, dconf) except: util.logException("LVHDoISCSISR.load") + raise self.iscsi = self.iscsiSRs[0] # Be extremely careful not to throw exceptions here since this function From 27a75029a6545cb37e2853ec21dc780b7c834e51 Mon Sep 17 00:00:00 2001 From: Kostas Ladopoulos Date: Fri, 19 Dec 2014 10:47:05 +0000 Subject: [PATCH 2/2] CA-128095: [UnitTest] Unhandled exception in LVMoISCSISR.load() Signed-off-by: Kostas Ladopoulos --- tests/test_LVHDoISCSISR.py | 77 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/test_LVHDoISCSISR.py diff --git a/tests/test_LVHDoISCSISR.py b/tests/test_LVHDoISCSISR.py new file mode 100644 index 000000000..40d1f0364 --- /dev/null +++ b/tests/test_LVHDoISCSISR.py @@ -0,0 +1,77 @@ +import unittest +import mock +import LVHDoISCSISR + + +class RandomException(Exception): + pass + + +class NonInitingLVHDoISCSISR(LVHDoISCSISR.LVHDoISCSISR): + + """ Helper class; Creates dummy LVHDoISCSISR object. + Add attributes/methods as appropriate. + """ + + def __init__(self, extra_dconf=None, extra_params=None): + + from SRCommand import SRCommand + from DummySR import DRIVER_INFO + + self.mpath = "false" + self.dconf = { + 'target': 'target', + 'localIQN': 'localIQN', + 'targetIQN': 'targetIQN', + 'SCSIid': 'SCSIid' + } + + self.srcmd = mock.Mock(spec=SRCommand(DRIVER_INFO)) + self.srcmd.dconf = self.dconf + + self.original_srcmd = self.srcmd + + self.srcmd.params = {'command': 'command'} + + self.srcmd.dconf.update(extra_dconf or {}) + self.srcmd.params.update(extra_params or {}) + + +class TestLVHDoISCSISR(unittest.TestCase): + + @mock.patch('iscsilib.ensure_daemon_running_ok') + @mock.patch('util._convertDNS') + @mock.patch('SR.driver') + @mock.patch('util.gen_uuid') + def test_load_reraise_unhandled_exceptions( + self, + mock_util_gen_uuid, + mock_SR_driver, + mock_util__convertDNS, + mock_iscsilib_ensure_daemon_running_ok): + + """ Asserts that all unhandled exceptions are reraised. + + In LVHDoISCSISR.load() there is a big try/except block that + doesn't handle all exceptions. Specifically, it logs the + exception and tries to continue after the block, almost certainly + resulting in an "IndexError" exception because the "self.iscsiSRs" + list is empty. This test asserts that all such exceptions are + reraised. + """ + + mock_util_gen_uuid.return_value = 'deadbeef' + mock_util__convertDNS.return_value = '127.0.0.1' + mock_iscsilib_ensure_daemon_running_ok.side_effect = \ + RandomException('test_load_reraise_unhandled_exceptions') + + lvhd_o_iscsi_sr = NonInitingLVHDoISCSISR( + {'targetIQN': '*'}, + {'command': 'sr_create'} + ) + + try: + lvhd_o_iscsi_sr.load(None) + except Exception, e: + self.assertTrue(isinstance(e, RandomException)) + self.assertEqual(str(e), 'test_load_reraise_unhandled_exceptions')