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

ybronhei at redhat.com ybronhei at redhat.com
Tue Nov 27 16:03:34 UTC 2012


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/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 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

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


More information about the vdsm-patches mailing list