slp / krun

krun - run programs from your system in a microVM
MIT License
25 stars 7 forks source link

Error: Connection reset by peer (os error 104) #29

Closed teohhanhui closed 4 weeks ago

teohhanhui commented 4 weeks ago

I'm getting this error if I try to run krun a second time, i.e. trying to launch a command through an existing krun-server.

Do you have any clues what might be wrong? I haven't been able to get any useful output / logs.

slp commented 4 weeks ago

Please check that you're using a version of libkrun that includes this commit: https://github.com/containers/libkrun/commit/36e0785b647efd73ecf7de096b37cbab11c6d38e

You can also look for tsi_hijack in the guest with dmesg. If it's there, you're using a version without that commit and port forward won't work.

teohhanhui commented 4 weeks ago

I'm on 1.9.1 (80c15a4)

But maybe this is a problem specific to running inside a container. I'll need to investigate further.

slp commented 4 weeks ago

Looks like at some point during the code review I changed krun-server to listen on 127.0.0.1 instead of 0.0.0.0. 😅

teohhanhui commented 4 weeks ago

More weird issues abound... Now the second krun bash invocation doesn't fail or give any errors. But it doesn't work either. And after the launch attempt, the terminal of the first running krun becomes broken. Is this #25?

Command for both invocations:

RUST_LOG='krun=debug,krun_guest=debug,krun_server=debug' RUST_BACKTRACE=1 krun -e=RUST_BACKTRACE bash

But I'm unable to get any helpful output / log messages.

slp commented 4 weeks ago

We can't support interactive terminals on the second invocation. We would need something like podman does with terminal redirection and that requires a lot of code. Since the goal is to run graphical applications, I don't think we need to cover that functionality right now. But we should document it in the README.md.

As for the terminal becoming broken, we need to prevent commands from stealing krun-server stdin. I've also noticed another couple errors I'll submit once we merge #28.

teohhanhui commented 4 weeks ago

We can't support interactive terminals on the second invocation. We would need something like podman does with terminal redirection and that requires a lot of code. Since the goal is to run graphical applications, I don't think we need to cover that functionality right now. But we should document it in the README.md.

Got it. I think we should give a useful error message, if that's possible. Currently it just exits silently.

IIUC we need to call isatty: https://pubs.opengroup.org/onlinepubs/9699919799/functions/isatty.html

Oh there's https://doc.rust-lang.org/std/io/struct.Stdin.html#method.is_terminal

teohhanhui commented 4 weeks ago

I've also noticed another couple errors I'll submit once we merge #28.

I notice that we're not properly escaping the command when used as part of the file name, e.g. krun-{command}-{ts}.stdout

teohhanhui commented 4 weeks ago

We would need something like podman does with terminal redirection and that requires a lot of code.

Thinking in terms of calling krun from a shell script:

I think we do need to provide something. Could we pass the file descriptor(s) if they're not tty? And/or allow explicitly specifying stdin/stdout/stderr as options?

(Specifically, I'd hate to not be able to run steam from the terminal to see the stdout/stderr. If one of the above is provided, I could write a wrapper script for steam to make it work...)