Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

serial: Add --device virtio-serial,pty support, with ConsoleDeviceConfiguration #259

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

evidolob
Copy link

This PR based on #113
Fixes #48

As @cfergeau suggested PR set 'isConsole' for VZVirtioConsolePortConfiguration

Also I found that not deleting slave file descriptor is increase stability of tty interface, and making connection to tty possible at any time.

And for, some, reason I found that using minicom instead of screen is more reliable on tty interaction.

evidolob and others added 3 commits February 14, 2025 14:56
When using the REST API over a unix socket, this ensures the unix socket
is removed from the filesystem when vfkit exits.

Co-authored-by: Christophe Fergeau <[email protected]>
Signed-off-by: Yevhen Vydolob <[email protected]>
This new virtio-serial option allocates a pseudo-tty for the VM console.
It can then be accessed using `screen` for example.
This is a bit similar to the `--device virtio-serial,stdio` option,
except that the console is not tied to the terminal running vfkit, it's
possible to connect/disconnect from the pseudo-tty from any terminal.

This fixes crc-org#48

Signed-off-by: Christophe Fergeau <[email protected]>
This uses more helper APIs from github.com/pkg/term/termios, and makes
the code closer to Apple's recommendations in
https://developer.apple.com/documentation/virtualization/running_linux_in_a_virtual_machine?language=objc#:~:text=Configure%20the%20Serial%20Port%20Device%20for%20Standard%20In%20and%20Out
This also removes direct use of `syscall` in pkg/vf/virtio.go

Signed-off-by: Christophe Fergeau <[email protected]>
Copy link

openshift-ci bot commented Feb 14, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign baude for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@evidolob
Copy link
Author

@praveenkumar @anjannath It would be grate if you test this PR on your workflow

@evidolob
Copy link
Author

@cfergeau Tests are failing because of changing vm behaviour when I add console device. On test we launch vm with:

./out/vfkit \
    --cpus 2 --memory 2048 \
    --kernel /var/folders/p3/pnj31p35351bnyj6vj8zhq1w0000gn/T/TestVirtioSerialPTY393398295/001/bzImage \
--initrd /var/folders/p3/pnj31p35351bnyj6vj8zhq1w0000gn/T/TestVirtioSerialPTY393398295/001/initramfs.cpio.gz \
--kernel-cmdline console=hvc0 \
--device virtio-net,nat,mac=56:46:4b:49:54:01 \
--device virtio-serial,pty

But with this PR changes linux not finished networking setup until someone not connect to tty interface, so test which wait for ssh connection is exiting with timeout.

I found that removing console=hvc0 from --kernel-cmdline is fixing that behaviour, but I'm not sure that it is good to commit that change.
WDYT?

@cfergeau
Copy link
Collaborator

I found that removing console=hvc0 from --kernel-cmdline is fixing that behaviour, but I'm not sure that it is good to commit that change.
WDYT?

In my opinion this is problematic, the expected behaviour is that when you use the --pty option, the VM runs in the background regardless of its configuration, and you can connect to it using minicom/screen when you need to. I don't remember seeing this behaviour, does it appear with your changes?

Is there anything shown on the GUI window when using --gui? or maybe this prevents this "freeze" behaviour from occurring?

I found utmapp/UTM#5143 which is very similar, but no solution there :(

@baude
Copy link
Collaborator

baude commented Feb 17, 2025

when we set the console in grub, do we get early boot messages now or not until we get to later in init ?

pkg/vf/virtio.go Outdated
@@ -230,39 +231,64 @@ func unixFd(fd uintptr) int {
// https://developer.apple.com/documentation/virtualization/running_linux_in_a_virtual_machine#3880009
func setRawMode(f *os.File) error {
// Get settings for terminal
attr, _ := unix.IoctlGetTermios(unixFd(f.Fd()), unix.TIOCGETA)
var attr unix.Termios
err := termios.Tcgetattr(f.Fd(), &attr)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if and only if you repush, because attr is being used as a pointer, this can be the if err := syntax (i believe)

VirtioConsoleDevice is used to configure serial interface, also 'isConsole' for port configuration is set to 'true'.

Signed-off-by: Yevhen Vydolob <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Access guest console when it is running
3 participants