Federico Simoncelli has posted comments on this change. Change subject: sparsify: integrating virt-sparsify into vdsm ......................................................................
Patch Set 8: Code-Review-1 (7 comments) Good work, few nits. http://gerrit.ovirt.org/#/c/28328/8/lib/vdsm/virtsparsify.py File lib/vdsm/virtsparsify.py: Line 38: return "ecode=%s, stdout=%s, stderr=%s, message=%s" % ( Line 39: self.ecode, self.stdout, self.stderr, self.message) Line 40: Line 41: Line 42: def sparsify(srcVol, tmpVol, dstVol): Please also add support for srcFormat and dstFormat. Line 43: """ Line 44: Sparsify the 'srcVol' volume to 'dstVol' volume using libguestfs Line 45: virt-sparsify Line 46: """ Line 43: """ Line 44: Sparsify the 'srcVol' volume to 'dstVol' volume using libguestfs Line 45: virt-sparsify Line 46: """ Line 47: cmd = [_virtsparsify.cmd, '--tmp', 'prebuilt:' + tmpVol, srcVol, dstVol] Here source and destination format are specified with: --convert format Format of output disk (default: same as input) --format format Format of input disk (eventually read virt-sparsify manual) Line 48: Line 49: rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL) Line 50: Line 51: if rc != 0: http://gerrit.ovirt.org/#/c/28328/8/vdsm.spec.in File vdsm.spec.in: Line 124: Requires: python-ethtool >= 0.6-3 Line 125: Requires: %{name}-python-zombiereaper = %{version}-%{release} Line 126: Requires: rpm-python Line 127: Requires: nfs-utils Line 128: Requires: m2crypto Remove this. Line 129: Requires: libguestfs-tools-c Line 130: Requires: libnl Line 131: Requires: %{name}-xmlrpc = %{version}-%{release} Line 132: Requires: %{name}-jsonrpc = %{version}-%{release} Line 150: %else Line 151: Requires: libvirt >= 1.0.2-1 Line 152: %endif Line 153: %endif Line 154: Requires: libvirt-python, libvirt-lock-sanlock, libvirt-client Let's add: %if 0%{?rhel} Requires: libguestfs-tools-c >= 1:1.20.11-10 %endif %if 0%{?fedora} Requires: libguestfs-tools-c >= 1:1.26.7-2 %endif Line 155: Line 156: # iscsi-intiator versions Line 157: %if 0%{?rhel} Line 158: %if 0%{?rhel} >= 7 http://gerrit.ovirt.org/#/c/28328/8/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 4092: Line 4093: ## Line 4094: # @Image.sparsify: Line 4095: # Line 4096: # Move or copy an image to another Storage Domain within the same Storage Pool. Leave just "Copy an image..." Line 4097: # Line 4098: # @srcSdUUID: The UUID of the source storage domain Line 4099: # Line 4100: # @srcImgUUID: The UUID of the source image http://gerrit.ovirt.org/#/c/28328/8/vdsm/storage/image.py File vdsm/storage/image.py: Line 544: srcVolume.prepare() Line 545: dstVolume.prepare() Line 546: Line 547: try: Line 548: # Extend sizes Please add a comment (TODO) that we may need some extra space for the qcow2 overhead. Line 549: srcVolume.extend(srcVolume.getSize()) Line 550: dstVolume.extend(srcVolume.getSize()) Line 551: Line 552: # Sparsify Line 551: Line 552: # Sparsify Line 553: virtsparsify.sparsify(baseVolume.getVolumePath(), Line 554: srcVolume.getVolumePath(), Line 555: dstVolume.getVolumePath()) Let's add the format support (see also the comments on sparsify module). The volume format can be obtained with srcVolume.getFormat() and dstVolume.getFormat(). Be careful because you need to convert the format strings (look at volume.py for some examples... check how we use fmt2str for the qemuimg module). Line 556: except Exception as e: Line 557: self.log.exception('Unexpected error sparsifying %s', srcVolUUID) Line 558: raise se.CannotSparsifyVolume(srcVolUUID) Line 559: 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: 8 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: 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
