Adam Litke has posted comments on this change. Change subject: qemuimg: Handle new output format ......................................................................
Patch Set 6: -Verified (5 comments) http://gerrit.ovirt.org/#/c/27552/6/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py: Line 86: 'virtualsize': int(__iregexSearch("virtualsize", out[2])), Line 87: } Line 88: Line 89: # Scan for optional fields in the output Line 90: row = 4 > We probably need a constant here. E.g.: Done Line 91: for field, filterFn in (('clustersize', int), ('backingfile', str)): Line 92: try: Line 93: info[field] = filterFn(__iregexSearch(field, out[row])) Line 94: row = row + 1 Line 94: row = row + 1 Line 95: except (_RegexSearchError, IndexError): Line 96: pass Line 97: except _RegexSearchError: Line 98: raise QImgError(rc, out, err, "unable to parse qemu-img info output") > this seems easy enough to be fixed now, we just need to move the except blo Done Line 99: Line 100: return info Line 101: Line 102: http://gerrit.ovirt.org/#/c/27552/6/tests/qemuimgTests.py File tests/qemuimgTests.py: Line 90: Line 91: @monkeypatch.MonkeyPatch(utils, 'execCmd', Line 92: partial(fakeCmd, outputQemu1NoBackingFile)) Line 93: def testQemu1NoBackingFile(self): Line 94: info = qemuimg.info('foo') > I'd rather use 'unused' or something like it, but this is very minor, don't Done Line 95: self.assertFalse('backingfile' in info) Line 96: Line 97: @monkeypatch.MonkeyPatch(utils, 'execCmd', Line 98: partial(fakeCmd, outputQemu1Backing)) Line 91: @monkeypatch.MonkeyPatch(utils, 'execCmd', Line 92: partial(fakeCmd, outputQemu1NoBackingFile)) Line 93: def testQemu1NoBackingFile(self): Line 94: info = qemuimg.info('foo') Line 95: self.assertFalse('backingfile' in info) > assertNotIn was backported, we can make use of it. Done Line 96: Line 97: @monkeypatch.MonkeyPatch(utils, 'execCmd', Line 98: partial(fakeCmd, outputQemu1Backing)) Line 99: def testQemu1Backing(self): Line 106: info = qemuimg.info('foo') Line 107: self.assertEquals('qcow2', info['format']) Line 108: self.assertEquals(1073741824, info['virtualsize']) Line 109: self.assertEquals(65536, info['clustersize']) Line 110: self.assertFalse('backingfile' in info) > I believe it would be better to do like we do in vmTestsData. ok, let's defer this one. Line 111: Line 112: @monkeypatch.MonkeyPatch(utils, 'execCmd', Line 113: partial(fakeCmd, outputQemu2BackingNoCluster)) Line 114: def testQemu2BackingNoCluster(self): -- To view, visit http://gerrit.ovirt.org/27552 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic229198ab7c2bb9743bdf8629416131186115431 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
