Adam Litke has uploaded a new change for review.
Change subject: storage: Move detect_format to new workarounds module
......................................................................
storage: Move detect_format to new workarounds module
The function Image._detect_format is a workaround for a bug where the
vdsm metadata disagrees with qemuimg about the format of a volume.
While the underlying bug has been fixed, we still need to maintain this
code in order to prevent problems with preexisting affected volumes.
For reusability and maintainability, move this function to a new module
called workarounds.py and add some basic unit tests. This code will be
reused by sdm.copy_data in a followup patch.
Change-Id: If4da9d2c16679f99b55438d7336d0cfb27429c12
Signed-off-by: Adam Litke <ali...@redhat.com>
---
M tests/Makefile.am
A tests/storage_workarounds_test.py
M vdsm.spec.in
M vdsm/storage/Makefile.am
M vdsm/storage/image.py
A vdsm/storage/workarounds.py
6 files changed, 153 insertions(+), 27 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/64229/1
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1984655..34434d0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -132,6 +132,7 @@
storage_volume_test.py \
storage_volume_artifacts_test.py \
storage_volume_metadata_test.py \
+ storage_workarounds_test.py \
tasksetTests.py \
testlibTests.py \
toolTests.py \
@@ -234,6 +235,7 @@
storage_volume_test.py \
storage_volume_artifacts_test.py \
storage_volume_metadata_test.py \
+ storage_workarounds_test.py \
storagefakelibTests.py \
storagetestlibTests.py \
toolBondingTests.py \
diff --git a/tests/storage_workarounds_test.py
b/tests/storage_workarounds_test.py
new file mode 100644
index 0000000..900b512
--- /dev/null
+++ b/tests/storage_workarounds_test.py
@@ -0,0 +1,81 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from __future__ import absolute_import
+import uuid
+
+from testlib import VdsmTestCase
+from storagetestlib import fake_file_env
+
+from storage import workarounds
+
+from vdsm import qemuimg
+from vdsm.storage import constants as sc
+
+
+class DetectFormatTest(VdsmTestCase):
+
+ def make_volumes(self, env, size, src_md_fmt, src_qemu_fmt, dst_md_fmt):
+ src_img_id = str(uuid.uuid4())
+ src_vol_id = str(uuid.uuid4())
+ dst_img_id = str(uuid.uuid4())
+ dst_vol_id = str(uuid.uuid4())
+ env.make_volume(size, src_img_id, src_vol_id, vol_format=src_md_fmt)
+ env.make_volume(size, dst_img_id, dst_vol_id, vol_format=dst_md_fmt)
+ src_vol = env.sd_manifest.produceVolume(src_img_id, src_vol_id)
+ dst_vol = env.sd_manifest.produceVolume(dst_img_id, dst_vol_id)
+ qemuimg.create(src_vol.getVolumePath(), size, src_qemu_fmt)
+ return src_vol, dst_vol
+
+ def test_bad_format_vm_conf_disk(self):
+ """
+ When the volume size matches the VM configuration disk size and the
+ source volume reports COW even though qemuimg reports RAW then we
+ expect the workaround to report both volumes as RAW.
+ """
+ size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
+ with fake_file_env() as env:
+ src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
+ qemuimg.FORMAT.RAW, sc.RAW_FORMAT)
+ self.assertEqual((qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW),
+ workarounds.detect_format(src, dst))
+
+ def test_bad_format_other_size(self):
+ """
+ When the volume size does not match the VM configuration disk size then
+ the workaround will not be activated even when the formats don't match
+ """
+ size = 2 * workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
+ with fake_file_env() as env:
+ src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
+ qemuimg.FORMAT.RAW, sc.RAW_FORMAT)
+ self.assertEqual((qemuimg.FORMAT.QCOW2, qemuimg.FORMAT.RAW),
+ workarounds.detect_format(src, dst))
+
+ def test_cow_vm_conf_disk(self):
+ """
+ When a VM configuration disk is actually COW format report it correctly
+ """
+ size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
+ with fake_file_env() as env:
+ src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
+ qemuimg.FORMAT.QCOW2, sc.COW_FORMAT)
+ self.assertEqual((qemuimg.FORMAT.QCOW2, qemuimg.FORMAT.QCOW2),
+ workarounds.detect_format(src, dst))
diff --git a/vdsm.spec.in b/vdsm.spec.in
index f234b09..d7fc83e 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1008,6 +1008,7 @@
%{_datadir}/%{vdsm_name}/storage/task.py*
%{_datadir}/%{vdsm_name}/storage/threadPool.py*
%{_datadir}/%{vdsm_name}/storage/volume.py*
+%{_datadir}/%{vdsm_name}/storage/workarounds.py*
%{_datadir}/%{vdsm_name}/storage/imageRepository/__init__.py*
%{_datadir}/%{vdsm_name}/storage/imageRepository/formatConverter.py*
%{_datadir}/%{vdsm_name}/storage/sdm/__init__.py*
diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am
index b0a66b7..4e1f50e 100644
--- a/vdsm/storage/Makefile.am
+++ b/vdsm/storage/Makefile.am
@@ -55,7 +55,9 @@
taskManager.py \
task.py \
threadPool.py \
- volume.py
+ volume.py \
+ workarounds.py \
+ $(NULL)
dist_vdsmexec_SCRIPTS = \
curl-img-wrap \
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 6faea2f..37619d9 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -40,6 +40,7 @@
from vdsm.exception import ActionStopped
import task
import resourceManager as rm
+import workarounds
log = logging.getLogger('storage.Image')
rmanager = rm.ResourceManager.getInstance()
@@ -68,9 +69,6 @@
OP_TYPES = {UNKNOWN_OP: 'UNKNOWN', COPY_OP: 'COPY', MOVE_OP: 'MOVE'}
RENAME_RANDOM_STRING_LEN = 8
-
-# Size in blocks of the conf file generated during RAM snapshot operation.
-VM_CONF_SIZE_BLK = 20
# Temporary size of a volume when we optimize out the prezeroing
TEMPORARY_VOLUME_SIZE = 20480 # in sectors (10M)
@@ -457,7 +455,8 @@
try:
dstVol = destDom.produceVolume(imgUUID=imgUUID,
volUUID=srcVol.volUUID)
- srcFormat, dstFormat = self._detect_format(srcVol, dstVol)
+ srcFormat, dstFormat = workarounds.detect_format(srcVol,
+ dstVol)
parentVol = dstVol.getParentVolume()
@@ -491,28 +490,6 @@
finally:
# teardown volumes
self.__cleanupMove(srcLeafVol, dstLeafVol)
-
- def _detect_format(self, srcVol, dstVol):
- """
- VM metadata image format is RAW, whereas in .meta file it's COW:
- see bz#1282239. Hence, detecting metadata files by size and format
- mis-correlation.
- """
- src_format = srcVol.getFormat()
- size_in_blk = srcVol.getSize()
- if src_format == sc.COW_FORMAT and size_in_blk == VM_CONF_SIZE_BLK:
- info = qemuimg.info(srcVol.getVolumePath())
- actual_format = info['format']
-
- if actual_format == qemuimg.FORMAT.RAW:
- self.log.warning("Incorrect volume format %r has been detected"
- " for volume %r, using the actual format %r.",
- qemuimg.FORMAT.QCOW2,
- srcVol.volUUID,
- qemuimg.FORMAT.RAW)
- return qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW
-
- return sc.fmt2str(src_format), sc.fmt2str(dstVol.getFormat())
def _finalizeDestinationImage(self, destDom, imgUUID, chains, force):
for srcVol in chains['srcChain']:
diff --git a/vdsm/storage/workarounds.py b/vdsm/storage/workarounds.py
new file mode 100644
index 0000000..7cea63d
--- /dev/null
+++ b/vdsm/storage/workarounds.py
@@ -0,0 +1,63 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import logging
+
+from vdsm import qemuimg
+from vdsm.storage import constants as sc
+
+log = logging.getLogger('storage.workarounds')
+
+# Size in blocks of the conf file generated during RAM snapshot operation.
+VM_CONF_SIZE_BLK = 20
+
+
+def detect_format(srcVol, dstVol):
+ """
+ set VM metadata images format to RAW
+
+ Since commit 0b61c4851a528fd6354d9ab77a68085c41f35dc9 copy of internal raw
+ volumes is done using 'qemu-img convert' instead of invoking 'dd'.
+
+ Consequently, exporting VM metadata images (produce during live snapshot)
+ fails on qemu-img convert - since the images 'impersonate' to qcow2 (the
+ format in .meta file is cow, whereas the real format is raw). This problem
+ is documented by https://bugzilla.redhat.com/show_bug.cgi?id=1282239 and
+ has subsequently been fixed in ovirt-engine
+ (see https://gerrit.ovirt.org/#/c/48768/).
+
+ Since VM metadata volumes with this problem may still exist in storage we
+ must keep using this workaround to avoid problems with copying VM disks.
+ """
+ src_format = srcVol.getFormat()
+ size_in_blk = srcVol.getSize()
+ if src_format == sc.COW_FORMAT and size_in_blk == VM_CONF_SIZE_BLK:
+ info = qemuimg.info(srcVol.getVolumePath())
+ actual_format = info['format']
+
+ if actual_format == qemuimg.FORMAT.RAW:
+ log.warning("Incorrect volume format %r has been detected"
+ " for volume %r, using the actual format %r.",
+ qemuimg.FORMAT.QCOW2,
+ srcVol.volUUID,
+ qemuimg.FORMAT.RAW)
+ return qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW
+
+ return sc.fmt2str(src_format), sc.fmt2str(dstVol.getFormat())
--
To view, visit https://gerrit.ovirt.org/64229
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If4da9d2c16679f99b55438d7336d0cfb27429c12
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
