Change in vdsm[master]: image: use qemu-img convert to copy internal volumes

nsoffer at redhat.com nsoffer at redhat.com
Thu Sep 25 19:48:22 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: image: use qemu-img convert to copy internal volumes
......................................................................


Patch Set 1:

(2 comments)

Great idea, need some cleanup.

http://gerrit.ovirt.org/#/c/33355/1/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 432:                                                    volUUID=srcVol.volUUID)
Line 433:                     srcFmtStr = volume.fmt2str(srcVol.getFormat())
Line 434:                     dstFmtStr = volume.fmt2str(dstVol.getFormat())
Line 435: 
Line 436:                     pntVol = dstVol.getParentVolume()
srcFmtStr? pntVol? This is too cryptic for me :-)

Lets use nicer names, and when possible, the same names used in qemuimg.convert():

  parentVol
  srcFormat
  dstFormat
  backingFile
  backingFormat
Line 437: 
Line 438:                     if pntVol is not None:
Line 439:                         pntBakPath = volume.getBackingVolumePath(
Line 440:                             imgUUID, pntVol.volUUID)


Line 447:                     qemuimg.convert(srcVol.getVolumePath(),
Line 448:                                     dstVol.getVolumePath(),
Line 449:                                     vars.task.aborting,
Line 450:                                     srcFmtStr, dstFmtStr,
Line 451:                                     pntBakPath, pntFmtStr)
There are 2 sins here:
- Mixing one parameter per line with multiple parameters
- Calling kwargs arguments like positional args
Line 452:                 except ActionStopped:
Line 453:                     raise
Line 454:                 except se.StorageException:
Line 455:                     self.log.error("Unexpected error", exc_info=True)


-- 
To view, visit http://gerrit.ovirt.org/33355
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c740d88d52ca678d6c02d0ea500d2459c26560c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list