Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm

nsoffer at redhat.com nsoffer at redhat.com
Wed Aug 13 08:12:51 UTC 2014


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 <utkarshsins at gmail.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Utkarsh Singh <utkarshsins at gmail.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list