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