Change in vdsm[master]: [WIP] BZ#748386 - refactor qemuConvert to only use qemu-img ...

ilvovsky at redhat.com ilvovsky at redhat.com
Tue Dec 11 09:27:22 UTC 2012


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 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: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list