Change in vdsm[master]: task: streamlining lock acquiring api

nsoffer at redhat.com nsoffer at redhat.com
Thu Aug 14 22:04:03 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: task: streamlining lock acquiring api
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/29729/3/vdsm/storage/task.py
File vdsm/storage/task.py:

Line 1325:             i += 1
Line 1326:         return s
Line 1327: 
Line 1328:     @misc.logskip("ResourceManager")
Line 1329:     def getExclusiveLock(self, namespace, *resources, **options):
> *resources is misleading because it makes you believe that you're locking m
To make it more clear, we can rename "*resources" to "*names".

In the old way, we had the logic of building a resource name spread all over the place. So if we don't put this here, where to do suggest to put it?

I think this is exactly the duty of the locking infra, to provide a clean interface and hide the details of how resource name is built. The user intention is to lock volume X and Y, not the string "X_Y".
Line 1330:         self._acquireResource(resourceManager.LockType.exclusive, namespace,
Line 1331:                               resources, options)
Line 1332: 
Line 1333:     @misc.logskip("ResourceManager")


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iac7eaa5ad577279a602651ab2df9e53ee58ac9d5
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yoav Kleinberger <ykleinbe at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list