Change in vdsm[master]: misc: Fix exception re-raising in RollbackContext

nsoffer at redhat.com nsoffer at redhat.com
Thu Jan 2 13:43:51 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: misc: Fix exception re-raising in RollbackContext
......................................................................


Patch Set 6: Code-Review-1

(1 comment)

....................................................
File vdsm/storage/misc.py
Line 708:                     exc_type, exc_value, traceback = sys.exc_info()
Line 709: 
Line 710:         # re-raise the earliest exception
Line 711:         if exc_type is not None:
Line 712:             raise exc_type, exc_value, traceback
Zhou is correct that both the original code and the path are wrong. But he is wrong about the correct fix.

First lets use Python 2.6 documentation [1] , because this the oldest version that we support.

According to the docs, we should never re-raise the original exception because it is raised automatically by Python if we return nothing in __exit__. If we want to suppress the original exception, we should return True from __exit__ - we never want to do that.

If an exception happens inside __exit__ and not handled, it will replace the original exception - we also do not want that.

So what we want to do is capture the first exception during undo. Then if an exception was not passed to the function and we captured an undo exception, we should re-raise it.

The code should be:

    def __exit__(ex_type, ex_value, traceback):
        undoExcInfo = None
        for undo, args, kwargs in self._finally:
            try:
                undo(*args, **kwargs)
            except Exception:
                if undoExcInfo is None:
                    undoExcInfo = sys.exc_info()
        if ex_type is None and undoExcInfo is not None:
            raise undoExcInfo[0], undoExcInfo[1], undoExcInfo[2]


[1] http://docs.python.org/2.6/library/stdtypes.html#contextmanager.__exit__
Line 713: 
Line 714:     def defer(self, func, *args, **kwargs):
Line 715:         self._finally.append((func, args, kwargs))
Line 716: 


-- 
To view, visit http://gerrit.ovirt.org/22860
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0052717e2307ad6fec2225b3ad5f438c5a60e1c6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list