Change in vdsm[master]: BZ#870024 - Make sure during upgrade that lease files have ... - vdsm-patches

List overview All Threads
Download

newer

Change in vdsm[master]: BZ#870024 - Make sure during upgrade that lease files have ...

older

Change in vdsm[master]:...

Change in vdsm[master]: Sending...

Haim Ateya

27 Nov 2012 27 Nov '12

9:47 a.m.

Haim Ateya has uploaded a new change for review.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

BZ#870024 - Make sure during upgrade that lease files have correct permission

Fix a bug where we tried to initialize sanlock lock before calling the function to set proper permissions.

Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Signed-off-by: Haim Ateya hateya@redhat.com --- M vdsm/storage/imageRepository/formatConverter.py 1 file changed, 4 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/9517/1

diff --git a/vdsm/storage/imageRepository/formatConverter.py b/vdsm/storage/imageRepository/formatConverter.py index 190f9b6..c7952d8 100644 --- a/vdsm/storage/imageRepository/formatConverter.py +++ b/vdsm/storage/imageRepository/formatConverter.py @@ -90,6 +90,10 @@ log = logging.getLogger('Storage.v3DomainConverter') log.debug("Starting conversion for domain %s", domain.sdUUID)

+ if domain.getStorageType() in sd.FILE_DOMAIN_TYPES: + log.debug("Setting permissions for domain %s", domain.sdUUID) + domain.setMetadataPermissions() + log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID) newClusterLock = safelease.SANLock(domain.sdUUID, domain.getIdsFilePath(), domain.getLeasesFilePath()) @@ -100,8 +104,6 @@

V2META_SECTORSIZE = 512

- if domain.getStorageType() in sd.FILE_DOMAIN_TYPES: - domain.setMetadataPermissions()

def v3ResetMetaVolSize(vol): # BZ811880 Verifiying that the volume size is the same size advertised

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

Gerrit-MessageType: newchange Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com

Show replies by date

Jenkins CI RO

27 Nov 27 Nov

9:47 a.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/205/ (1/2)

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

Jenkins CI RO

9:47 a.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/171/ (2/2)

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

Haim Ateya

9:48 a.m.

Haim Ateya has posted comments on this change.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

Patch Set 1: Verified

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

Jenkins CI RO

9:49 a.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

Patch Set 1: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/171/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/205/ : FAILURE

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

Federico Simoncelli

9:58 a.m.

Federico Simoncelli has posted comments on this change.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

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

(3 inline comments)

Temporary -1 only to have the commit message fixed.

.................................................... Commit Message Line 3: AuthorDate: 2012-11-27 17:33:22 +0200 Line 4: Commit: Haim Ateya hateya@redhat.com Line 5: CommitDate: 2012-11-27 17:44:17 +0200 Line 6: Line 7: BZ#870024 - Make sure during upgrade that lease files have correct permission Remove the bz from here. The correct format would be something like:

upgrade: make sure that the lease files have correct permission Line 8: Line 9: Fix a bug where we tried to initialize sanlock lock before calling Line 10: the function to set proper permissions. Line 11:

Line 7: BZ#870024 - Make sure during upgrade that lease files have correct permission Line 8: Line 9: Fix a bug where we tried to initialize sanlock lock before calling Line 10: the function to set proper permissions. Line 11: Add the Bug-Url here:

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=870024 Line 12: Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f

.................................................... File vdsm/storage/imageRepository/formatConverter.py Line 89: def v3DomainConverter(repoPath, hostId, domain, isMsd): Line 90: log = logging.getLogger('Storage.v3DomainConverter') Line 91: log.debug("Starting conversion for domain %s", domain.sdUUID) Line 92: Line 93: if domain.getStorageType() in sd.FILE_DOMAIN_TYPES: As the previous review I'm against this check (I'd prefer setMetadataPermissions to be a noop in BlockStorageDomain). Anyway it's not the time to make this change. Line 94: log.debug("Setting permissions for domain %s", domain.sdUUID) Line 95: domain.setMetadataPermissions() Line 96: Line 97: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID)

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

ybronhei＠redhat.com

10:03 a.m.

Yaniv Bronhaim has posted comments on this change.

Change subject: BZ#870024 - Make sure during upgrade that lease files have correct permission ......................................................................

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

(3 inline comments)

.................................................... File vdsm/storage/imageRepository/formatConverter.py Line 84: else: Line 85: log.debug("Skipping the upgrade to tag based metadata version %s " Line 86: "for the domain %s", targetVersion, domain.sdUUID) Line 87: Line 88: raising pep8 error (look on jenkins report - http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/171/violations/file/vdsm/stora... ) Line 89: def v3DomainConverter(repoPath, hostId, domain, isMsd): Line 90: log = logging.getLogger('Storage.v3DomainConverter') Line 91: log.debug("Starting conversion for domain %s", domain.sdUUID) Line 92:

Line 89: def v3DomainConverter(repoPath, hostId, domain, isMsd): Line 90: log = logging.getLogger('Storage.v3DomainConverter') Line 91: log.debug("Starting conversion for domain %s", domain.sdUUID) Line 92: Line 93: if domain.getStorageType() in sd.FILE_DOMAIN_TYPES: I'm not sure that I agree to that. we don't need stub functions just to avoid one condition.. the getStorageType function is exactly for that. risky, but make it easier to read the code .. Line 94: log.debug("Setting permissions for domain %s", domain.sdUUID) Line 95: domain.setMetadataPermissions() Line 96: Line 97: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID)

Line 97: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID) Line 98: newClusterLock = safelease.SANLock(domain.sdUUID, domain.getIdsFilePath(), Line 99: domain.getLeasesFilePath()) Line 100: newClusterLock.initLock() Line 101: Don't you want to put that after the SANLock's lock? Line 102: log.debug("Acquiring the host id %s for domain %s", hostId, domain.sdUUID) Line 103: newClusterLock.acquireHostId(hostId, async=False) Line 104: Line 105: V2META_SECTORSIZE = 512

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

Jenkins CI RO

10:15 a.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/207/ (2/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

10:15 a.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/173/ (1/2)

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

Jenkins CI RO

10:17 a.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/173/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/207/ : SUCCESS

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

ybronhei＠redhat.com

10:27 a.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Yaniv Bronhaim has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

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

You see :) now jenkis likes your patch. Hit the verified please

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

Federico Simoncelli

10:34 a.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Federico Simoncelli has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2: Looks good to me, approved

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Haim Ateya

10:54 a.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Haim Ateya has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2: Verified

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Dan Kenigsberg

1:43 p.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Dan Kenigsberg has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2: (1 inline comment)

.................................................... File vdsm/storage/imageRepository/formatConverter.py Line 91: log.debug("Starting conversion for domain %s", domain.sdUUID) Line 92: Line 93: if domain.getStorageType() in sd.FILE_DOMAIN_TYPES: Line 94: log.debug("Setting permissions for domain %s", domain.sdUUID) Line 95: domain.setMetadataPermissions() luckily, on file domains we can do such things before taking an exclusive lock. Line 96: Line 97: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID) Line 98: newClusterLock = safelease.SANLock(domain.sdUUID, domain.getIdsFilePath(), Line 99: domain.getLeasesFilePath())

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

smizrahi＠redhat.com

1:45 p.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Saggi Mizrahi has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

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

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Federico Simoncelli

2:12 p.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Federico Simoncelli has posted comments on this change.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

Patch Set 2: (1 inline comment)

.................................................... File vdsm/storage/imageRepository/formatConverter.py Line 91: log.debug("Starting conversion for domain %s", domain.sdUUID) Line 92: Line 93: if domain.getStorageType() in sd.FILE_DOMAIN_TYPES: Line 94: log.debug("Setting permissions for domain %s", domain.sdUUID) Line 95: domain.setMetadataPermissions() no Dan, we already have the exclusive lock, you can upgrade a pool/domain only if you're the SPM, we are under the safelease lock here (still version < 3). Line 96: Line 97: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID) Line 98: newClusterLock = safelease.SANLock(domain.sdUUID, domain.getIdsFilePath(), Line 99: domain.getLeasesFilePath())

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

Gerrit-MessageType: comment Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Dan Kenigsberg

2:23 p.m.

New subject: Change in vdsm[master]: upgrade: make sure that the lease files have correct permiss...

Dan Kenigsberg has submitted this change and it was merged.

Change subject: upgrade: make sure that the lease files have correct permission ......................................................................

upgrade: make sure that the lease files have correct permission

Fix a bug where we tried to initialize sanlock lock before calling the function to set proper permissions.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=870024 Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Signed-off-by: Haim Ateya hateya@redhat.com --- M vdsm/storage/imageRepository/formatConverter.py 1 file changed, 4 insertions(+), 3 deletions(-)

Approvals: Yaniv Bronhaim: Looks good to me, but someone else must approve Saggi Mizrahi: Looks good to me, but someone else must approve Haim Ateya: Verified Federico Simoncelli: Looks good to me, approved Dan Kenigsberg:

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

Gerrit-MessageType: merged Gerrit-Change-Id: If28c48d2aeedd092ff3b8100dd64cd8ba673e94f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Haim Ateya hateya@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

4235

Age (days ago)

4235

Last active (days ago)

vdsm-patches@lists.fedorahosted.org

16 comments

6 participants

tags (0)

participants (6)

Dan Kenigsberg
Federico Simoncelli
Haim Ateya
Jenkins CI RO
smizrahi＠redhat.com
ybronhei＠redhat.com