Federico Simoncelli has posted comments on this change. Change subject: sparsify: integrating virt-sparsify into vdsm ......................................................................
Patch Set 7: Code-Review-1 (7 comments) Please also verify that it works. Remove the "Draft" and hit "Verified" (once you've successfully tested it either with vdsClient or with engine). Thanks! http://gerrit.ovirt.org/#/c/28328/7/vdsm/storage/image.py File vdsm/storage/image.py: Line 524: dstVolUUID)) Line 525: self.log.info(argsStr) Line 526: Line 527: srcVolClass = sdCache.produce(srcSdUUID).getVolumeClass() Line 528: dstVolClass = sdCache.produce(dstSdUUID).getVolumeClass() Both srcVolClass and dstVolClass are probably unneeded. See below. Line 529: Line 530: # Get source, destination and temporary volumes Line 531: srcVolume = srcVolClass(self.repoPath, srcSdUUID, srcImgUUID, Line 532: srcVolUUID) Line 528: dstVolClass = sdCache.produce(dstSdUUID).getVolumeClass() Line 529: Line 530: # Get source, destination and temporary volumes Line 531: srcVolume = srcVolClass(self.repoPath, srcSdUUID, srcImgUUID, Line 532: srcVolUUID) This should be: srcVolume = sdCache.produce(srcSdUUID).produceVolume(srcImgUUID, srcVolUUID) Line 533: dstVolume = dstVolClass(self.repoPath, dstSdUUID, dstImgUUID, Line 534: dstVolUUID) Line 535: Line 536: if not srcVolume.isSparse() or not dstVolume.isSparse(): Line 530: # Get source, destination and temporary volumes Line 531: srcVolume = srcVolClass(self.repoPath, srcSdUUID, srcImgUUID, Line 532: srcVolUUID) Line 533: dstVolume = dstVolClass(self.repoPath, dstSdUUID, dstImgUUID, Line 534: dstVolUUID) This should be: dstVolume = sdCache.produce(dstSdUUID).produceVolume(dstImgUUID, dstVolUUID) Line 535: Line 536: if not srcVolume.isSparse() or not dstVolume.isSparse(): Line 537: raise se.VolumeNotSparse() Line 538: Line 536: if not srcVolume.isSparse() or not dstVolume.isSparse(): Line 537: raise se.VolumeNotSparse() Line 538: Line 539: baseVolume = srcVolClass(self.repoPath, srcSdUUID, srcImgUUID, Line 540: srcVolume.getParent()) This should be: baseVolume = sdCache.produce(srcSdUUID).produceVolume(srcImgUUID, srcVolume.getParent()) Line 541: Line 542: srcVolume.prepare() Line 543: dstVolume.prepare() Line 544: Line 543: dstVolume.prepare() Line 544: Line 545: # Extend sizes Line 546: srcVolume.extend(srcVolume.getSize()) Line 547: dstVolume.extend(srcVolume.getSize()) It would be better to put both inside the "try:" below. Line 548: Line 549: # Sparsify Line 550: try: Line 551: virtsparsify.sparsify(baseVolume.getVolumePath(), Line 554: except Exception as e: Line 555: raise se.CannotSparsifyVolume(srcVolUUID) Line 556: finally: Line 557: srcVolume.teardown(sdUUID = srcSdUUID, volUUID = srcVolUUID) Line 558: dstVolume.teardown(sdUUID = dstSdUUID, volUUID = dstVolUUID) Either (no spaces near "="): srcVolume.teardown(sdUUID=srcSdUUID, volUUID=srcVolUUID) dstVolume.teardown(sdUUID=dstSdUUID, volUUID=dstVolUUID) or: srcVolume.teardown(srcSdUUID, srcVolUUID) dstVolume.teardown(dstSdUUID, dstVolUUID) Line 559: Line 560: # Shrink to optimal size Line 561: dstVolume = dstVolClass(self.repoPath, dstSdUUID, dstImgUUID, Line 562: dstVolUUID) Line 558: dstVolume.teardown(sdUUID = dstSdUUID, volUUID = dstVolUUID) Line 559: Line 560: # Shrink to optimal size Line 561: dstVolume = dstVolClass(self.repoPath, dstSdUUID, dstImgUUID, Line 562: dstVolUUID) No need to get the dstVolume once again. Line 563: dstVolume.shrinkToOptimalSize() Line 564: Line 565: def cloneStructure(self, sdUUID, imgUUID, dstSdUUID): Line 566: self._createTargetImage(sdCache.produce(dstSdUUID), sdUUID, imgUUID) -- To view, visit http://gerrit.ovirt.org/28328 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7bd2b4b6d45781fa27a128dd68d14b7561d0901 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Utkarsh Singh <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Utkarsh Singh <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
