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