Change in vdsm[ovirt-3.5]: qemuimg: Handle new output format

alitke at redhat.com alitke at redhat.com
Tue Sep 2 17:20:43 UTC 2014


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/32321

to review the following change.

Change subject: qemuimg: Handle new output format
......................................................................

qemuimg: Handle new output format

The current qemu-img info parsing logic assumes that if the output is
long enough, the backing image will appear on line 6.  In newer qemu
versions, some additional information has been added to the output.  In
the case of no backing file, the additional lines will fail to parse.
Handle this by trying to parse line 6 as a backing file and ignoring
regex parse errors for this line only.

Sample output from qemu-2.0.0-rc1:

[alitke at lager qemu-2.0.0-rc1]$ ./qemu-img info base.img
image: base.img
file format: qcow2
virtual size: 1.0G (1073741824 bytes)
disk size: 196K
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false
[alitke at lager qemu-2.0.0-rc1]$ ./qemu-img info leaf.img
image: leaf.img
file format: qcow2
virtual size: 1.0G (1073741824 bytes)
disk size: 196K
cluster_size: 65536
backing file: base.img
Format specific information:
    compat: 1.1
    lazy refcounts: false

Change-Id: Ic229198ab7c2bb9743bdf8629416131186115431
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1127294
Signed-off-by: Adam Litke <alitke at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/27552
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M lib/vdsm/qemuimg.py
M tests/Makefile.am
A tests/qemuimgTests.py
3 files changed, 138 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/32321/1

diff --git a/lib/vdsm/qemuimg.py b/lib/vdsm/qemuimg.py
index 28ecc87..3cdec8d 100644
--- a/lib/vdsm/qemuimg.py
+++ b/lib/vdsm/qemuimg.py
@@ -44,9 +44,18 @@
     'offset': re.compile("^Image end offset: (?P<value>\d+)$"),
 }
 
+INFO_OPTFIELDS_STARTIDX = 4  # qemu-img info optional fields start in this line
+
+
+class _RegexSearchError(Exception):
+    pass
+
 
 def __iregexSearch(pattern, text):
-    return __iregex[pattern].search(text).group("value")
+    m = __iregex[pattern].search(text)
+    if m is None:
+        raise _RegexSearchError()
+    return m.group("value")
 
 
 class QImgError(Exception):
@@ -78,15 +87,19 @@
             'format': __iregexSearch("format", out[1]),
             'virtualsize': int(__iregexSearch("virtualsize", out[2])),
         }
-
-        if len(out) > 4:
-            info['clustersize'] = int(__iregexSearch("clustersize", out[4]))
-
-        if len(out) > 5:
-            info['backingfile'] = __iregexSearch("backingfile", out[5])
-    except:
+    except _RegexSearchError:
         raise QImgError(rc, out, err, "unable to parse qemu-img info output")
 
+    # Scan for optional fields in the output
+    row = INFO_OPTFIELDS_STARTIDX
+    for field, filterFn in (('clustersize', int), ('backingfile', str)):
+        try:
+            info[field] = filterFn(__iregexSearch(field, out[row]))
+        except (_RegexSearchError, IndexError):
+            pass
+        else:
+            row = row + 1
+
     return info
 
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6507165..378a69f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -62,6 +62,7 @@
 	permutationTests.py \
 	persistentDictTests.py \
 	profileTests.py \
+	qemuimgTests.py \
 	remoteFileHandlerTests.py \
 	resourceManagerTests.py \
 	samplingTests.py \
diff --git a/tests/qemuimgTests.py b/tests/qemuimgTests.py
new file mode 100644
index 0000000..4a35694
--- /dev/null
+++ b/tests/qemuimgTests.py
@@ -0,0 +1,116 @@
+#
+# 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
+#
+
+import textwrap
+from functools import partial
+
+from testrunner import VdsmTestCase as TestCaseBase
+import monkeypatch
+from vdsm import qemuimg
+from vdsm import utils
+
+
+def fakeCmd(txt, *args, **kwargs):
+    txt = textwrap.dedent(txt).split('\n')
+    if txt[0] == '':
+        txt.pop(0)
+    return (0, txt, '')
+
+
+outputParseError = """
+foo bar
+"""
+
+
+outputQemu1NoBackingFile = """
+image: base.img
+file format: qcow2
+virtual size: 1.0G (1073741824 bytes)
+disk size: 196K
+cluster_size: 65536
+"""
+
+
+outputQemu1Backing = """
+image: leaf.img
+file format: qcow2
+virtual size: 1.0G (1073741824 bytes)
+disk size: 196K
+cluster_size: 65536
+backing file: base.img (actual path: /tmp/base.img)
+"""
+
+
+outputQemu2NoBackingFile = """
+image: base.img
+file format: qcow2
+virtual size: 1.0G (1073741824 bytes)
+disk size: 196K
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+"""
+
+
+outputQemu2BackingNoCluster = """
+image: leaf.img
+file format: qcow2
+virtual size: 1.0G (1073741824 bytes)
+disk size: 196K
+backing file: base.img (actual path: /tmp/base.img)
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+"""
+
+
+class qemuimgTests(TestCaseBase):
+    @monkeypatch.MonkeyPatch(utils, 'execCmd',
+                             partial(fakeCmd, outputParseError))
+    def testParseError(self):
+        self.assertRaises(qemuimg.QImgError, qemuimg.info, 'unused')
+
+    @monkeypatch.MonkeyPatch(utils, 'execCmd',
+                             partial(fakeCmd, outputQemu1NoBackingFile))
+    def testQemu1NoBackingFile(self):
+        info = qemuimg.info('unused')
+        self.assertNotIn('backingfile', info)
+
+    @monkeypatch.MonkeyPatch(utils, 'execCmd',
+                             partial(fakeCmd, outputQemu1Backing))
+    def testQemu1Backing(self):
+        info = qemuimg.info('unused')
+        self.assertEquals('base.img', info['backingfile'])
+
+    @monkeypatch.MonkeyPatch(utils, 'execCmd',
+                             partial(fakeCmd, outputQemu2NoBackingFile))
+    def testQemu2NoBackingFile(self):
+        info = qemuimg.info('unused')
+        self.assertEquals('qcow2', info['format'])
+        self.assertEquals(1073741824, info['virtualsize'])
+        self.assertEquals(65536, info['clustersize'])
+        self.assertNotIn('backingfile', info)
+
+    @monkeypatch.MonkeyPatch(utils, 'execCmd',
+                             partial(fakeCmd, outputQemu2BackingNoCluster))
+    def testQemu2BackingNoCluster(self):
+        info = qemuimg.info('unused')
+        self.assertEquals('base.img', info['backingfile'])


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic229198ab7c2bb9743bdf8629416131186115431
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list