Change in vdsm[master]: rwlock: Add simpler RWLock

nsoffer at redhat.com nsoffer at redhat.com
Tue Jul 14 16:43:54 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: rwlock: Add simpler RWLock
......................................................................


Patch Set 10:

(3 comments)

https://gerrit.ovirt.org/#/c/42908/10/lib/vdsm/rwlock.py
File lib/vdsm/rwlock.py:

Line 28: 
Line 29:     This lock is not prefering writers or readers. Each acquire request is put
Line 30:     into a queue, and will be served in the order of the request.
Line 31: 
Line 32:     To acquire a write lock, use acquire_write(), followed by relase() when you
> typo: release
Done
Line 33:     are done.
Line 34: 
Line 35:         lock.acquire_write()
Line 36: 


Line 75:     Lock promotion or demotion is forbidden and will raise RuntimeError.
Line 76:     """
Line 77: 
Line 78:     def __init__(self):
Line 79:         self.shared = Context(self, False)
> very minor: perhaps using an (unneeded) named argument makes the code a tin
This is little more clear, but I don't like to call positional arguments in kwargs calling style, and I don't like to make exclusive a kwarg, since it should not have a default.

I'll keep the code as is for now.
Line 80:         self.exclusive = Context(self, True)
Line 81:         self._lock = threading.Lock()
Line 82:         self._waiters = []
Line 83:         self._readers = set()


Line 90:         if me is self._writer:
Line 91:             raise RuntimeError("Recursive lock is forbidden")
Line 92:         with self._lock:
Line 93:             if self._writer or self._readers or self._waiters:
Line 94:                 self._wait(True)
> same (and below on line 105). Maybe
Same issue with calling positional args as kwargs.
Line 95:             self._writer = me
Line 96: 
Line 97:     def acquire_read(self):
Line 98:         me = threading.current_thread()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2466c137c89598772fb46347eb02195916883cac
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list