Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add QEMU mode #402

Closed
wants to merge 1 commit into from
Closed

add QEMU mode #402

wants to merge 1 commit into from

Conversation

tomirgang
Copy link
Contributor

This PR allows to use a local QEMU VM without the need to use libvirt.

This change makes use of elbe in containers much more easy, since libvirtd is hard to containerize and comes with a long list of dependencies and requirements.

Copy link
Contributor

@t-8ch t-8ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

FYI it is a general goal of me to simplify the initvm (and maybe also remove it somehow)
Running in containers is also a general goal.

elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
@jogness
Copy link
Contributor

jogness commented Mar 12, 2024

FYI it is a general goal of me to simplify the initvm (and maybe also remove it somehow) Running in containers is also a general goal.

Note that ELBE originally did not use a virtual machine. This change was in 2012 (unfortunately pre-github and pre-mailing list). The project started in 2009. The main reasons why the ELBE team decided to change the build environment to be in a virtual machine:

  • Minimizing dependencies so that ELBE can run on different Linux distributions and operating systems (even native Windows is supported).
  • Having full control over dependencies so that ELBE can build reliably on any host system.
  • Running foreign-arch software requires binfmt support.
  • Avoid pulling in build host details from /proc /sys /dev.
  • Use root for many tasks without risks to host.

Perhaps some of these requirements have changed in the last decade. The available technologies certainly have changed. I just wanted to point out these reasons because they tend to get lost when a project switches maintainers a few times.

BTW, I still have many interesting emails about ELBE design requirements that were posted to the elbe-devel mailing list (before the list was archiving). It may be of use to look them over to avoid repeating the same mistakes we made 15 years ago. It may also be of value to create some sort of manifesto so that the goals/requirements are not lost 10 years from now.

@tomirgang
Copy link
Contributor Author

tomirgang commented Mar 15, 2024

FYI it is a general goal of me to simplify the initvm (and maybe also remove it somehow) Running in containers is also a general goal.

Note that ELBE originally did not use a virtual machine. This change was in 2012 (unfortunately pre-github and pre-mailing list). The project started in 2009. The main reasons why the ELBE team decided to change the build environment to be in a virtual machine:

  • Minimizing dependencies so that ELBE can run on different Linux distributions and operating systems (even native Windows is supported).
  • Having full control over dependencies so that ELBE can build reliably on any host system.
  • Running foreign-arch software requires binfmt support.
  • Avoid pulling in build host details from /proc /sys /dev.
  • Use root for many tasks without risks to host.

Perhaps some of these requirements have changed in the last decade. The available technologies certainly have changed. I just wanted to point out these reasons because they tend to get lost when a project switches maintainers a few times.

BTW, I still have many interesting emails about ELBE design requirements that were posted to the elbe-devel mailing list (before the list was archiving). It may be of use to look them over to avoid repeating the same mistakes we made 15 years ago. It may also be of value to create some sort of manifesto so that the goals/requirements are not lost 10 years from now.

The initvm is from my point of view a good thing, and wit the QEMU kvm and user-mode combination the resulting build performance is quite good.

My challenge is that libvirt is hard to use in a VS Code devcontainer (but it's possible, I have a working PoC), and QEMU kvm without libvirt is very easy to use (only a privileged container is needed). Without kvm not even a priviledged container is needed, but then the build performance is horrible slow

@tomirgang
Copy link
Contributor Author

@t-8ch Your review findings are now fixed form my point of view. Please tell me if you don't agree.

elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/shellhelper.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
@t-8ch
Copy link
Contributor

t-8ch commented Mar 18, 2024

The initvm is from my point of view a good thing, and wit the QEMU kvm and user-mode combination the resulting build performance is quite good.

It is indeed a good thing. My hope is to reach all of its goals but avoid some of the problems the current implementation incurs.

Please tell me if you don't agree.

A few things still need some work, please see my comments.

elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
@tomirgang tomirgang force-pushed the QEMU_mode branch 4 times, most recently from 67c4d05 to 72f5f6a Compare March 20, 2024 20:28
@tomirgang
Copy link
Contributor Author

@t-8ch Rebase and rework done. Please review again.

elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/shellhelper.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
@tomirgang
Copy link
Contributor Author

@t-8ch Most of your remarks are fixed, but I don't think it should be part of this PR to implement a shell client.

Copy link
Contributor

@t-8ch t-8ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just some more tiny nitpicks.
Could you also clean up the commit history?

As for the shellclient:
I'll implement one myself after your changes are merged.
It would be great if you can send the patch to you for testing then.

elbepack/initvmaction.py Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
elbepack/initvmaction.py Outdated Show resolved Hide resolved
@tomirgang
Copy link
Contributor Author

Thanks! Just some more tiny nitpicks. Could you also clean up the commit history?

As for the shellclient: I'll implement one myself after your changes are merged. It would be great if you can send the patch to you for testing then.

Done.

I can test the shell client if you want to implement one.
Form my point of view, it would be a better way forward to drop the attach command or only print a help text how to attach, instead of implementing unrelated features. If you want to have a robust shellclient, which is handling all edge cases and special characters correctly (e.g. Ctrl + C), I guess it will become quite complex.

@tomirgang tomirgang force-pushed the QEMU_mode branch 8 times, most recently from 2e75057 to 83c1b4f Compare March 23, 2024 18:00
@t-8ch
Copy link
Contributor

t-8ch commented Mar 25, 2024

Done.

Thanks!

If you want to have a robust shellclient, which is handling all edge cases and special characters correctly

My idea was that all characters are blindly forwarded without any special handling. Ctrl-C should not be a character at all but an exception that gets generated by python itself.

Copy link
Contributor

@t-8ch t-8ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also check the code style with flake8 elbepack/ --max-line-length 100

elbepack/initvmaction.py Show resolved Hide resolved
elbepack/init/Makefile.mako Show resolved Hide resolved
@tomirgang
Copy link
Contributor Author

Done.

Thanks!

If you want to have a robust shellclient, which is handling all edge cases and special characters correctly

My idea was that all characters are blindly forwarded without any special handling. Ctrl-C should not be a character at all but an exception that gets generated by python itself.

From my point of view, this is not a good idea. Not being able to cancel or sleep shell processes in the VM is a huge restriction.

Adds an option to run QEMU directly instead of using libvirt to start
the initvm.

Signed-off-by: Tom Irgang <me@tomirgang.de>
@tomirgang
Copy link
Contributor Author

flake8 elbepack/ --max-line-length 100

Done.

@t-8ch
Copy link
Contributor

t-8ch commented Mar 26, 2024

Toplevel note (to myself):
This is used for devcontainer: https://github.com/tomirgang/elbe_dev_container/tree/main/.devcontainer

@t-8ch t-8ch closed this in bcc4f42 Mar 26, 2024
@t-8ch
Copy link
Contributor

t-8ch commented Mar 26, 2024

Thanks! I picked this up manually to add my review tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants