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@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@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(a)redhat.com>
Reviewed-on:
http://gerrit.ovirt.org/27552
Reviewed-by: Dan Kenigsberg <danken(a)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(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>