Rewrite it in Rust - Githubissues

teohhanhui commented 1 month ago

Replaces https://github.com/slp/krun.legacy/pull/4

As discussed in https://github.com/slp/krun.legacy/pull/1#issuecomment-2076839288

Bugs fixed:

[x] 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

[x] Stop using hardcoded /tmp/passt_1.socket
[x] 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
[x] 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 commented 1 month ago

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

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 commented 1 month ago

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: https://github.com/hawkw/matchers/pull/5

It will only benefit us when tracing-subscriber updates to matchers 0.2: https://github.com/tokio-rs/tracing/issues/2923

teohhanhui commented 1 month ago

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

graph

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

slp commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Working on cutting this down further...

teohhanhui commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

https://github.com/containers/libkrun/issues/185

slp commented 1 month ago

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

teohhanhui commented 1 month ago

Oops! I will send a follow-up PR.

slp / krun

Rewrite it in Rust #1

[x] `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.

slp / krun

Rewrite it in Rust #1

[x] 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.

[x] `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.