Change in vdsm[master]: volume: use qemuimg and remove custom qemuConvert

danken at redhat.com danken at redhat.com
Mon May 26 12:39:21 UTC 2014


Dan Kenigsberg has submitted this change and it was merged.

Change subject: volume: use qemuimg and remove custom qemuConvert
......................................................................


volume: use qemuimg and remove custom qemuConvert

This patch removes the custom qemuConvert implementation. It is safe
to drop the ddWatchCopy part because:

- it was initially introduced to workaround a bug in qemu-img that was
  preventing it from working with raw volumes on block devices [1]
- it was later on used to try and maintain the preallocation of a
  volume on file domains, but it was covering only one specific case
  (raw to raw) and in the most common case of copyCollapsed (cow chain
  to raw) was using qemu-img (not maintaining the preallocation)
- given bz#1097843 we are trying to phase out preallocation on file
  domains because it is hard (or sometimes impossible) to guarantee,

This patch has just shown that there are at least two cases where we
were (and are) not able to guarantee preallocation (mergeSnapshot and
copyCollapsed).

[1] the problem is that qemu-img is designed to maintain sparseness
when possible. This means that if it fails to detect that the
destination is a block device then it will seek instead of writing
zeroes, leving some of the previous data in place.
This has been fixed since some time and Kevin Wolf mentioned that
the code is now really explicit about this case being handled (force
to write zeroes if the destination is raw on a block device).
Moreover vdsm has been already relying on qemu-img to convert cow to
raw and it seems that we never hit any issue.

Related-To: http://bugzilla.redhat.com/1097843
Change-Id: Ic27770241495e21737f926b1820494377467383b
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/26921
Reviewed-by: Kevin Wolf <kwolf%redhat.com at gtempaccount.com>
Reviewed-by: Allon Mureinik <amureini at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/storage/image.py
M vdsm/storage/volume.py
2 files changed, 17 insertions(+), 47 deletions(-)

Approvals:
  Federico Simoncelli: Verified
  Allon Mureinik: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved
  Kevin Wolf: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic27770241495e21737f926b1820494377467383b
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Kevin Wolf <kwolf%redhat.com at gtempaccount.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list