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

libcrun: handle SIGWINCH by resizing terminal_fd #1262

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

terinjokes
Copy link
Contributor

When receiving the WINCH signal, crun was using the default signal behavior of passing it through to the child process. A child that cared about the terminal size would then call the TIOCGWINSZ ioctl, but find the size had not changed. This is due to crun not copying the terminal size for its stdin to the child's pty.

This changeset handles the WINCH signal by getting crun's terminal size, and setting the child's pty to match. The WINCH signal is not passed through, as the kernel will send it as part of handling the TIOCSWINSZ ioctl.

@kolyshkin
Copy link
Collaborator

@terinjokes thanks!

Can you please fix formatting issues? make clang-format; git commit --amend -sa should be enough.

@terinjokes
Copy link
Contributor Author

Ah, sorry. I'm used to my editor formatting on save. 😅 One moment.

When receiving the WINCH signal, crun was using the default signal
behavior of passing it through to the child process. A child that cared
about the terminal size would then call the TIOCGWINSZ ioctl, but find
the size had not changed. This is due to crun not copying the terminal
size for its stdin to the child's pty.

This changeset handles the WINCH signal by getting crun's terminal size,
and setting the child's pty to match. The WINCH signal is not passed
through, as the kernel will send it as part of handling the TIOCSWINSZ
ioctl.

Signed-off-by: Terin Stock <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Aug 8, 2023

LGTM

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe giuseppe merged commit 727fd28 into containers:main Aug 8, 2023
36 checks passed
@terinjokes terinjokes deleted the sigwinch branch August 8, 2023 13:34
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.

4 participants