Change in vdsm[master]: Replace SPM domain locks with SANLock

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: Replace SPM domain locks with SANLock
......................................................................

Replace SPM domain locks with SANLock

Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
---
M configure.ac
M vdsm.spec.in
M vdsm/constants.py.in.in
M vdsm/storage/blockSD.py
M vdsm/storage/safelease.py
M vdsm/storage/sd.py
M vdsm/storage/sp.py
M vdsm/storage/storage_exception.py
M vdsm/vdsmd.init.in
9 files changed, 167 insertions(+), 23 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/726/1
--
To view, visit http://gerrit.ovirt.org/726
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli

Saggi Mizrahi has posted comments on this change.

Change subject: Replace SPM domain locks with SANLock
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/sd.py
Line 99: # Lock Version
Sorry for not saying this in the call but I think that from version 3 it should just be configurable in the MD which clustered lock system we want to use.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Replace SPM domain locks with SANLock
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/sd.py
Line 99: # Lock Version
First safelease will disappear in the future (no need to have a domain version 3 with safelease), second If we want to provide a way to make it configurable (how?) then we don't need a new version at all. Probably if we believe it is possible, the domain version 2 will be the one with the configurable locking system. Third, domain version 3 at the moment is something hidden and we can change its format as we want (maybe adding what you want later on).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

Saggi Mizrahi has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

Only minor issue in delete flow

....................................................
File vdsm/storage/fileVolume.py
Line 237:             self.oop.fileUtils.cleanupfiles([vol_path])
Probably you need to remove .lease file here

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 4: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm/storage/blockSD.py
Line 660:             freeSlot += 1
shouldn't this fail if all slots in _getLeasesFilePath() are exhausted?

....................................................
File vdsm/storage/safelease.py
Line 150:         return 250
is this related to MAX_HOST_ID?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

Ayal Baron has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 5: I would prefer that you didn't submit this

(7 inline comments)

....................................................
File vdsm/storage/blockVolume.py
Line 240:             cls.newVolumeLease(sdUUID, volUUID, offs)
The previous line marks the volume as 'legal' effectively committing the operation so I believe the volume lease should be initialized before writing the MD.

....................................................
File vdsm/storage/fileVolume.py
Line 193:             cls.newVolumeLease(sdUUID, volUUID, vol_path)
Same as in block (should come before newMetadata

Line 212:         vol_path = self.getVolumePath()
You can change vol_path to volPath for consistency

Line 213:         lease_path = self.__leaseVolumePath(vol_path)
why is this here and not down where you actually use it?

Line 547:             return None
why do we support vol_path being empty?
why is the variable not named volPath?

....................................................
File vdsm/storage/sd.py
Line 102: DOM_SAFELEASE_VERS = (0,2)
If 3 doesn't support safelease then how do you do the transition without stopping everything?

Line 400:     def hasVolumeLeases(self):
'has' is not really the right word here.
'supports' maybe.
But I would rather see a capabilities dictionary or something and test against that, so that we'd have a generic way for checking capabilities.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

Ayal Baron has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 6: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 6: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 6: (2 inline comments)

....................................................
File vdsm/storage/safelease.py
Line 26: import sanlock
sanlock is not (yet) in python's stdlib, contextlib is.

....................................................
File vdsm/vdsmd.init.in
Line 35: NEEDED_SERVICES="iscsid multipathd ntpd wdmd sanlock"
wdmd?!

Do you intentionally not require python-sanlock?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 6: (1 inline comment)

....................................................
File vdsm/vdsmd.init.in
Line 35: NEEDED_SERVICES="iscsid multipathd ntpd wdmd sanlock"
python-sanlock is not a service, it's a library.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 6: Verified; Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 7: Verified; Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Use SANLock for the SPM resource
......................................................................

Use SANLock for the SPM resource

Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
---
M configure.ac
M vdsm.spec.in
M vdsm/constants.py.in
M vdsm/storage/blockSD.py
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/safelease.py
M vdsm/storage/sd.py
M vdsm/vdsmd.init.in
9 files changed, 197 insertions(+), 14 deletions(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

Adam Litke has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 7: (1 inline comment)

....................................................
File vdsm.spec.in
Line 12: 
How does this user and group get created?  In order to install the latest built RPMs, I had to manually create them.  This adds an additional step to preparing a host for vdsm.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Use SANLock for the SPM resource

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Use SANLock for the SPM resource
......................................................................

Patch Set 7:

Adam, https://bugzilla.redhat.com/show_bug.cgi?id=785824 tracks this issue.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3958a171e35d65544e0f2c3593daaf7daf8750ef
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi