Stream main command output to screen #44
Conversation
It turns out qemu-guest-exec does not work like I (and others) expected. In particular, guest-guest-exec only returns output after the command exits. So there's no point at all to do it in the loop. This also removes the line tracking bits cuz it's useless.
|
cc @chantra , would appreciate a review if you get a chance |
Great stuff. I will check it out in the coming days. |
|
Early feedback... pulled in the change and it seems to be working just fine when running the |
| @@ -7,5 +7,26 @@ | |||
| cd { host_shared } | |||
| {{ endif }} | |||
|
|
|||
| # Discover where the output chardev is located | |||
There was a problem hiding this comment.
Thinking about pulling out vport discovery into qemu.rs. So that we can generate better error messages. run_in_vm() could be called with a sink that just appends output to a string. And we can parse that output for any discovered vport.
src/init/command.template
Outdated
| # Kernel targets share host FS so this `cd` should succeed. If it doesn't, | ||
| # the error is shown to the user as a hint. |
There was a problem hiding this comment.
This is only a should since #34 :) . It could be solveable if needed though.
There was a problem hiding this comment.
Yeah, good catch. Seems like the fix should be to disable cd ... logic if custom rootfs is provided. Since cwd on host is irrelevant w.r.t. provided rootfs
There was a problem hiding this comment.
Went ahead and added the proper fix
There was a problem hiding this comment.
IIRC, the issue was that we cd into the host path. But a simple workaround would be to cd into /mnt/vmtest right? So maybe substituting host_shared to SHARED_9P_FS_MOUNT_PATH may do the trick?
There was a problem hiding this comment.
I don't think SHARED_9P_FS_MOUNT_PATH works for relative paths, right? So a command like ls ../.. would still break.
There was a problem hiding this comment.
oh yeah, I see:
Line 82 in 5b49862
Definitely, relative path won't work. We would be in the same directory.... but under a different mount point.
This commit moves command generation logic to a templating engine rather than a simple format string. We will be adding more complex logic in the next few commits, so this will help with maintainability.
This virtio-serial device is presented to the guest as a chardev. We will redirect all the command's output to the chardev so that we can stream the output from the host.
This commit adds support to run_in_vm() to stream command output. This is nice for long running commands so the user can see what's going on. Unfortunately run_in_vm() and its callers are getting messier and messier. It could use a good refactor at some point. But I hesitate to refactor it too soon before we have at least a couple major use cases satisfied.
This teaches the command entrypoint to locate the virtio-serial port and stream all output (with stdout and stderr merged) to the chardev. This gives us streaming command output which is a much nicer UX for the user. But b/c we support virtual machine images (as well as plain kernel), we make the virtio-serial port optional. B/c we don't require folks build VM image kernels with CONFIG_VIRTIO_CONSOLE.
danobi
force-pushed
the
virtioserial_output
branch
from
9bbb32f
to
8546982
Compare
This fixes one of the major annoyances with vmtest: that long running
commands "hang" until they are completed. With this PR, output is live
streamed so that the user can be less anxious.
This PR is easier to review per-commit.
This closes #39.