Nir Soffer has posted comments on this change. Change subject: core: Expose API for qemuimg commit ......................................................................
Patch Set 6: (5 comments) Partial review. https://gerrit.ovirt.org/#/c/64222/6/tests/qemuimg_test.py File tests/qemuimg_test.py: Line 335: @expandPermutations Line 336: class TestCommit(TestCaseBase): Line 337: Line 338: @MonkeyPatch(qemuimg, 'config', CONFIG) Line 339: @permutations([ We need a comment here for the argument # name1, name2, ... Line 340: (1, qemuimg.FORMAT.RAW, False), Line 341: (1, qemuimg.FORMAT.RAW), Line 342: (1, qemuimg.FORMAT.QCOW2, False), Line 343: (1, qemuimg.FORMAT.QCOW2), Line 337: Line 338: @MonkeyPatch(qemuimg, 'config', CONFIG) Line 339: @permutations([ Line 340: (1, qemuimg.FORMAT.RAW, False), Line 341: (1, qemuimg.FORMAT.RAW), Please keep the same number of arguments to make the format consistent and simple. Line 342: (1, qemuimg.FORMAT.QCOW2, False), Line 343: (1, qemuimg.FORMAT.QCOW2), Line 344: (3, qemuimg.FORMAT.RAW), Line 345: (3, qemuimg.FORMAT.QCOW2) Line 343: (1, qemuimg.FORMAT.QCOW2), Line 344: (3, qemuimg.FORMAT.RAW), Line 345: (3, qemuimg.FORMAT.QCOW2) Line 346: ]) Line 347: def test_commit(self, chainLen, baseFormat, useBacking=True): This looks like use_base, or with_base. backing is another term not related to commit. Line 348: size = 1048576 Line 349: with namedTemporaryDir() as tmpdir: Line 350: # create base Line 351: base = os.path.join(tmpdir, "base.img") Line 361: chain.append(vol) Line 362: parent = vol Line 363: Line 364: if useBacking: Line 365: op = qemuimg.commit(chain[chainLen-1], You can use: chain[-1] The last item. Line 366: topFormat=topFormat, base=base) Line 367: else: Line 368: op = qemuimg.commit(chain[chainLen-1], topFormat) Line 369: op.wait_for_completion() Line 364: if useBacking: Line 365: op = qemuimg.commit(chain[chainLen-1], Line 366: topFormat=topFormat, base=base) Line 367: else: Line 368: op = qemuimg.commit(chain[chainLen-1], topFormat) This is little ugly, can we have the same call, taking None as default value when we don't have a backing? For example, always use: op = qemuimg.commit(chain[-1], format, base=base) This call is equivalent to: op = qemuimg.commit(chain[-1], format) When base is None. Line 369: op.wait_for_completion() Line 370: Line 371: # verify base data at offset 0 after commit Line 372: qemu_pattern_verify(base, baseFormat, offset='0', len='1k', -- To view, visit https://gerrit.ovirt.org/64222 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If7a13be40541fb268541bd8614a642263b96b487 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
