Lee Yarwood has uploaded a new change for review.
Change subject: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert. ......................................................................
[WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert.
This change refactors the qemuConvert method, dropping the use of dd with RAW volumes in favour of qemu-img convert thus preserving sparseness. The change also introduces the use of the backing_file and backing_fmt options when converting COW volumes.
Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Signed-off-by: Lee Yarwood lyarwood@redhat.com --- M vdsm/storage/image.py M vdsm/storage/volume.py 2 files changed, 25 insertions(+), 38 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/8728/1
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index e86d94c..69ab6b6 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -788,19 +788,9 @@
try: # Start the actual copy image procedure - srcVol.prepare(rw=False) + srcVol.prepare(rw=True, chainrw=True, setrw=True) dstVol.prepare(rw=True, setrw=True) - - try: - (rc, out, err) = volume.qemuConvert(volParams['path'], dstPath, - volParams['volFormat'], dstVolFormat, vars.task.aborting, - size=srcVol.getVolumeSize(bs=1), dstvolType=dstVol.getType()) - if rc: - raise se.StorageException("rc: %s, err: %s" % (rc, err)) - except se.ActionStopped, e: - raise e - except se.StorageException, e: - raise se.CopyImageError(str(e)) + (rc, out, err) = volume.qemuConvert(srcVol, dstVol, self.idle, vars.task.aborting)
# Mark volume as SHARED if volType == volume.SHARED_VOL: @@ -997,12 +987,12 @@
# Step 2: Convert successor to new volume # qemu-img convert -f qcow2 successor -O raw newUUID - (rc, out, err) = volume.qemuConvert(srcVolParams['path'], newVol.getVolumePath(), - srcVolParams['volFormat'], volParams['volFormat'], vars.task.aborting, - size=volParams['apparentsize'], dstvolType=newVol.getType()) - if rc: - self.log.error("qemu-img convert failed: rc=%s, out=%s, err=%s", - rc, out, err) + try: + (rc, out, err) = volume.qemuConvert(srcVol, newVol, self.idle, vars.task.aborting) + except se.ActionStopped: + raise + except se.StorageException: + self.log.error("Unexpected error", exc_info=True) raise se.MergeSnapshotsError(newUUID)
newVol.teardown(sdUUID=newVol.sdUUID, volUUID=newVol.volUUID) diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index b9eb356..875b59a 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -960,28 +960,25 @@ log.debug('(qemuRebase): REBASE %s DONE' % (src)) return (rc, out, err)
+def qemuConvert(src, dst, idle, stop): + """ + Copy the src image into the new dst. + """ + cmd = constants.CMD_LOWPRIO + [constants.EXT_QEMUIMG, "convert", "-t", "none", + "-f", fmt2str(src.getFormat()), "-O", fmt2str(dst.getFormat())]
-def qemuConvert(src, dst, src_fmt, dst_fmt, stop, size, dstvolType): - """ - Convert the 'src' image (or chain of images) into a new single 'dst' - """ - src_fmt = fmt2str(src_fmt) - dst_fmt = fmt2str(dst_fmt) - log.debug('(qemuConvert): COPY %s (%s) to %s (%s) START' % - (src, src_fmt, dst, dst_fmt)) + # backing_file only used when src has a parent and both volumes are COW. + if src.getParentVolume() and src.getFormat() == COW_FORMAT and dst.getFormat() == COW_FORMAT : + cmd += ["-o", "backing_file=%s,backing_fmt=%s" % (src.getParentVolume().getVolumePath(), + fmt2str(src.getParentVolume().getFormat()))]
- if (src_fmt == "raw" and dst_fmt == "raw" and - dstvolType == PREALLOCATED_VOL): - (rc, out, err) = misc.ddWatchCopy( - src=src, dst=dst, - stop=stop, size=size, - recoveryCallback=baseAsyncTasksRollback) - else: - cmd = constants.CMD_LOWPRIO + [constants.EXT_QEMUIMG, "convert", - "-t", "none", "-f", src_fmt, src, - "-O", dst_fmt, dst] - (rc, out, err) = misc.watchCmd(cmd, stop=stop, + cmd += [src.getVolumePath(), dst.getVolumePath()] + + (rc, out, err) = misc.watchCmd(cmd, stop=stop, idle=idle, sudo=False, recoveryCallback=baseAsyncTasksRollback)
- log.debug('(qemuConvert): COPY %s to %s DONE' % (src, dst)) + if rc: + raise se.StorageException + return (rc, out, err) +
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert. ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/56/ (2/2)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert. ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/33/ (1/2)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert. ......................................................................
Patch Set 4: I would prefer that you didn't submit this
Build Unstable
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/56/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/33/ : UNSTABLE
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Yeela Kaplan has posted comments on this change.
Change subject: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert. ......................................................................
Patch Set 4: I would prefer that you didn't submit this
(11 inline comments)
A few small changes to make, but other than that the idea itself looks good.
.................................................... 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: (rc, out, err) = volume.qemuConvert(srcVol, dstVol, vars.task.aborting) qemuConvert no longer returns (rc, out, err) Line 586: except se.ActionStopped: Line 587: raise Line 588: except se.StorageException: Line 589: self.log.error("Unexpected error", exc_info=True)
Line 584: dstVol = destDom.produceVolume(imgUUID=imgUUID, volUUID=srcVol.volUUID) Line 585: (rc, out, err) = volume.qemuConvert(srcVol, dstVol, vars.task.aborting) Line 586: except se.ActionStopped: Line 587: raise Line 588: except se.StorageException: Would be nice to change unexpected error and storageException here too... Line 589: self.log.error("Unexpected error", exc_info=True) Line 590: raise Line 591: except Exception: Line 592: self.log.error("Copy image error: image=%s, src domain=%s, dst domain=%s", imgUUID, srcSdUUID,
Line 856: try: Line 857: # Start the actual copy image procedure Line 858: srcVol.prepare(rw=True, chainrw=True, setrw=True) Line 859: dstVol.prepare(rw=True, setrw=True) Line 860: (rc, out, err) = volume.qemuConvert(srcVol, dstVol, vars.task.aborting) qemuConvert no longer returns (rc, out, err) Line 861: Line 862: # Mark volume as SHARED Line 863: if volType == volume.SHARED_VOL: Line 864: dstVol.setShared()
Line 866: if force: Line 867: # Now we should re-link all deleted hardlinks, if exists Line 868: self.__templateRelink(destDom, dstImgUUID, dstVolUUID) Line 869: except se.ActionStopped: Line 870: raise too general:
except se.ActionStopped, e: raise e
will be better, this way we can see what the error is. Line 871: except se.StorageException, e: Line 872: self.log.error("Unexpected error", exc_info=True) Line 873: raise Line 874: except Exception, e:
Line 868: self.__templateRelink(destDom, dstImgUUID, dstVolUUID) Line 869: except se.ActionStopped: Line 870: raise Line 871: except se.StorageException, e: Line 872: self.log.error("Unexpected error", exc_info=True) Would be nice to change unexpected error here too... Line 873: raise Line 874: except Exception, e: Line 875: self.log.error("Unexpected error", exc_info=True) Line 876: raise se.CopyImageError("src image=%s, dst image=%s: msg=%s" % (srcImgUUID, dstImgUUID, str(e)))
Line 1056: Line 1057: # Step 2: Convert successor to new volume Line 1058: # qemu-img convert -f qcow2 successor -O raw newUUID Line 1059: try: Line 1060: (rc, out, err) = volume.qemuConvert(srcVol, newVol, vars.task.aborting) Here we have the source and destination format in volParams, it's a shame to call getFormat() inside qemuConvert.
Also, qemuConvert no longer returns (rc, out, err) Line 1061: except se.ActionStopped: Line 1062: raise Line 1063: except se.StorageException: Line 1064: self.log.error("Unexpected error", exc_info=True)
Line 1060: (rc, out, err) = volume.qemuConvert(srcVol, newVol, vars.task.aborting) Line 1061: except se.ActionStopped: Line 1062: raise Line 1063: except se.StorageException: Line 1064: self.log.error("Unexpected error", exc_info=True) Change StorageExcetion.... Line 1065: raise se.MergeSnapshotsError(newUUID) Line 1066: Line 1067: if chList: Line 1068: newVol.setInternal()
.................................................... File vdsm/storage/volume.py Line 997: log.debug('(qemuRebase): REBASE %s DONE' % (src)) Line 998: return (rc, out, err) Line 999: Line 1000: Line 1001: def qemuConvert(src, dst, stop): This function belongs to vdsm/qemuImg.py, I think we should move it there. Line 1002: cmd = constants.CMD_LOWPRIO + [constants.EXT_QEMUIMG, "convert", Line 1003: "-t", "none", Line 1004: "-f", fmt2str(src.getFormat()), Line 1005: "-O", fmt2str(dst.getFormat())]
Line 1002: cmd = constants.CMD_LOWPRIO + [constants.EXT_QEMUIMG, "convert", Line 1003: "-t", "none", Line 1004: "-f", fmt2str(src.getFormat()), Line 1005: "-O", fmt2str(dst.getFormat())] Line 1006: seems redundant to remove the src format and dst format parameters and then call getFormat() several times, when we already have the format info when calling qemuConvert. Line 1007: if (src.getParentVolume() and src.getFormat() == COW_FORMAT Line 1008: and dst.getFormat() == COW_FORMAT): Line 1009: cmd += ["-o backing_file=%s,backing_fmt=%s" % Line 1010: src.getParentVolume().getVolumePath(),
Line 1011: fmt2str(src.getParentVolume().getFormat())] Line 1012: Line 1013: cmd += [src.getVolumePath(), dst.getVolumePath()] Line 1014: (rc, out, err) = misc.watchCmd(cmd, stop, Line 1015: recoveryCallback=baseAsyncTasksRollback) We should use misc.execCmd() Line 1016: Line 1017: if rc != 0: Line 1018: raise se.StorageException Line 1019:
Line 1014: (rc, out, err) = misc.watchCmd(cmd, stop, Line 1015: recoveryCallback=baseAsyncTasksRollback) Line 1016: Line 1017: if rc != 0: Line 1018: raise se.StorageException We should raise:
raise QImgError(rc, out, err)
just like with qemu-img info and check in qemuImg.py, since this is no longer a generic storageException as in ddWatchCopy. Line 1019:
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img convert. ......................................................................
Patch Set 4: (3 inline comments)
.................................................... File vdsm/storage/image.py Line 854: raise se.CopyImageError("Destination volume %s error: %s" % (dstVolUUID, str(e))) Line 855: Line 856: try: Line 857: # Start the actual copy image procedure Line 858: srcVol.prepare(rw=True, chainrw=True, setrw=True) why you need setrw=True? It's a source chain. Line 859: dstVol.prepare(rw=True, setrw=True) Line 860: (rc, out, err) = volume.qemuConvert(srcVol, dstVol, vars.task.aborting) Line 861: Line 862: # Mark volume as SHARED
Line 866: if force: Line 867: # Now we should re-link all deleted hardlinks, if exists Line 868: self.__templateRelink(destDom, dstImgUUID, dstVolUUID) Line 869: except se.ActionStopped: Line 870: raise I think it's fine, anyway it will raise same exception Line 871: except se.StorageException, e: Line 872: self.log.error("Unexpected error", exc_info=True) Line 873: raise Line 874: except Exception, e:
.................................................... File vdsm/storage/volume.py Line 1007: if (src.getParentVolume() and src.getFormat() == COW_FORMAT Line 1008: and dst.getFormat() == COW_FORMAT): Line 1009: cmd += ["-o backing_file=%s,backing_fmt=%s" % Line 1010: src.getParentVolume().getVolumePath(), Line 1011: fmt2str(src.getParentVolume().getFormat())] I am not sure but what src.getParentVolume().getVolumePath() returns? Is it a full path (I think so) or a relative path? If it a full path it can be a problem because across all vdsm we using relative path (according to imgUUID) as backing path vol snapshots Line 1012: Line 1013: cmd += [src.getVolumePath(), dst.getVolumePath()] Line 1014: (rc, out, err) = misc.watchCmd(cmd, stop, Line 1015: recoveryCallback=baseAsyncTasksRollback)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - Move qemuConvert into qemuImg.convert ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/358/ (1/2)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - Move qemuConvert into qemuImg.convert ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/392/ (2/2)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - Move qemuConvert into qemuImg.convert ......................................................................
Patch Set 5: Fails; I would prefer that you didn't submit this
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/358/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/392/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - Refactor and move volume.qemuConvert into qemuImg. ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/395/ (1/2)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - Refactor and move volume.qemuConvert into qemuImg. ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/361/ (2/2)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] BZ#748386 - Refactor and move volume.qemuConvert into qemuImg. ......................................................................
Patch Set 6: Fails; I would prefer that you didn't submit this
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/361/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/395/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Ayal Baron has posted comments on this change.
Change subject: [WIP] BZ#748386 - Refactor and move volume.qemuConvert into qemuImg. ......................................................................
Patch Set 6: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/qemuImg.py Line 110: and dstFmt == volume.COW_FORMAT): Line 111: Line 112: # TODO - confirm backing_file format in use already Line 113: # Is it the full path or img/vol? Line 114: cmd += ["-o","backing_file="+src.getParentVolume().getVolumePath() + \ src looks wrong here. iiuc this will create a tree instead of 2 separate chains. you should check to see if the destination volume has a backing file and only in that case pass the backing_file. Don't forget that this method is used to collapse images and always using a backing file breaks that functionality. Line 115: ",backing_fmt="+volume.fmt2str(src.getParentVolume().getFormat())] Line 116: Line 117: cmd += [src.getVolumePath(), dst.getVolumePath()] Line 118:
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Ayal Baron abaron@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Ayal Baron has posted comments on this change.
Change subject: [WIP] BZ#748386 - Refactor and move volume.qemuConvert into qemuImg. ......................................................................
Patch Set 6: (1 inline comment)
.................................................... File vdsm/qemuImg.py Line 111: Line 112: # TODO - confirm backing_file format in use already Line 113: # Is it the full path or img/vol? Line 114: cmd += ["-o","backing_file="+src.getParentVolume().getVolumePath() + \ Line 115: ",backing_fmt="+volume.fmt2str(src.getParentVolume().getFormat())] note that I'm not sure how this would work at all. Perhaps you do need to use the source and after qemu-img finishes rebase unsafe on the destination parent. In any event, this seems like a dangerous change that needs a lot of testing. I would actually opt for an initial solution replacing ddWatchCopy in interImagesCopy with cp (like you already did but abandoned) and after that is in (to at least have sparse copy) then start moving to a qemu-img based solution. Line 116: Line 117: cmd += [src.getVolumePath(), dst.getVolumePath()] Line 118: Line 119: rc, out, err = execCmd(cmd)
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Ayal Baron abaron@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Lee Yarwood has abandoned this change.
Change subject: [WIP] BZ#748386 - Refactor and move volume.qemuConvert into qemuImg. ......................................................................
Patch Set 6: Abandoned
http://gerrit.ovirt.org/#/c/10979/
-- To view, visit http://gerrit.ovirt.org/8728 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon Gerrit-Change-Id: I7adc41e8a9f5d4b9faa58a8cf5bec685b66e303e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Lee Yarwood lyarwood@redhat.com Gerrit-Reviewer: Ayal Baron abaron@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: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org