Nir Soffer has posted comments on this change. Change subject: StorageDomainManifest: move logBlkSize ......................................................................
Patch Set 13: (6 comments) https://gerrit.ovirt.org/#/c/41996/13/tests/manifest_tests.py File tests/manifest_tests.py: Line 57 Line 58 Line 59 Line 60 Line 61 path -> tmpdir Lets add md parameter, so the caller can control the metadata object: md = {} manifest = self.make_manifest(tmpdir, md=md) md[sd.DMDK_SDUUID] = manifest.sdUUID Now we don't need to add setter to the real code. Line 60: Line 61: def test_getmetaparam(self): Line 62: with namedTemporaryDir() as tmpdir: Line 63: manifest = self.make_manifest(tmpdir) Line 64: # XXX: Replace this with a setter once it's available See bellow. Line 65: manifest._metadata.update({sd.DMDK_SDUUID: manifest.sdUUID}) Line 66: self.assertEquals(manifest.sdUUID, Line 67: manifest.getMetaParam(sd.DMDK_SDUUID)) Line 68: Line 116: Line 117: def test_getmetaparam(self): Line 118: with namedTemporaryDir() as tmpdir: Line 119: manifest = self.make_manifest(tmpdir) Line 120: # XXX: Replace this with a setter once it's available See above Line 121: manifest._metadata.update({sd.DMDK_SDUUID: manifest.sdUUID}) Line 122: self.assertEquals(manifest.sdUUID, Line 123: manifest.getMetaParam(sd.DMDK_SDUUID)) Line 124: Line 124: Line 125: def test_getblocksize(self): Line 126: with namedTemporaryDir() as tmpdir: Line 127: manifest = self.make_manifest(tmpdir) Line 128: # Test that we get 512 when no sizes are set Lets make this two tests - one the case you test here, other for the TODO, so the comment is not needed. Line 129: self.assertEquals(512, manifest.logBlkSize) Line 130: self.assertEquals(512, manifest.phyBlkSize) Line 131: # TODO: Set new sizes and verify Line 132: https://gerrit.ovirt.org/#/c/41996/13/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 400 Line 401 Line 402 Line 403 Line 404 Initialize xxxBlkSize vars Never add variable to a class at runtime, it makes the class hard to understand. Line 416: # *blkSize keys may be missing from metadata only for domains that Line 417: # existed before the introduction of the keys. Line 418: # Such domains supported only 512 sizes Line 419: self.logBlkSize = 512 Line 420: self.phyBlkSize = 512 We must initialize these in __init__, either to the default 512, or to None. Line 421: Line 422: def getReadDelay(self): Line 423: stats = misc.readspeed(lvm.lvPath(self.sdUUID, sd.METADATA), 4096) Line 424: return stats['seconds'] -- To view, visit https://gerrit.ovirt.org/41996 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b9b20e9c8c20fa5c151f7c86337bc54ab526e11 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches