Rewrite it in Rust #1

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

As discussed in slp/krun.legacy#1 (comment)

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.83
├── bpaf v0.9.12
├── env_logger v0.11.3
│   ├── anstream v0.6.14
│   │   ├── anstyle v1.0.7
│   │   ├── anstyle-parse v0.2.4
│   │   │   └── utf8parse v0.2.1
│   │   ├── anstyle-query v1.0.3
│   │   ├── colorchoice v1.0.1
│   │   ├── is_terminal_polyfill v1.70.0
│   │   └── utf8parse v0.2.1
│   ├── anstyle v1.0.7
│   ├── env_filter v0.1.0
│   │   └── log v0.4.21
│   ├── humantime v2.1.0
│   └── log v0.4.21
├── libkrun v1.8.1 (/home/teohhanhui/Projects/slp/krun/crates/libkrun)
├── log v0.4.21
├── nix v0.28.0
│   ├── bitflags v2.5.0
│   ├── cfg-if v1.0.0
│   └── libc v0.2.154
├── rustix v0.38.34
│   ├── bitflags v2.5.0
│   └── linux-raw-sys v0.4.13
└── utils v0.0.0 (/home/teohhanhui/Projects/slp/krun/crates/utils)

krun-guest v0.1.0 (/home/teohhanhui/Projects/slp/krun/crates/krun-guest)
├── anyhow v1.0.83
├── bpaf v0.9.12
├── env_logger v0.11.3
│   ├── anstream v0.6.14
│   │   ├── anstyle v1.0.7
│   │   ├── anstyle-parse v0.2.4
│   │   │   └── utf8parse v0.2.1
│   │   ├── anstyle-query v1.0.3
│   │   ├── colorchoice v1.0.1
│   │   ├── is_terminal_polyfill v1.70.0
│   │   └── utf8parse v0.2.1
│   ├── anstyle v1.0.7
│   ├── env_filter v0.1.0
│   │   └── log v0.4.21
│   ├── humantime v2.1.0
│   └── log v0.4.21
├── log v0.4.21
├── nix v0.28.0
│   ├── bitflags v2.5.0
│   ├── cfg-if v1.0.0
│   └── libc v0.2.154
├── 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
└── utils v0.0.0 (/home/teohhanhui/Projects/slp/krun/crates/utils)

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

utils v0.0.0 (/home/teohhanhui/Projects/slp/krun/crates/utils)

Requires:

teohhanhui · 2024-05-07T14:31:11Z

A note that the tracing feature bloats the resulting binaries quite a fair bit, from < 1 MiB to 3.8 MiB 😿

Let me know if you need me to change the default feature to exclude tracing. (Or turn off the attributes feature on tracing crate to avoid proc macros... Not sure this would help actually...)

teohhanhui · 2024-05-07T15:11:58Z

cargo tree --duplicates -e normal:

regex-automata v0.1.10
└── matchers v0.1.0
    └── tracing-subscriber v0.3.18
        ├── krun v0.1.0 (/home/teohhanhui/Projects/slp/krun/crates/krun)
        └── krun-guest v0.1.0 (/home/teohhanhui/Projects/slp/krun/crates/krun-guest)

regex-automata v0.4.6
└── regex v1.10.4
    └── tracing-subscriber v0.3.18 (*)

regex-syntax v0.6.29
└── regex-automata v0.1.10 (*)

regex-syntax v0.8.3
├── regex v1.10.4 (*)
└── regex-automata v0.4.6 (*)

There's an open PR to deduplicate this in matchers: hawkw/matchers#5

It will only benefit us when tracing-subscriber updates to matchers 0.2: tokio-rs/tracing#2923

teohhanhui · 2024-05-07T15:27:41Z

cargo depgraph --all-deps | dot -Tpng > graph.png:

https://sr.ht/~jplatte/cargo-depgraph/#output-explanation

slp · 2024-05-07T16:30:29Z

A note that the tracing feature bloats the resulting binaries quite a fair bit, from < 1 MiB to 3.8 MiB 😿

Let me know if you need me to change the default feature to exclude tracing. (Or turn off the attributes feature on tracing crate to avoid proc macros... Not sure this would help actually...)

Hm... yes, that's quite significant. For components that are as relatively simple as krun and krun-guest, how much value are we getting from tracing?

teohhanhui · 2024-05-07T18:50:08Z

Switching to log (with kv feature) + env_logger cuts down release binary sizes to 2.5 MiB and 2.4 MiB respectively, while preserving the same expressiveness. Also, libkrun uses log too, so there's familiarity for contributors.

teohhanhui · 2024-05-07T19:34:11Z

Working on cutting this down further...

teohhanhui · 2024-05-07T21:42:55Z

Both binaries are down to 1.1 MiB respectively. This was primarily achieved by disabling the regex feature in env_logger: https://docs.rs/env_logger/latest/env_logger/#filtering-results

And we're also being explicit with all feature flags now, i.e. default-features = false

teohhanhui · 2024-05-09T05:44:29Z

I've noticed a problem from my testing: passt processes hanging around...

teohhanhui@Han-MacBook-Air:~$ ps aux | grep passt
teohhan+   73572  0.0  0.0  72272  2560 ?        S    05:23   0:01 /usr/bin/passt -q -f --fd 4
teohhan+   73609  0.0  0.0  72288  3072 ?        S    05:23   0:01 /usr/bin/passt -q -f --fd 4
teohhan+   73763  0.0  0.0  72272  2560 ?        S    05:24   0:01 /usr/bin/passt -q -f --fd 4
teohhan+  168592  0.0  0.0  72288  1536 ?        S    06:54   0:17 /usr/bin/passt -q -f --fd 4
teohhan+  170108  0.0  0.0  72272  2048 ?        S    07:03   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  171325  0.0  0.0  72272  2048 ?        S    07:11   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  171861  0.0  0.0  72288  2048 ?        S    07:14   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  174435  0.0  0.0  72272  2560 ?        S    07:27   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  174531  0.0  0.0  72272  2560 ?        S    07:27   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  174563  0.0  0.0  72272  2560 ?        S    07:28   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  174639  0.0  0.0  72288  2048 ?        S    07:28   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  174673  0.0  0.0  72272  2560 ?        S    07:28   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  176527  0.0  0.0  72272  2560 ?        S    07:36   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  178771  0.0  0.0  72272  2560 ?        S    07:49   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  187569  0.0  0.0  72272  2560 ?        S    08:30   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  199298  0.0  0.0  72272  2048 ?        S    09:47   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  200732  0.0  0.0  72288  1536 ?        S    09:57   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  202108  0.0  0.0  72272  2048 ?        S    10:02   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  203001  0.0  0.0  72288  2048 ?        S    10:09   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  204247  0.0  0.0  72272  1536 ?        S    10:16   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  205135  0.0  0.0  72272  1536 ?        S    10:19   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  209267  0.0  0.0  72288  2048 ?        S    10:45   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210018  0.0  0.0  72272  2048 ?        S    10:49   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210061  0.0  0.0  72272  2560 ?        S    10:49   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210117  0.0  0.0  72288  2048 ?        S    10:50   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210183  0.0  0.0  72272  2560 ?        S    10:50   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210235  0.0  0.0  72288  2560 ?        S    10:51   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210358  0.0  0.0  72288  2048 ?        S    10:52   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210393  0.0  0.0  72272  2048 ?        S    10:52   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210573  0.0  0.0  72272  2560 ?        S    10:55   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  210662  0.0  0.0  72288  3072 ?        S    10:55   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  211600  0.0  0.0  72288  2560 ?        S    11:04   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  212464  0.0  0.0  72272  2560 ?        S    11:10   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  212682  0.0  0.0  72272  2048 ?        S    11:12   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  213080  0.0  0.0  72272  2048 ?        S    11:14   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  213442  0.0  0.0  72272  1536 ?        S    11:19   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  213565  0.0  0.0  72288  2048 ?        S    11:20   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  213820  0.7  0.0  72288  1536 ?        S    11:23   0:58 /usr/bin/passt -q -f --fd 4
teohhan+  223715  1.1  0.0  72272  1536 ?        S    12:13   1:00 /usr/bin/passt -q -f --fd 4
teohhan+  227106  0.0  0.0  72272  3072 ?        S    12:40   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  227653  0.0  0.0  72272  2560 ?        S    12:47   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  228052  0.0  0.0  72288  1536 ?        S    12:51   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  228434  0.0  0.0  72272  2048 ?        S    12:53   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  228535  0.0  0.0  72272  2048 ?        S    12:54   0:01 /usr/bin/passt -q -f --fd 4
teohhan+  229990  0.0  0.0  72272  2048 ?        S    13:12   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  230903  0.0  0.0  72272  3584 ?        S    13:14   0:00 /usr/bin/passt -q -f --fd 4
teohhan+  236073  0.0  0.0 222704  2048 pts/12   S+   13:41   0:00 grep --color=auto passt

teohhanhui · 2024-05-09T05:54:24Z

crates/krun/src/net.rs

+
+    // SAFETY: `child_fd` is an `OwnedFd` and consumed to prevent closing on drop.
+    // See https://doc.rust-lang.org/std/io/index.html#io-safety
+    let child = Command::new(passt_path)


~~We'd have to explicitly kill the Child: https://doc.rust-lang.org/std/process/struct.Child.html#method.kill~~ (not possible to do)

Also, it seems like krun is exiting with -1 status code, perhaps because of sommelier?

This is still a problem...

This is a limitation of libkrun (containers/libkrun#10). We need to implement a mechanism to relay signals to the main process in the VM and the exit code from it to the host. It's been open since a long time, but being a low priority issue we never got to implement it.

teohhanhui · 2024-05-09T20:26:24Z

containers/libkrun#185

crates/krun/src/net.rs

crates/krun/src/cli_options.rs

slp · 2024-05-10T15:03:42Z

Thanks a lot for the work put on this PR. Merging it!

teohhanhui · 2024-05-10T15:23:10Z

Oops! I will send a follow-up PR.

teohhanhui requested a review from slp as a code owner May 7, 2024 14:27

teohhanhui mentioned this pull request May 7, 2024

Rewrite it in Rust slp/krun.legacy#4

Closed

4 tasks

teohhanhui force-pushed the riir branch from 4c9110f to fe8d00d Compare May 7, 2024 18:48

teohhanhui force-pushed the riir branch from fe8d00d to cdaec6f Compare May 7, 2024 18:58

teohhanhui force-pushed the riir branch from cdaec6f to 5fae070 Compare May 7, 2024 21:42

teohhanhui commented May 9, 2024

View reviewed changes

teohhanhui force-pushed the riir branch from 5fae070 to bb058a6 Compare May 10, 2024 05:08

slp reviewed May 10, 2024

View reviewed changes

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

slp reviewed May 10, 2024

View reviewed changes

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

Rewrite it in Rust

0cdc3e1

teohhanhui force-pushed the riir branch from bb058a6 to 0cdc3e1 Compare May 10, 2024 14:43

teohhanhui mentioned this pull request May 10, 2024

passt socket set via krun_set_passt_fd is never closed containers/libkrun#185

Closed

slp merged commit e35c7da into slp:main May 10, 2024

teohhanhui mentioned this pull request May 10, 2024

Use std::os::unix::net::UnixStream::pair instead of rustix::net::socketpair #2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite it in Rust #1

Rewrite it in Rust #1

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

slp commented May 7, 2024

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 9, 2024 •

edited

teohhanhui May 9, 2024 •

edited

teohhanhui May 10, 2024

slp May 10, 2024

teohhanhui commented May 9, 2024

slp commented May 10, 2024

teohhanhui commented May 10, 2024

Rewrite it in Rust #1

Rewrite it in Rust #1

Conversation

teohhanhui commented May 7, 2024 • edited

teohhanhui commented May 7, 2024 • edited

teohhanhui commented May 7, 2024 • edited

teohhanhui commented May 7, 2024 • edited

slp commented May 7, 2024

teohhanhui commented May 7, 2024 • edited

teohhanhui commented May 7, 2024

teohhanhui commented May 7, 2024 • edited

teohhanhui commented May 9, 2024 • edited

teohhanhui May 9, 2024 • edited

Choose a reason for hiding this comment

teohhanhui May 10, 2024

Choose a reason for hiding this comment

slp May 10, 2024

Choose a reason for hiding this comment

teohhanhui commented May 9, 2024

slp commented May 10, 2024

teohhanhui commented May 10, 2024

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 7, 2024 •

edited

teohhanhui commented May 9, 2024 •

edited

teohhanhui May 9, 2024 •

edited