Hello Federico Simoncelli, Allon Mureinik,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
qemuimg: Create qcow2 compat 0.10 images
Recent qemu versions introduced a new default qcow2 format (1.1), which is not compatible with older versions of qemu. For example, EL6 host cannot create snapshots if an image was created on EL7 host.
This patch uses the "compat" option to create images in the older format (0.10), which are are compatible with different versions of qemu.
The "compat" option is not available on older qemu-img, so we have to use the "-o ?" option to detect the availability of the "compat" option before creating an image. This is the same method used by libvirt.
When all supported platforms provide the "compat" option, we can remove the "compat" detection code. Chanding the compat version will require a new storage domain format.
Note: this patch adds qemuimgTests.py since this file does not exists in 3.4. Only the tests relevant to this fix were backported.
Change-Id: Iffd45b394c49e8b12fb7a4cbaa5c7a3519a2cc1c Bug-Url: https://bugzilla.redhat.com/1142691 Signed-off-by: Nir Soffer nsoffer@redhat.com Reviewed-on: http://gerrit.ovirt.org/32836 Reviewed-by: Allon Mureinik amureini@redhat.com Reviewed-by: Federico Simoncelli fsimonce@redhat.com --- M lib/vdsm/qemuImg.py A tests/qemuimgTests.py 2 files changed, 109 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/33022/1
diff --git a/lib/vdsm/qemuImg.py b/lib/vdsm/qemuImg.py index e13524c..b99188c 100644 --- a/lib/vdsm/qemuImg.py +++ b/lib/vdsm/qemuImg.py @@ -33,6 +33,12 @@ RAW = "raw" VMDK = "vmdk"
+ +# Recent qemu-img supports two incompatible qcow2 versions. We use 0.10 format +# so hosts with older qemu can consume images created by newer versions. +# See https://bugzilla.redhat.com/1139707 +QCOW2_COMPAT = '0.10' + __iregex = { 'format': re.compile("^file format: (?P<value>\w+)$"), 'virtualsize': re.compile("^virtual size: " @@ -93,6 +99,8 @@
if format: cmd.extend(("-f", format)) + if format == FORMAT.QCOW2 and _supports_qcow2_compat(): + cmd.extend(('-o', 'compat=' + QCOW2_COMPAT))
if backing: cmd.extend(("-b", backing)) @@ -111,6 +119,24 @@ raise QImgError(rc, out, err)
+def _supports_qcow2_compat(): + """ + TODO: Remove this when qemu versions providing the "compat" option are + available on all platforms. + """ + cmd = [_qemuimg.cmd, "create", "-f", FORMAT.QCOW2, "-o", "?", "/dev/null"] + + rc, out, err = utils.execCmd(cmd, raw=True) + + if rc != 0: + raise QImgError(rc, out, err) + + # Supported options: + # compat Compatibility level (0.10 or 1.1) + + return '\ncompat ' in out + + def check(image, format=None): cmd = [_qemuimg.cmd, "check"]
diff --git a/tests/qemuimgTests.py b/tests/qemuimgTests.py new file mode 100644 index 0000000..587eb00 --- /dev/null +++ b/tests/qemuimgTests.py @@ -0,0 +1,83 @@ +# +# Copyright 2014 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 testlib import VdsmTestCase as TestCaseBase +from vdsm import qemuImg +from vdsm import utils + + +class FakeExecCmd(object): + + def __init__(self, *calls): + self.calls = list(calls) + self.saved = None + + def __call__(self, cmd, **kw): + call = self.calls.pop(0) + return call(cmd, **kw) + + def __enter__(self): + self.saved = utils.execCmd + utils.execCmd = self + + def __exit__(self, t=None, v=None, tb=None): + utils.execCmd = self.saved + + +class QemuimgCreateTests(TestCaseBase): + + def test_no_format(self): + + def create_no_format(cmd, **kw): + assert cmd == [qemuImg._qemuimg.cmd, 'create', 'image'] + return 0, '', '' + + with FakeExecCmd(create_no_format): + qemuImg.create('image') + + def test_qcow2_compat_not_supported(self): + + def qcow2_compat_not_supported(cmd, **kw): + assert cmd == [qemuImg._qemuimg.cmd, 'create', '-f', 'qcow2', '-o', + '?', '/dev/null'] + return 0, 'Supported options:\nsize ...\n', '' + + def create_qcow2_no_compat(cmd, **kw): + assert cmd == [qemuImg._qemuimg.cmd, 'create', '-f', 'qcow2', + 'image'] + return 0, '', '' + + with FakeExecCmd(qcow2_compat_not_supported, create_qcow2_no_compat): + qemuImg.create('image', format='qcow2') + + def test_qcow2_compat_supported(self): + + def qcow2_compat_supported(cmd, **kw): + assert cmd == [qemuImg._qemuimg.cmd, 'create', '-f', 'qcow2', '-o', + '?', '/dev/null'] + return 0, 'Supported options:\ncompat ...\n', '' + + def create_qcow2_compat(cmd, **kw): + assert cmd == [qemuImg._qemuimg.cmd, 'create', '-f', 'qcow2', '-o', + 'compat=0.10', 'image'] + return 0, '', '' + + with FakeExecCmd(qcow2_compat_supported, create_qcow2_compat): + qemuImg.create('image', format='qcow2')
Dan Kenigsberg has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/33022/1/lib/vdsm/qemuImg.py File lib/vdsm/qemuImg.py:
Line 118: if rc != 0: Line 119: raise QImgError(rc, out, err) Line 120: Line 121: Line 122: def _supports_qcow2_compat(): in a different patch: consider using @memoize to save some time. Line 123: """ Line 124: TODO: Remove this when qemu versions providing the "compat" option are Line 125: available on all platforms. Line 126: """
Federico Simoncelli has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/33022/1/lib/vdsm/qemuImg.py File lib/vdsm/qemuImg.py:
Line 118: if rc != 0: Line 119: raise QImgError(rc, out, err) Line 120: Line 121: Line 122: def _supports_qcow2_compat():
in a different patch: consider using @memoize to save some time.
no because if qemu-img is updated you're in trouble. Line 123: """ Line 124: TODO: Remove this when qemu versions providing the "compat" option are Line 125: available on all platforms. Line 126: """
Allon Mureinik has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1: Code-Review+1
Nir, can you verify?
Dan Kenigsberg has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/33022/1/lib/vdsm/qemuImg.py File lib/vdsm/qemuImg.py:
Line 118: if rc != 0: Line 119: raise QImgError(rc, out, err) Line 120: Line 121: Line 122: def _supports_qcow2_compat():
no because if qemu-img is updated you're in trouble.
I don't expect a qemu-0.10 -> qemu-1.1 upgrade while Vdsm is running.
But we can live with this repetitive check for a while. Line 123: """ Line 124: TODO: Remove this when qemu versions providing the "compat" option are Line 125: available on all platforms. Line 126: """
Federico Simoncelli has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1: Code-Review-1
-1 for visibility. This may not be enough:
https://bugzilla.redhat.com/show_bug.cgi?id=1139707#c26
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1:
I will send additional patch if we find that this is indeed not enough.
Federico Simoncelli has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1: Code-Review+1
We need another patch regardless because I trust Kevin's comment. If there's a glitch in qemu-img at the moment... it won't be there forever.
Allon Mureinik has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1:
@Dan/Fede/Yaniv - can we get this patch merged so 3.4 is aligned with 3.5 wrt qemuing?
The additional issues should be handled in a separate patch, and backported to all stable branches.
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1:
Since we don't ship this tomorrow, I think it would be better to wait for the other patch and merge both together.
I will send the patch soon and I don't see a reason why it cannot be merged upstream early next week.
Tal Nisan has posted comments on this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Patch Set 1:
Guys, can we go forward with this patch?
Nir Soffer has abandoned this change.
Change subject: qemuimg: Create qcow2 compat 0.10 images ......................................................................
Abandoned
Ovirt 3.4 does not accept patches any more. This patch is merged in rhev 3.4.
vdsm-patches@lists.fedorahosted.org