Federico Simoncelli has posted comments on this change.
Change subject: Use cp to copy sparse volumes when moving a vm ......................................................................
Patch Set 7: (1 inline comment)
.................................................... 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: dstVol.watchCopy(srcVol, vars.task.aborting) What I don't like here is that we don't check the source and destination domain types, which means that we could end up using "cp" also from a block domain to a file domain. I think we already discussed this and we said that "cp" is able to handle the copy correctly, nonetheless it looks ugly to me (and it might even work only by accident). Either we check for the domains to be of the same kind eg:
if type(dstVol) == type(srcVol): dstVol.watchCopy(srcVol, vars.task.aborting) else: ...use the regular dd...
Or we just use qemu-img that is capable of generating sparseness on the fly (even from block devices to files). Line 586: except se.ActionStopped: Line 587: raise Line 588: except se.StorageException: Line 589: self.log.error("Unexpected error", exc_info=True)
-- 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: 7 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