Change in vdsm[master]: StorageDomainManifest: Move clusterLock into Manifest
alitke at redhat.com
alitke at redhat.com
Thu Sep 10 19:08:42 UTC 2015
Adam Litke has posted comments on this change.
Change subject: StorageDomainManifest: Move clusterLock into Manifest
......................................................................
Patch Set 12:
(8 comments)
https://gerrit.ovirt.org/#/c/43549/12/tests/manifest_tests.py
File tests/manifest_tests.py:
Line 64: self.assertEquals(mdpath, manifest.getMDPath())
Line 65:
Line 66: def test_getmetaparam(self):
Line 67: with namedTemporaryDir() as tmpdir:
Line 68: metadata = {sd.DMDK_VERSION: 3}
> Why not use FakeMetadata object for consistency? (for another patch?)
Good point, but not related to this patch.
Line 69: manifest = make_filesd_manifest(tmpdir, metadata)
Line 70: metadata[sd.DMDK_SDUUID] = manifest.sdUUID
Line 71: self.assertEquals(manifest.sdUUID,
Line 72: manifest.getMetaParam(sd.DMDK_SDUUID))
https://gerrit.ovirt.org/#/c/43549/12/tests/storagetestlib.py
File tests/storagetestlib.py:
Line 38: def make_blocksd_manifest(tmpdir=None, metadata=None, sduuid=None):
Line 39: if sduuid is None:
Line 40: sduuid = str(uuid.uuid4())
Line 41: if metadata is None:
Line 42: metadata = FakeMetadata([(sd.DMDK_VERSION, 3)])
> You can use:
Done
Line 43: manifest = blockSD.BlockStorageDomainManifest(sduuid, metadata)
Line 44: if tmpdir is not None:
Line 45: manifest.domaindir = tmpdir
Line 46: os.makedirs(os.path.join(manifest.domaindir, sduuid, sd.DOMAIN_IMAGES))
Line 77: sduuid = str(uuid.uuid4())
Line 78: domain_path = os.path.join(tmpdir, sduuid)
Line 79: make_file(get_metafile_path(domain_path))
Line 80: if metadata is None:
Line 81: metadata = FakeMetadata([(sd.DMDK_VERSION, 3)])
> You can use:
Done
Line 82: manifest = fileSD.FileStorageDomainManifest(domain_path, metadata)
Line 83: os.makedirs(os.path.join(manifest.domaindir, sduuid, sd.DOMAIN_IMAGES))
Line 84: return manifest
Line 85:
https://gerrit.ovirt.org/#/c/43549/12/vdsm/storage/imageRepository/formatConverter.py
File vdsm/storage/imageRepository/formatConverter.py:
Line 117: log.debug("Setting permissions for domain %s", domain.sdUUID)
Line 118: domain.setMetadataPermissions()
Line 119:
Line 120: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID)
Line 121: newClusterLock = domain._manifest._makeDomainLock(targetVersion)
> Why no not use domain._makeClusterLock()?
Because we removed it. This is the only consumer of that logic so I decided to call into the manifest. In general this module is badly behaved as it accesses many protected class members.
Line 122: newClusterLock.initLock()
Line 123:
Line 124: log.debug("Acquiring the host id %s for domain %s", hostId, domain.sdUUID)
Line 125: newClusterLock.acquireHostId(hostId, async=False)
https://gerrit.ovirt.org/#/c/43549/12/vdsm/storage/sd.py
File vdsm/storage/sd.py:
Line 555
Line 556
Line 557
Line 558
Line 559
> Lets move it to the manifest, so we don't have to make the manifest domainL
Done
Line 304: def __init__(self, sdUUID, domaindir, metadata):
Line 305: self.sdUUID = sdUUID
Line 306: self.domaindir = domaindir
Line 307: self.replaceMetadata(metadata)
Line 308: self.domainLock = self._makeDomainLock()
> Why public? old lock was private.
Done
Line 309:
Line 310: @property
Line 311: def oop(self):
Line 312: return oop.getProcessPool(self.sdUUID)
Line 428: DEFAULT_LEASE_PARAMS[DMDK_LOCK_RENEWAL_INTERVAL_SEC],
Line 429: DEFAULT_LEASE_PARAMS[DMDK_LEASE_TIME_SEC],
Line 430: DEFAULT_LEASE_PARAMS[DMDK_LEASE_RETRIES],
Line 431: DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC],
Line 432: )
> For another patch, move leaseParams after lock class selection.
ok
Line 433:
Line 434: try:
Line 435: clusterLockClass = self._domainLockTable[domVersion]
Line 436: except KeyError:
Line 431: DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC],
Line 432: )
Line 433:
Line 434: try:
Line 435: clusterLockClass = self._domainLockTable[domVersion]
> If you renamed _makeClusterLock to _makeDomainLock, you should also rename
Done
Line 436: except KeyError:
Line 437: raise se.UnsupportedDomainVersion(domVersion)
Line 438:
Line 439: return clusterLockClass(self.sdUUID, self.getIdsFilePath(),
--
To view, visit https://gerrit.ovirt.org/43549
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c019a7fed6ba793d2b3a2459af221b1abbff2a3
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list