Yaniv Bronhaim has posted comments on this change.

Change subject: add simple VM creation functional test
......................................................................


Patch Set 8: Verified; I would prefer that you didn't submit this

You right. All tests pass both in rhel and fedora (with kvm support of course.. 
you should detect if kvm is enabled alright)

The errors I mentioned in my last comment related to fake qemu support. after 
trying to enable everything that needs to be enable for fake qemu, i tried to 
run your tests over rhel\fedora guests. 
I still have some errors about loading kvm modules. until i fix it, i think 
that patch that verifies that kvm is enabled before running the test will be 
very helpful.

about the code: 
1. can you add comments on your assert functions so we will be able to 
understand the meaning for each function without or before trying to follow the 
code ?
2. are you sure that this the right place to add those assert functions? maybe 
you can add them to external py file so other uts will use it. maybe it'll be 
useful for more developers in the future... and they won't look for it in 
xmlrpcTests.py file.
3. please add check that we run the tests as root (because otherwise it fails) 
and that kvm module is enables.

--
To view, visit http://gerrit.ovirt.org/7396
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb0d86ce20a547ef809d5407fe12d6ade474c4d2
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Gal Hammer <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Royce Lv <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to