Adam Litke has posted comments on this change. Change subject: extract a method for booting a test VM through kernel boot ......................................................................
Patch Set 5: I would prefer that you didn't submit this (2 inline comments) It's not ok to change the permissions of a file that your package does not own. .................................................... File tests/functional/xmlrpcTests.py Line 54: kernelVer) Line 55: Line 56: if os.path.isfile(initramfsPath): Line 57: # There is an initramfs shipped with the distro, use it Line 58: os.chmod(initramfsPath, 0644) You cannot do this. It is not okay to change the permissions of system files just so we can use them. Perhaps the system administrator did not want you to have access to that file. Line 59: try: Line 60: yield (kernelPath, initramfsPath) Line 61: finally: Line 62: pass Line 167: 'initrd': initramfsPath, Line 168: # The initramfs is generated by dracut. The following Line 169: # arguments will be interpreted by init scripts created Line 170: # by dracut. Line 171: 'kernelArgs': 'rd.break=cmdline rd.shell rd.skipfsck'}) You've added some distro-specific things here. You should consider breaking them out so that on other distributions the args would be easy to override. Line 172: try: Line 173: self.assertVdsOK(self.s.create(template)) Line 174: # wait 65 seconds for VM to come up until timeout Line 175: retry(assertVMAndGuestUp, timeout=65) -- To view, visit http://gerrit.ovirt.org/8414 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f2e94651d0279b19b1ce6849fb6f0c1b530a10c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Hunt Xu <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Royce Lv <[email protected]> Gerrit-Reviewer: Ryan Harper <[email protected]> Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
