cyris
cyris copied to clipboard
Ports Cyris to Ubuntu 20.04 and Python 3, fixes several/potential bugs
As title says, I would like to share the changes made to make Cyris work with Ubuntu 20.04 with Python 3 (tested with Python 3.8).
- Iptables related changes: Opens both given TCP and UDP ports,
main/entities.py:551 - Waiting for host shutting down: The fix for a bug in the logic,
main/cyris.py:1391. - Making Cyris work under new version of QEMU in
instantiation/vm_clone/vm_clone_xml.sh:31 - PEP8 compliance and encoding related changes
- I also confirm that this version worked for me in a nested virtualization setting (i.e. Ubuntu 20.04 Host -> Ubuntu 20.40 Cyris Guest -> VMs created by Cyris).
- TODO: I had issues with some multiple host related commands, I disabled one in
main/cyris.py:1457.
Hopefully, this will be useful for somebody! :)
Dear Taha Doğan Güneş,
Thank you for your contribution, we really appreciate your wish to help. I have checked your pull request, and unfortunately we cannot accept it as it is. The main reasons are the following:
- The changes you made to support Ubuntu 20.04 in HOST-PREPARE.sh break compatibility with older OSes which we would still like to support, such as Ubuntu 18.04 and even 16.04. One idea could be to prepare a new script just for Ubuntu 20.04, and commit it in addition to the current HOST-PREPARE.sh.
- Your commits are somewhat difficult to review because several changes are not related to the purpose of the commit, as they seem to be simple reformatting of the code, done perhaps by your IDE. I suggest you disable such automatic reformatting in the future to minimize the differences between your commit and the original code.
The contribution that would be most welcome is related to adding compatibility with Python 3, as I believe these changes do not introduce any incompatibility with Python 2, which we still use on several servers. If you are willing to put some more effort, I would suggest you make another commit that contains only the Python 2 to 3 compatibility modifications and resubmit.
I understand that your main goal is to make the code running for you, but as main developers and maintainers we have to consider compatibility issues very carefully, and we also must review the code carefully for security reasons. We are looking forward to any future contributions that you have the time to make. As a side comment, we'd also be interested in knowing more about how you are using our system, as your feedback could be useful for future development.
Best wishes, Razvan
Hi Razvan,
You are welcome and thanks for checking the pull request. Regarding the issues that you mentioned:
- Having separate bash script
HOST-PREPARE.shfor each version of Ubuntu is a good idea. However, the changes ininstantiation/vm_clone/vm_clone_xml.shneeds to be reverted when an older version of Ubuntu is used. - Regarding commits being difficult to review, the original code was not
PEP8compliant. This made porting to Python 3 harder, when I used2to3. Therefore, I runautopep8which fixed issues that are described here before porting semi-manually with2to3. I think if you useautopep8to fix original codebase and get adiffbetween this pull request, it should make reviewing the changes easier. The main changes are the ones mentioned in my first comment. - Regarding Python 2 compatibility, I think this requires looking into using
futuresmodule to make Cyris work in both versions of Python.
About making additional changes to this PR, unfortunately I am very occupied nowadays. But if the time allows, I may be able to share some feedback, and hopefully may be able to get back to this PR.
Thanks again for Cyris, it has been quite useful!
Best, Taha.
Hi Taha,
I understand about issue 1, it seems to be more complicated than I thought. As for 2 and 3, we'll take into account your advice when we'll consider this aspect. Sharing some feedback when you have time is perfectly fine. Good luck with your research!
Best wishes, Razvan