Rewrite it in Rust #4

teohhanhui · 2024-04-26T22:45:51Z

Bugs fixed:

Current implementation of krun incorrectly passes own arguments to krun-guest, e.g.

teohhanhui@Han-MacBook-Air:~/Projects/slp/krun$ ./krun --net=PASST box64 --help
No IPv6 nameserver available for NDP/DHCPv6
Couldn't execute '--net=PASST' inside the vm: No such file or directory

Stop using hardcoded /tmp/passt_1.socket
When forking krun-guest to run dhclient in child process, stdin/stdout/stderr were illegally closed.

Usage of close() on file descriptors STDIN_FILENO, STDOUT_FILENO, or
STDERR_FILENO should immediately be followed by an operation to reopen these
file descriptors. Unexpected behavior will result if any of these file
descriptors is left in a closed state

See https://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html#tag_16_67_07
File exists (os error 17) when trying to create the temp directory for XDG_RUNTIME_DIR after the first run.
Use properly unique temp dir names for each run.

Dependency tree with cargo tree -e normal --no-dedupe (excluding build-dependencies):

krun v0.1.0 (/home/teohhanhui/Projects/slp/krun/crates/krun)
├── anyhow v1.0.82
├── bpaf v0.9.11
├── libkrun v1.8.1 (/home/teohhanhui/Projects/slp/krun/crates/libkrun)
├── nix v0.28.0
│   ├── bitflags v2.5.0
│   ├── cfg-if v1.0.0
│   └── libc v0.2.153
├── rustix v0.38.34
│   ├── bitflags v2.5.0
│   └── linux-raw-sys v0.4.13
├── tracing v0.1.40
│   ├── pin-project-lite v0.2.14
│   ├── tracing-attributes v0.1.27 (proc-macro)
│   │   ├── proc-macro2 v1.0.81
│   │   │   └── unicode-ident v1.0.12
│   │   ├── quote v1.0.36
│   │   │   └── proc-macro2 v1.0.81
│   │   │       └── unicode-ident v1.0.12
│   │   └── syn v2.0.60
│   │       ├── proc-macro2 v1.0.81
│   │       │   └── unicode-ident v1.0.12
│   │       ├── quote v1.0.36
│   │       │   └── proc-macro2 v1.0.81
│   │       │       └── unicode-ident v1.0.12
│   │       └── unicode-ident v1.0.12
│   └── tracing-core v0.1.32
│       └── once_cell v1.19.0
├── tracing-subscriber v0.3.18
│   ├── matchers v0.1.0
│   │   └── regex-automata v0.1.10
│   │       └── regex-syntax v0.6.29
│   ├── nu-ansi-term v0.46.0
│   │   └── overload v0.1.1
│   ├── once_cell v1.19.0
│   ├── regex v1.10.4
│   │   ├── regex-automata v0.4.6
│   │   │   └── regex-syntax v0.8.3
│   │   └── regex-syntax v0.8.3
│   ├── sharded-slab v0.1.7
│   │   └── lazy_static v1.4.0
│   ├── smallvec v1.13.2
│   ├── thread_local v1.1.8
│   │   ├── cfg-if v1.0.0
│   │   └── once_cell v1.19.0
│   ├── tracing v0.1.40
│   │   ├── pin-project-lite v0.2.14
│   │   ├── tracing-attributes v0.1.27 (proc-macro)
│   │   │   ├── proc-macro2 v1.0.81
│   │   │   │   └── unicode-ident v1.0.12
│   │   │   ├── quote v1.0.36
│   │   │   │   └── proc-macro2 v1.0.81
│   │   │   │       └── unicode-ident v1.0.12
│   │   │   └── syn v2.0.60
│   │   │       ├── proc-macro2 v1.0.81
│   │   │       │   └── unicode-ident v1.0.12
│   │   │       ├── quote v1.0.36
│   │   │       │   └── proc-macro2 v1.0.81
│   │   │       │       └── unicode-ident v1.0.12
│   │   │       └── unicode-ident v1.0.12
│   │   └── tracing-core v0.1.32
│   │       └── once_cell v1.19.0
│   ├── tracing-core v0.1.32
│   │   └── once_cell v1.19.0
│   └── tracing-log v0.2.0
│       ├── log v0.4.21
│       ├── once_cell v1.19.0
│       └── tracing-core v0.1.32
│           └── once_cell v1.19.0
└── utils v0.0.0 (/home/teohhanhui/Projects/slp/krun/crates/utils)
    └── tracing v0.1.40
        ├── pin-project-lite v0.2.14
        ├── tracing-attributes v0.1.27 (proc-macro)
        │   ├── proc-macro2 v1.0.81
        │   │   └── unicode-ident v1.0.12
        │   ├── quote v1.0.36
        │   │   └── proc-macro2 v1.0.81
        │   │       └── unicode-ident v1.0.12
        │   └── syn v2.0.60
        │       ├── proc-macro2 v1.0.81
        │       │   └── unicode-ident v1.0.12
        │       ├── quote v1.0.36
        │       │   └── proc-macro2 v1.0.81
        │       │       └── unicode-ident v1.0.12
        │       └── unicode-ident v1.0.12
        └── tracing-core v0.1.32
            └── once_cell v1.19.0

krun-guest v0.1.0 (/home/teohhanhui/Projects/slp/krun/crates/krun-guest)
├── anyhow v1.0.82
├── bpaf v0.9.11
├── nix v0.28.0
│   ├── bitflags v2.5.0
│   ├── cfg-if v1.0.0
│   └── libc v0.2.153
├── rustix v0.38.34
│   ├── bitflags v2.5.0
│   └── linux-raw-sys v0.4.13
├── tempfile v3.10.1
│   ├── cfg-if v1.0.0
│   ├── fastrand v2.1.0
│   └── rustix v0.38.34
│       ├── bitflags v2.5.0
│       └── linux-raw-sys v0.4.13
├── tracing v0.1.40
│   ├── pin-project-lite v0.2.14
│   ├── tracing-attributes v0.1.27 (proc-macro)
│   │   ├── proc-macro2 v1.0.81
│   │   │   └── unicode-ident v1.0.12
│   │   ├── quote v1.0.36
│   │   │   └── proc-macro2 v1.0.81
│   │   │       └── unicode-ident v1.0.12
│   │   └── syn v2.0.60
│   │       ├── proc-macro2 v1.0.81
│   │       │   └── unicode-ident v1.0.12
│   │       ├── quote v1.0.36
│   │       │   └── proc-macro2 v1.0.81
│   │       │       └── unicode-ident v1.0.12
│   │       └── unicode-ident v1.0.12
│   └── tracing-core v0.1.32
│       └── once_cell v1.19.0
├── tracing-subscriber v0.3.18
│   ├── matchers v0.1.0
│   │   └── regex-automata v0.1.10
│   │       └── regex-syntax v0.6.29
│   ├── nu-ansi-term v0.46.0
│   │   └── overload v0.1.1
│   ├── once_cell v1.19.0
│   ├── regex v1.10.4
│   │   ├── regex-automata v0.4.6
│   │   │   └── regex-syntax v0.8.3
│   │   └── regex-syntax v0.8.3
│   ├── sharded-slab v0.1.7
│   │   └── lazy_static v1.4.0
│   ├── smallvec v1.13.2
│   ├── thread_local v1.1.8
│   │   ├── cfg-if v1.0.0
│   │   └── once_cell v1.19.0
│   ├── tracing v0.1.40
│   │   ├── pin-project-lite v0.2.14
│   │   ├── tracing-attributes v0.1.27 (proc-macro)
│   │   │   ├── proc-macro2 v1.0.81
│   │   │   │   └── unicode-ident v1.0.12
│   │   │   ├── quote v1.0.36
│   │   │   │   └── proc-macro2 v1.0.81
│   │   │   │       └── unicode-ident v1.0.12
│   │   │   └── syn v2.0.60
│   │   │       ├── proc-macro2 v1.0.81
│   │   │       │   └── unicode-ident v1.0.12
│   │   │       ├── quote v1.0.36
│   │   │       │   └── proc-macro2 v1.0.81
│   │   │       │       └── unicode-ident v1.0.12
│   │   │       └── unicode-ident v1.0.12
│   │   └── tracing-core v0.1.32
│   │       └── once_cell v1.19.0
│   ├── tracing-core v0.1.32
│   │   └── once_cell v1.19.0
│   └── tracing-log v0.2.0
│       ├── log v0.4.21
│       ├── once_cell v1.19.0
│       └── tracing-core v0.1.32
│           └── once_cell v1.19.0
└── utils v0.0.0 (/home/teohhanhui/Projects/slp/krun/crates/utils)
    └── tracing v0.1.40
        ├── pin-project-lite v0.2.14
        ├── tracing-attributes v0.1.27 (proc-macro)
        │   ├── proc-macro2 v1.0.81
        │   │   └── unicode-ident v1.0.12
        │   ├── quote v1.0.36
        │   │   └── proc-macro2 v1.0.81
        │   │       └── unicode-ident v1.0.12
        │   └── syn v2.0.60
        │       ├── proc-macro2 v1.0.81
        │       │   └── unicode-ident v1.0.12
        │       ├── quote v1.0.36
        │       │   └── proc-macro2 v1.0.81
        │       │       └── unicode-ident v1.0.12
        │       └── unicode-ident v1.0.12
        └── tracing-core v0.1.32
            └── once_cell v1.19.0

libkrun v1.8.1 (/home/teohhanhui/Projects/slp/krun/crates/libkrun)

utils v0.0.0 (/home/teohhanhui/Projects/slp/krun/crates/utils)
└── tracing v0.1.40
    ├── pin-project-lite v0.2.14
    ├── tracing-attributes v0.1.27 (proc-macro)
    │   ├── proc-macro2 v1.0.81
    │   │   └── unicode-ident v1.0.12
    │   ├── quote v1.0.36
    │   │   └── proc-macro2 v1.0.81
    │   │       └── unicode-ident v1.0.12
    │   └── syn v2.0.60
    │       ├── proc-macro2 v1.0.81
    │       │   └── unicode-ident v1.0.12
    │       ├── quote v1.0.36
    │       │   └── proc-macro2 v1.0.81
    │       │       └── unicode-ident v1.0.12
    │       └── unicode-ident v1.0.12
    └── tracing-core v0.1.32
        └── once_cell v1.19.0

Requires containers/libkrun#181

teohhanhui · 2024-04-27T12:07:34Z

I think we can remove the errno dependency by not calling set_errno (we're not a library so it's pointless for us to set errno anyway...)

If I understand correctly, errno was only being assigned to for the side effect in perror, i.e. to print out the human-readable error message corresponding to the errno.

rustix already has us covered for that use case.

EDIT: Done.

slp · 2024-04-29T08:45:17Z

I think it's looking good so far, thanks for working on this. Do you plan to rewrite krun-guest too or do you plan on keeping it as C?

teohhanhui · 2024-04-29T11:42:43Z

I plan to rewrite it too, but I'll only find time in the next few days.

teohhanhui · 2024-05-01T15:16:56Z

krun-guest is now tested to be working. Only exec_sommelier is not implemented yet...

EDIT: Done.

src/net.rs

slp · 2024-05-02T09:22:37Z

@teohhanhui This is looking really good so far! Is there anything else remaining to be done apart for the fork+exec replacement for passt?

slp · 2024-05-02T09:24:44Z

@teohhanhui I'm also thinking about renaming this repo as krun.legacy and creating a clean one with just README.md and LICENSE. Would you mind rebasing your PR to that one?

teohhanhui · 2024-05-02T09:26:14Z

I think there's value in keeping the commit history, but it's your call.

teohhanhui · 2024-05-02T09:36:51Z

Is there anything else remaining to be done apart for the fork+exec replacement for passt?

There's the hardcoded /home/{username} for HOME env var. Should we pass in the actual path from getpwnam_r to krun-guest instead (already used in krun)?

There's also the TODO that you've noted about only overriding the mesa driver selection on Asahi Linux. Is it possible for us to set that in krun instead (through envp in krun_set_exec)?

Other than that, it's housekeeping stuff like updating the GitHub Actions workflows and removing the old code etc. (if you choose to keep the commit history).

Cargo.toml

slp · 2024-05-05T09:20:14Z

Cargo.toml

+[dependencies]
+anyhow = "1.0.82"
+arrayvec = "0.7.4"
+bpaf = "0.9.11"


Doesn't look like bpaf is packaged in Fedora. Unless there's a strong reason to use it, I think switching to something like clap would make things easier for us.

I don't mind packaging bpaf. I have a strong dislike for clap (it sucks).

That would imply that packaging krun becomes dependent on packaging bpaf first. It's additional maintenance burden, but if you're up to it, that's fine by me.

src/bin/krun.rs

slp · 2024-05-05T10:40:45Z

Is there anything else remaining to be done apart for the fork+exec replacement for passt?

There's the hardcoded /home/{username} for HOME env var. Should we pass in the actual path from getpwnam_r to krun-guest instead (already used in krun)?

That would be better indeed.

There's also the TODO that you've noted about only overriding the mesa driver selection on Asahi Linux. Is it possible for us to set that in krun instead (through envp in krun_set_exec)?

Yes. We can probably generalize this by having an argument in krun that allows injecting environment variables into the guest without the need for them to be exported in the host, so the user can do something like:

krun -e MESA_LOADER_DRIVER_OVERRIDE=asahi /bin/bash

We can also do this later, doesn't need to be part of this PR. As you prefer.

Other than that, it's housekeeping stuff like updating the GitHub Actions workflows and removing the old code etc. (if you choose to keep the commit history).

Ack. IMO keeping the (very short) commit history after a full rewrite that switches to another language creates confusion more than anything else. I'll create the repo either later today or tomorrow.

teohhanhui · 2024-05-05T13:39:02Z

Switched to using a Cargo workspace. This will help enforce a clean separation between the krun and krun-guest binaries.

teohhanhui · 2024-05-05T13:40:04Z

overriding the mesa driver selection on Asahi Linux

I was looking into https://registry.khronos.org/EGL/extensions/MESA/EGL_MESA_query_driver.txt but that seems like too much work for now...

Can we just use eglinfo? 🤣 I guess we could just have a krun.asahi wrapper for the RPM package? (This still means we need to implement the --env option.)

EDIT: Done.

teohhanhui · 2024-05-05T16:21:06Z

krun -e MESA_LOADER_DRIVER_OVERRIDE=asahi /bin/bash

On that point... Do you need me to restore the hidden short options? (-N, -P)

Or were those only there because of how getopt_long works? (I couldn't find anything in the docs that suggests a way to disable short options, but I'm not so familiar with C, so I could be mistaken...)

teohhanhui · 2024-05-05T16:34:54Z

There's the hardcoded /home/{username} for HOME env var. Should we pass in the actual path from getpwnam_r to krun-guest instead (already used in krun)?

Never mind. It seems like we can just do the same call from krun-guest. It's cleaner and allows us more flexibility in the future...

EDIT: Done.

teohhanhui · 2024-05-05T23:51:23Z

Added tracing for better debuggability, and cleaner output for the end user.
FEXInterpreter and sommelier are now looked up in PATH.

crates/krun/src/bin/krun.rs

teohhanhui · 2024-05-06T19:24:57Z

There are no more remaining tasks other than setting up the CI workflow, which I intend to do next.

But perhaps we can get this into the new repo first, if you'd like.

slp · 2024-05-07T13:47:04Z

There are no more remaining tasks other than setting up the CI workflow, which I intend to do next.

But perhaps we can get this into the new repo first, if you'd like.

Great! Could you please rebase this PR into the new repo?

teohhanhui · 2024-05-07T14:28:16Z

Done.

teohhanhui force-pushed the riir branch 3 times, most recently from 5c2a1e3 to b883745 Compare April 27, 2024 23:08

teohhanhui force-pushed the riir branch 3 times, most recently from 77937fe to 797a679 Compare May 1, 2024 15:13

teohhanhui force-pushed the riir branch 2 times, most recently from 79d07c1 to c176314 Compare May 1, 2024 20:13

teohhanhui marked this pull request as ready for review May 1, 2024 20:14

teohhanhui changed the title ~~[WIP] Rewrite it in Rust~~ Rewrite it in Rust May 1, 2024

teohhanhui commented May 1, 2024

View reviewed changes

src/net.rs Outdated Show resolved Hide resolved

teohhanhui force-pushed the riir branch 3 times, most recently from a9fee64 to 239d90e Compare May 1, 2024 20:38

slp reviewed May 5, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

slp reviewed May 5, 2024

View reviewed changes

src/bin/krun.rs Outdated Show resolved Hide resolved

teohhanhui force-pushed the riir branch from 239d90e to 4a209bc Compare May 5, 2024 13:37

teohhanhui force-pushed the riir branch from 4a209bc to 8bf2954 Compare May 5, 2024 16:09

teohhanhui force-pushed the riir branch from 8bf2954 to 57ba9ad Compare May 5, 2024 16:30

teohhanhui force-pushed the riir branch 4 times, most recently from ec2e60f to 5c3522b Compare May 5, 2024 23:45

teohhanhui force-pushed the riir branch from 5c3522b to 4569fb1 Compare May 6, 2024 00:42

teohhanhui commented May 6, 2024

View reviewed changes

crates/krun/src/bin/krun.rs Outdated Show resolved Hide resolved

teohhanhui force-pushed the riir branch 3 times, most recently from a88b7e1 to 5fe27f3 Compare May 6, 2024 18:35

Rewrite it in Rust

283a1f7

teohhanhui force-pushed the riir branch from 5fe27f3 to 283a1f7 Compare May 6, 2024 19:21

teohhanhui mentioned this pull request May 7, 2024

Rewrite it in Rust AsahiLinux/muvm#1

Merged

4 tasks

teohhanhui closed this May 7, 2024

teohhanhui mentioned this pull request May 13, 2024

Set $MESA_LOADER_DRIVER_OVERRIDE to "asahi" to make Windows visible. AsahiLinux/muvm#4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite it in Rust #4

Rewrite it in Rust #4

teohhanhui commented Apr 26, 2024 •

edited

Loading

teohhanhui commented Apr 27, 2024 •

edited

Loading

slp commented Apr 29, 2024

teohhanhui commented Apr 29, 2024

teohhanhui commented May 1, 2024 •

edited

Loading

slp commented May 2, 2024

slp commented May 2, 2024

teohhanhui commented May 2, 2024

teohhanhui commented May 2, 2024 •

edited

Loading

slp May 5, 2024

teohhanhui May 5, 2024

slp May 5, 2024

slp commented May 5, 2024

teohhanhui commented May 5, 2024

teohhanhui commented May 5, 2024 •

edited

Loading

teohhanhui commented May 5, 2024

teohhanhui commented May 5, 2024 •

edited

Loading

teohhanhui commented May 5, 2024 •

edited

Loading

teohhanhui commented May 6, 2024

slp commented May 7, 2024

teohhanhui commented May 7, 2024

Rewrite it in Rust #4

Rewrite it in Rust #4

Conversation

teohhanhui commented Apr 26, 2024 • edited Loading

teohhanhui commented Apr 27, 2024 • edited Loading

slp commented Apr 29, 2024

teohhanhui commented Apr 29, 2024

teohhanhui commented May 1, 2024 • edited Loading

slp commented May 2, 2024

slp commented May 2, 2024

teohhanhui commented May 2, 2024

teohhanhui commented May 2, 2024 • edited Loading

slp May 5, 2024

Choose a reason for hiding this comment

teohhanhui May 5, 2024

Choose a reason for hiding this comment

slp May 5, 2024

Choose a reason for hiding this comment

slp commented May 5, 2024

teohhanhui commented May 5, 2024

teohhanhui commented May 5, 2024 • edited Loading

teohhanhui commented May 5, 2024

teohhanhui commented May 5, 2024 • edited Loading

teohhanhui commented May 5, 2024 • edited Loading

teohhanhui commented May 6, 2024

slp commented May 7, 2024

teohhanhui commented May 7, 2024

teohhanhui commented Apr 26, 2024 •

edited

Loading

teohhanhui commented Apr 27, 2024 •

edited

Loading

teohhanhui commented May 1, 2024 •

edited

Loading

teohhanhui commented May 2, 2024 •

edited

Loading

teohhanhui commented May 5, 2024 •

edited

Loading

teohhanhui commented May 5, 2024 •

edited

Loading

teohhanhui commented May 5, 2024 •

edited

Loading