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 <ahino(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes