Saggi Mizrahi has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation ......................................................................
Patch Set 7: (4 comments) There seem to be 3 things going on in this patch Please split up or at least enumerate in the commit message .................................................... Commit Message Line 3: AuthorDate: 2013-07-15 10:16:45 +0300 Line 4: Commit: Yeela Kaplan <ykap...@redhat.com> Line 5: CommitDate: 2013-09-08 14:20:27 +0200 Line 6: Line 7: Remove redundant check that causes lvm cache to refresh every volume creation When? Why is it redundant Line 8: Line 9: Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Line 10: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=979193 .................................................... File vdsm/storage/blockVolume.py Line 161: cls.__putMetadata((sdUUID, int(offs)), Line 162: {"NONE": "#" * (sd.METASIZE - 10)}) Line 163: Line 164: @classmethod Line 165: def _createUnderlyingVol(cls, dom, volUUID, size, preallocate, volPath): The fact that it returns the volume size seems, at least to me, very odd. It's probably the last thing I expect being returned. You should probably return all the new volume metadata Line 166: if preallocate == volume.SPARSE_VOL: Line 167: volSize = "%s" % config.get("irs", "volume_utilization_chunk_mb") Line 168: else: Line 169: volSize = "%s" % (size / 2 / 1024) .................................................... File vdsm/storage/fileVolume.py Line 115: if oop.getProcessPool(sdUUID).os.path.lexists(metaPath): Line 116: oop.getProcessPool(sdUUID).os.unlink(metaPath) Line 117: Line 118: @classmethod Line 119: def _createUnderlyingVol(cls, dom, volUUID, size, preallocate, volPath): ditto Line 120: volSize = int(size) * BLOCK_SIZE Line 121: Line 122: try: Line 123: oop.getProcessPool(dom.sdUUID).truncateFile(volPath, volSize, .................................................... File vdsm/storage/lvm.py Line 462: for lvName in staleLVs: Line 463: log.warning("Removing stale lv: %s/%s", vgName, lvName) Line 464: self._lvs.pop((vgName, lvName), None) Line 465: Line 466: log.debug("Finished lvs reload") When did I start? If you don't want to add one at the start you can change it to "lvs reloaded" Line 467: Line 468: return updatedLVs Line 469: Line 470: def _reloadAllLvs(self): -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
