Ala Hino has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
......................................................................


Patch Set 5:

(7 comments)

https://gerrit.ovirt.org/#/c/65112/5/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 459:             # Empty run
Line 460:             run = img_map[0]
Line 461:             self.assertEqual(run["length"], size)
Line 462:             self.assertEqual(run["data"], False)
Line 463:             self.assertEqual(run["zero"], True)
> Lets use the same format as in the other tests, and call the new shiny help
Done
Line 464: 
Line 465:     @permutations([
Line 466:         # offset, qcow2_compat
Line 467:         (4 * 1024, "0.10"),


Line 462:             self.assertEqual(run["data"], False)
Line 463:             self.assertEqual(run["zero"], True)
Line 464: 
Line 465:     @permutations([
Line 466:         # offset, qcow2_compat
> This is length, not offset.
Done
Line 467:         (4 * 1024, "0.10"),
Line 468:         (4 * 1024, "1.1"),
Line 469:         (64 * 1024, "0.10"),
Line 470:         (64 * 1024, "1.1"),


Line 479:             qemu_pattern_write(image, fmt, offset=offset, len=length,
Line 480:                                pattern=0xf0)
Line 481: 
Line 482:             img_map = qemuimg.map(image)
Line 483:             self.assertEqual(3, len(img_map))
> We can move this check into the helper:
Done
Line 484: 
Line 485:             expected = [
Line 486:                 # run 1 - empty
Line 487:                 {


Line 508:             ]
Line 509: 
Line 510:             for actual, expected in zip(img_map, expected):
Line 511:                 for key in expected:
Line 512:                     self.assertEqual(actual[key], expected[key])
> We have same verification code twice, and we actually want to use it for th
Done
Line 513: 
Line 514:     @permutations([
Line 515:         # offset, length, expected_length, expected_start, 
qcow2_compat
Line 516:         (64 * 1024, 4 * 1024, 65536, "0.10"),


Line 528:             qemu_pattern_write(image, fmt, offset=offset, len=length,
Line 529:                                pattern=0xf0)
Line 530: 
Line 531:             img_map = qemuimg.map(image)
Line 532:             self.assertEqual(3, len(img_map))
> Move to check_map helper
Done
Line 533: 
Line 534:             expected = [
Line 535:                 # run 1 - empty
Line 536:                 {


Line 541:                 },
Line 542:                 # run 2 - data
Line 543:                 {
Line 544:                     "start": offset,
Line 545:                     "offset": 327680,
> Lets add a comment abut this (no clue what is this value), or just remove i
Removed ...
Line 546:                     "length": expected_length,
Line 547:                     "data": True,
Line 548:                     "zero": False,
Line 549:                 },


Line 557:             ]
Line 558: 
Line 559:             for actual, expected in zip(img_map, expected):
Line 560:                 for key in expected:
Line 561:                     self.assertEqual(actual[key], expected[key])
> Repalce wiith call to check_map
Done
Line 562: 
Line 563: 
Line 564: def make_image(path, size, format, index, qcow2_compat, backing=None):
Line 565:     qemuimg.create(path, size=size, format=format, 
qcow2Compat=qcow2_compat,


-- 
To view, visit https://gerrit.ovirt.org/65112
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ah...@redhat.com>
Gerrit-Reviewer: Ala Hino <ah...@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

Reply via email to