Federico Simoncelli has posted comments on this change. Change subject: sparsify: integrating virt-sparsify into vdsm ......................................................................
Patch Set 3: (6 comments) http://gerrit.ovirt.org/#/c/28328/3/client/vdsClient.py File client/vdsClient.py: Line 1303: srcVolUUID, dstSdUUID, dstImgUUID, Line 1304: dstVolUUID) Line 1305: if status['status']['code']: Line 1306: return status['status']['code'], status['status']['message'] Line 1307: return 0, '' I think that for consistency this should return the task id: status['uuid'] Line 1308: Line 1309: def cloneImageStructure(self, args): Line 1310: spUUID, sdUUID, imgUUID, dstSdUUID = args Line 1311: image = self.s.cloneImageStructure(spUUID, sdUUID, imgUUID, dstSdUUID) http://gerrit.ovirt.org/#/c/28328/3/configure.ac File configure.ac: Line 246: AC_PATH_PROG([TUNE2FS_PATH], [tune2fs], [/sbin/tune2fs]) Line 247: AC_PATH_PROG([UDEVADM_PATH], [udevadm], [/sbin/udevadm]) Line 248: AC_PATH_PROG([UMOUNT_PATH], [umount], [/bin/umount]) Line 249: AC_PATH_PROG([UNPERSIST_PATH], [unpersist], [/usr/sbin/unpersist]) Line 250: AC_PATH_PROG([VIRTSPARSIFY_PATH], [virt-sparsify], [/usr/bin/virt-sparsify]) Anything to add to the vdsm.spec file? Line 251: AC_PATH_PROG([WGET_PATH], [wget], [/usr/bin/wget]) Line 252: AC_PATH_PROG([YUM_PATH], [yum], [/usr/bin/yum]) Line 253: Line 254: # Keep sorted http://gerrit.ovirt.org/#/c/28328/3/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 4012: # @Image.sparsify: Line 4013: # Line 4014: # Move or copy an image to another Storage Domain within the same Storage Pool. Line 4015: # Line 4016: # @srcSdUUID: The UUID of the source storage domain Don't use tabs here (and lines below as well) Line 4017: # Line 4018: # @srcImgUUID: The UUID of the source image Line 4019: # Line 4020: # @srcVolUUID: The UUID of the source volume http://gerrit.ovirt.org/#/c/28328/3/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1598: else: Line 1599: sdUUIDs = sorted((srcSdUUID, dstSdUUID)) Line 1600: Line 1601: for dom in sdUUIDs: Line 1602: vars.task.getSharedLock(STORAGE, dom) I think that the entire thing could be summarized with: for sdUUID in sorted(set((srcSdUUID, dstSdUUID))): vars.task.getSharedLock(STORAGE, sdUUID) Line 1603: Line 1604: self._spmSchedule(spUUID, "sparsifyImage", pool.sparsifyImage, Line 1605: srcSdUUID, srcImgUUID, srcVolUUID, dstSdUUID, Line 1606: dstImgUUID, dstVolUUID) http://gerrit.ovirt.org/#/c/28328/3/vdsm/storage/image.py File vdsm/storage/image.py: Line 536: raise se.VolumeNotSparse() Line 537: Line 538: (rc, out, err) = virtSparsify(srcVolume.getVolumePath(), Line 539: dstVolume.getVolumePath(), Line 540: vars.task.aborting) You probably need to test this with real data (>1Gb). I think you should extend the destination volume path to the size of the source, and then when it's over you should use shrinkToOptimalSize. Line 541: Line 542: if rc: Line 543: raise se.CannotSparsifyVolume(self.srcVolUUID) Line 544: Line 1224: finally: Line 1225: domain.deactivateImage(imgUUID) Line 1226: Line 1227: Line 1228: def virtSparsify(srcVol, dstVol, stop): Move this to a new file lib/vdsm/virtsparsify.py. For the content take inspiration from lib/vdsm/qemuimg.py. Line 1229: """ Line 1230: Sparsify the 'srcVol' volume to 'dstVol' volume using libguestfs Line 1231: virt-sparsify Line 1232: """ -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Utkarsh Singh <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[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
