Ayal Baron has posted comments on this change.
Change subject: Use cp to copy sparse volumes when moving a vm ......................................................................
Patch Set 6: I would prefer that you didn't submit this
(5 inline comments)
.................................................... File vdsm/storage/blockVolume.py Line 639: mtime = 0 Line 640: Line 641: return mtime Line 642: Line 643: def watchCopy(self, srcVol, stop, sparse=False): I don't think we need sparse in the API here Line 644: srcSize = srcVol.getVolumeSize(bs=1) Line 645: misc.ddWatchCopy(srcVol.getVolumePath(), Line 646: self.getVolumePath(), Line 647: stop, srcSize)
.................................................... File vdsm/storage/fileVolume.py Line 615: return self.getMetaParam(volume.MTIME) Line 616: except se.MetaDataKeyNotFoundError: Line 617: return self.oop.os.stat(volPath).st_mtime Line 618: Line 619: def watchCopy(self, srcVol, stop, sparse=False): why would we want sparse to ever be False here? as mentioned before, I think it should just be set to True when calling cpWatchCopy below and not part of the API here. Line 620: misc.cpWatchCopy(srcVol.getVolumePath(), Line 621: self.getVolumePath(),
.................................................... File vdsm/storage/image.py Line 581: for srcVol in chains['srcChain']: Line 582: # Do the actual copy Line 583: try: Line 584: dstVol = destDom.produceVolume(imgUUID=imgUUID, volUUID=srcVol.volUUID) Line 585: # Copy sparse files with cp. comment is misplaced (this code is agnostic to internal implementation) Line 586: dstVol.watchCopy(srcVol, vars.task.aborting, sparse=True) Line 587: except se.ActionStopped: Line 588: raise Line 589: except se.StorageException:
Line 582: # Do the actual copy Line 583: try: Line 584: dstVol = destDom.produceVolume(imgUUID=imgUUID, volUUID=srcVol.volUUID) Line 585: # Copy sparse files with cp. Line 586: dstVol.watchCopy(srcVol, vars.task.aborting, sparse=True) I don't understand why the sparse property belongs here and not inside fileVolume Line 587: except se.ActionStopped: Line 588: raise Line 589: except se.StorageException: Line 590: self.log.error("Unexpected error", exc_info=True)
.................................................... File vdsm/storage/volume.py Line 904: volParams['descr'] = self.getDescription() Line 905: volParams['legality'] = self.getLegality() Line 906: return volParams Line 907: Line 908: def watchCopy(self, srcVol, stop, sparse=False): redundant, and if Eduardo were here he would oppose this method :) Line 909: """ Line 910: Copy the volume contents Line 911: """ Line 912: pass
-- To view, visit http://gerrit.ovirt.org/10543 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I0c4c693884a225c868c18bdcb4930c2c1042db2c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dafna Ron dron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server