Nir Soffer has posted comments on this change. Change subject: sparsify: integrating virt-sparsify into vdsm ......................................................................
Patch Set 11: (2 comments) http://gerrit.ovirt.org/#/c/28328/11/vdsm.spec.in File vdsm.spec.in: Line 142: Line 143: %if 0%{?rhel} Line 144: Requires: libguestfs-tools-c >= 1:1.20.11-10 Line 145: %else # fedora Line 146: Requires: libguestfs-tools-c >= 1:1.26.7-2 > The fc20 version is available in Fedora Updates. Then this patch needs to disable the feature on the platform which do not support it yet, or we will have to wait until the package is available. Line 147: %endif Line 148: Line 149: %if 0%{?rhel} >= 7 || 0%{?fedora} >= 18 Line 150: Requires: libvirt-daemon >= 1.0.2-1 http://gerrit.ovirt.org/#/c/28328/11/vdsm/storage/image.py File vdsm/storage/image.py: Line 566: virtsparsify.sparsify(baseVolume.getVolumePath(), Line 567: srcVolume.getVolumePath(), Line 568: dstVolume.getVolumePath(), Line 569: volume.fmt2str(srcVolume.getFormat()), Line 570: volume.fmt2str(dstVolume.getFormat())) > The inconsistency is with src for virt-sparsify actually being our base vol I think you should change the names so it is clear what is going on. Since this is a wrapper for virtsparsify, the terms that virtsparsify are mandatory, and we want to keep them trough the api. Please also explain the virtsparsify flow in the commit message - what is the input, what are the requirements (we must create a snapshot as temporary volume for virtsparsify - right?) what is the output. Line 571: except Exception as e: Line 572: self.log.exception('Unexpected error sparsifying %s', srcVolUUID) Line 573: raise se.CannotSparsifyVolume(srcVolUUID) Line 574: finally: -- 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: 11 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: 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
