Nir Soffer has posted comments on this change.
Change subject: clusterLock: Acquire, release, and inquire volume leases
......................................................................
Patch Set 8:
(2 comments)
Mostly ok, but if we add new api, we need to rename to old api so both of them make sense.
Since this not public api, there is no reason to keep the old names.
I think we should have:
- acquireClusterLock
- releaseClusterLock
- inquireClusterLock
- acquireResource
- releaseResource
- inquireResource
What do you think?
https://gerrit.ovirt.org/#/c/38622/8/vdsm/storage/clusterlock.py
File vdsm/storage/clusterlock.py:
Line 297: # timestamp, sanlock will return HOST_UNKNOWN and then HOST_LIVE or
Line 298: # HOST_FAIL.
Line 299: return HOST_STATUS_FREE
Line 300:
Line 301: def _acquire(self, resource, lockDisk, shared=False):
Please move _private method to the end of the class, or to the end of the section using
the private method. It should be easy to understand what is the public interface of the
class by reading the code from top to bottom.
Line 302: with nested(self._lock, SANLock._sanlock_lock):
Line 303: self.log.info("Acquiring resource %s, shared=%s",
resource, shared)
Line 304:
Line 305: while True:
Line 337: "(id: %s)", self._sdUUID, hostId)
Line 338:
Line 339: def acquireResource(self, resource, lockDisk, shared=False):
Line 340: self._acquire(resource, lockDisk, shared)
Line 341: res, owners = self._inquire(resource, lockDisk)
Why do we need to call _inquire() here?
Add same logging as in acquire before and after the operation.
Line 342:
Line 343: def _inquire(self, resource, lockDisk):
Line 344: res = sanlock.read_resource(*lockDisk[0])
Line 345: owners = sanlock.read_resource_owners(self._sdUUID, resource, lockDisk)
--
To view, visit
https://gerrit.ovirt.org/38622
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icca901ccd27358767c023cd55b7a3823531d2a5a
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes