Francesco Romani has posted comments on this change.

Change subject: virt: add device setup and teardown
......................................................................


Patch Set 7: Code-Review+1

(3 comments)

I've minor nits, but not worthy a resubmit. Full ACK on hold to avoid hiding 
other reviews.

https://gerrit.ovirt.org/#/c/55135/7/tests/vmTests.py
File tests/vmTests.py:

PS7, Line 1084:     conf = {'vmName': 'testVm',
              :             'vmId': '9ffe28b6-6134-4b1e-beef-1185f49c436f',
              :             'smp': '8',
              :             'maxVCpus': '160',
              :             'memSize': '1024',
              :             'memGuaranteedSize': '512',
              :             'devices': [],
              :             }
if you resubmit, please reformat as

  conf = {
    'vmName': 'testVm',
    ...


PS7, Line 1093:     def assertDeviceCalls(self, devices, expected_calls):
              :         for index, device in enumerate(devices):
              :             self.assertEquals(device.__calls__, 
expected_calls[index])
if you resubmit, I'd move this at the end of the class, below the actual tests.


https://gerrit.ovirt.org/#/c/55135/7/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS7, Line 1862: continue
useless, but I don't mind


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f99b855de43cff693b99b6873a835b7ad56db1b
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to