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

Report executable not found errors after tty has been setup #1548

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

saschagrunert
Copy link
Member

This allows to pass the error (as others) through the tty and not lose it.

Fixes #1524

This allows to pass the error (as others) through the tty and not lose
it.

Signed-off-by: Sascha Grunert <[email protected]>
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Copy link

podman system tests failed. @containers/packit-build please check.

@kwilczynski
Copy link
Member

@saschagrunert, this is because of the ordering? Because of this code block?

crun/src/libcrun/container.c

Lines 1269 to 1294 in c5c6679

if (has_terminal)
{
cleanup_close int terminal_fd = -1;
fflush (stderr);
terminal_fd = libcrun_set_terminal (container, err);
if (UNLIKELY (terminal_fd < 0))
return terminal_fd;
if (console_socket >= 0)
{
ret = send_fd_to_socket (console_socket, terminal_fd, err);
if (UNLIKELY (ret < 0))
return ret;
close_and_reset (&console_socket);
}
else if (entrypoint_args->has_terminal_socket_pair && console_socketpair >= 0)
{
ret = send_fd_to_socket (console_socketpair, terminal_fd, err);
if (UNLIKELY (ret < 0))
return ret;
close_and_reset (&console_socketpair);
}
}

The description is unclear as to how this issue is being fixed.

@saschagrunert
Copy link
Member Author

@saschagrunert, this is because of the ordering? Because of this code block?

Yes, in theory we could also remove the pre-check of the command availability but I thought the whole block was intentionally added so I kept it.

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

@saschagrunert
Copy link
Member Author

The codespell is fixed in #1547

Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@giuseppe giuseppe merged commit dc993bd into containers:main Sep 4, 2024
33 of 56 checks passed
@saschagrunert saschagrunert deleted the err-tty branch September 4, 2024 11:40
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.

crun does not wait for tty to receive error message for short lived process
4 participants