Change in vdsm[master]: Use cp to copy sparse volumes when moving a vm
abaron at redhat.com
abaron at redhat.com
Tue Jan 8 19:19:20 UTC 2013
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 at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dafna Ron <dron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Lee Yarwood <lyarwood at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list