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

Merge guest binaries. #134

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

WhatAmISupposedToPutHere
Copy link
Collaborator

Merge all guest binaries into one binary and process. This removes sommelier support, but people should use x11bridge anyway. Also stop leaking muvm configuration into vm processes.

crates/muvm/src/bin/muvm.rs Outdated Show resolved Hide resolved
crates/muvm/src/guest/bin/muvm-guest.rs Outdated Show resolved Hide resolved
crates/muvm/src/guest/bin/muvm-guest.rs Outdated Show resolved Hide resolved
crates/muvm/src/bin/muvm.rs Outdated Show resolved Hide resolved
crates/muvm/src/guest/bin/muvm-guest.rs Outdated Show resolved Hide resolved
crates/muvm/src/server/worker.rs Outdated Show resolved Hide resolved
crates/muvm/src/guest/x11.rs Show resolved Hide resolved
crates/muvm/src/guest/hidpipe.rs Outdated Show resolved Hide resolved
@asahilina
Copy link
Member

FYI, I left a TODO in #130 about moving an env var to the muvm-guest config. If that gets merged first, please make that change too as part of the last commit here when you rebase.

It is in the way for a refactor i want to do, and you should be
using x11bridge anyway.

Signed-off-by: Sasha Finkelstein <[email protected]>
@WhatAmISupposedToPutHere WhatAmISupposedToPutHere force-pushed the the-merge branch 2 times, most recently from 9b974f5 to f614c5f Compare January 9, 2025 13:06
@asahilina
Copy link
Member

Needs clippy fixes (probably just an #[allow(clippy::too_many_arguments)] in the problem functions).

We can keep the root uid in our saved set user id, and copy it to ruid/euid
after fork, if launching as root is requested. Writing to drop_caches becomes
a bit tricker when running unprivileged, but we can acquire the privileges in
a forked process for it to work and not affect other launches.

Signed-off-by: Sasha Finkelstein <[email protected]>
We no longer need the server to be a separate binary.

Signed-off-by: Sasha Finkelstein <[email protected]>
@sbrivio-rh
Copy link
Contributor

sbrivio-rh commented Jan 17, 2025

Ah, thanks, it works headless again. It's also a bit faster now. Tried with #111, without these commits:

$ time for i in `seq 1 10`; do ./target/debug/muvm --sommelier -- true; done

real	0m10.397s
user	0m7.653s
sys	0m3.165s

with:

$ time for i in `seq 1 10`; do ./target/debug/muvm -- true; done

real    0m10.100s
user    0m7.162s
sys     0m2.638s

By the way:

$ time for i in `seq 1 10`; do ./target/debug/muvm --passt-socket=/tmp/passt_3.socket -- true; done

real	0m9.808s
user	0m7.053s
sys	0m2.725s

...not really substantial.

Edit:

$ time for i in `seq 1 10`; do ./target/release/muvm --mem=64 --vram=0 -c 0 -- true; done

real	0m5.049s
user	0m3.135s
sys	0m0.361s

...neat, this.

@sbrivio-rh
Copy link
Contributor

Whee, it comes with --privileged too! \o/

@WhatAmISupposedToPutHere
Copy link
Collaborator Author

It being a tiny bit faster is expected (exec is not free), but is not the reason for those changes.

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.

4 participants