Change in vdsm[master]: Add ResourceLocker
nsoffer at redhat.com
nsoffer at redhat.com
Sun Jul 26 16:32:22 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: Add ResourceLocker
......................................................................
Patch Set 3:
(2 comments)
Looks very nice, sad that we cannot use this now.
https://gerrit.ovirt.org/#/c/43554/3/vdsm/storage/resourceLocker.py
File vdsm/storage/resourceLocker.py:
Line 43: self._get_lock(res).acquire(exclusive=False)
Line 44:
Line 45: def release_lock(self, res):
Line 46: try:
Line 47: lock = self._resources[res]['lock']
This hides resource manager error (missing 'lock' key) and mask it as client error (accessing not existent resource).
Should be:
try:
entry = self._resources[res]
except KeyError:
# This will raise now only if res is missing
...
# This will fail loudly with KeyError
entry['lock'].release()
...
Line 48: except KeyError:
Line 49: raise ResourceDoesNotExistError()
Line 50: lock.release()
Line 51: self._put_lock(res)
Line 70: with self._lock:
Line 71: try:
Line 72: entry = self._resources[res]
Line 73: except KeyError:
Line 74: entry = self._resources[res] = dict(lock=RWLock(), refcount=0)
I would create the dict with refcount=1 in this case, and increment the refcount in else: block, because refcount=0 is something that we should never have.
Line 75: entry['refcount'] += 1
Line 76: return entry['lock']
Line 77:
Line 78: def _put_lock(self, res):
--
To view, visit https://gerrit.ovirt.org/43554
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I29589889efd39920d72979e89405ff6cf36b803b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list