Nir Soffer has posted comments on this change.

Change subject: storage: Introduce guarded utilities
......................................................................


Patch Set 9:

(4 comments)

https://gerrit.ovirt.org/#/c/61435/9/lib/vdsm/storage/guarded.py
File lib/vdsm/storage/guarded.py:

Line 54:     Errors are handled as gracefully as possible with any acquired 
locks being
Line 55:     released in the proper order.
Line 56:     """
Line 57: 
Line 58:     def __init__(self, *entities):
I think we should remove the entity layer, and accept list of locks.

This will allow using same code other places that create today tuples (ns, 
name, mode) and sort them - for example, see sp.py:cloneImageStructure.

In code that have entities (like copy_data), we will do this:

    with guarded.context(src.locks + dst.locks):
        ...
Line 59:         """
Line 60:         Receives a list of objects that each provide a 'locks' 
property.
Line 61:         'locks' must contain an iterable of guarded.LockInterface 
objects.
Line 62:         """


Line 61:         'locks' must contain an iterable of guarded.LockInterface 
objects.
Line 62:         """
Line 63:         locks = set()
Line 64:         for entity in entities:
Line 65:             locks.update(set(entity.locks))
We don't need to create a temporary set, any iterable works.
Line 66:         self._locks = sorted(locks)
Line 67:         self._held_locks = []
Line 68: 
Line 69:     def __enter__(self):


Line 62:         """
Line 63:         locks = set()
Line 64:         for entity in entities:
Line 65:             locks.update(set(entity.locks))
Line 66:         self._locks = sorted(locks)
If we remove the entity thing, this becomes:

    self._locks = sorted(set(locks))
Line 67:         self._held_locks = []
Line 68: 
Line 69:     def __enter__(self):
Line 70:         for lock in self._locks:


Line 85:             self._release()
Line 86:         except ReleaseError:
Line 87:             if exc_type is not None:
Line 88:                 log.exception("Error releasing locks")
Line 89:                 six.reraise(exc_type, exc_val, exc_tb)
We don't need this, please check again my comment from version 6, maybe you 
uploaded the wrong version?
Line 90:             raise
Line 91: 
Line 92:     def _release(self):
Line 93:         errors = []


-- 
To view, visit https://gerrit.ovirt.org/61435
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0a204818d44b6205515277f4c2834cb2b7a057
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to