Utkarsh Singh has posted comments on this change. Change subject: sparsify: integrating virt-sparsify into vdsm ......................................................................
Patch Set 14: (5 comments) http://gerrit.ovirt.org/#/c/28328/14/vdsm/storage/image.py File vdsm/storage/image.py: Line 546: Line 547: tmpVolume = self._getSparsifyVolume(tmpSdUUID, tmpImgUUID, tmpVolUUID) Line 548: dstVolume = self._getSparsifyVolume(dstSdUUID, dstImgUUID, dstVolUUID) Line 549: Line 550: if not tmpVolume.isSparse() or not dstVolume.isSparse(): > Probably the correct check here would be that tmpVolume format is COW. Anyw Sorry about the comment. Dropped tmpVolume check now. Line 551: raise se.VolumeNotSparse() Line 552: Line 553: srcVolume = self._getSparsifyVolume(tmpSdUUID, tmpImgUUID, Line 554: tmpVolume.getParent()) Line 553: srcVolume = self._getSparsifyVolume(tmpSdUUID, tmpImgUUID, Line 554: tmpVolume.getParent()) Line 555: Line 556: tmpVolume.prepare() Line 557: > I commented about the uneeded line below the next prepare, but assumed you Sorry about that. Removed this line too. Line 558: try: Line 559: dstVolume.prepare() Line 560: try: Line 561: # TODO: Some extra space may be needed for QCOW2 headers Line 558: try: Line 559: dstVolume.prepare() Line 560: try: Line 561: # TODO: Some extra space may be needed for QCOW2 headers Line 562: tmpVolume.extend(tmpVolume.getSize()) > Lets add this info in Federico reply in a comment above the extend invocati Done Line 563: dstVolume.extend(tmpVolume.getSize()) Line 564: Line 565: srcFormat = volume.fmt2str(srcVolume.getFormat()) Line 566: dstFormat = volume.fmt2str(dstVolume.getFormat()) Line 558: try: Line 559: dstVolume.prepare() Line 560: try: Line 561: # TODO: Some extra space may be needed for QCOW2 headers Line 562: tmpVolume.extend(tmpVolume.getSize()) > By definition "sparsification" is implemented writing a file with zeroes as Added TODO. Line 563: dstVolume.extend(tmpVolume.getSize()) Line 564: Line 565: srcFormat = volume.fmt2str(srcVolume.getFormat()) Line 566: dstFormat = volume.fmt2str(dstVolume.getFormat()) http://gerrit.ovirt.org/#/c/28328/14/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 1572: """ Line 1573: srcNamespace = sd.getNamespace(tmpSdUUID, IMAGE_NAMESPACE) Line 1574: dstNamespace = sd.getNamespace(dstSdUUID, IMAGE_NAMESPACE) Line 1575: Line 1576: with nested( > When I wrote "explain why...", I meant that you will add a comments explain Done Line 1577: rmanager.acquireResource(srcNamespace, tmpImgUUID, Line 1578: rm.LockType.exclusive), Line 1579: rmanager.acquireResource(dstNamespace, dstImgUUID, Line 1580: rm.LockType.exclusive)): -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Utkarsh Singh <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[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
