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

console: exit mainloop on SIGTERM #1920

Merged
merged 2 commits into from Nov 13, 2017

Conversation

2 participants
@brauner
Member

brauner commented Nov 12, 2017

This allows cleanly exiting a console session without control sequences.

Relates to lxc/lxd#4001 .

Signed-off-by: Christian Brauner christian.brauner@ubuntu.com

@@ -371,7 +371,7 @@ static int get_pty_on_host(struct lxc_container *c, struct wrapargs *wrap, int *
lxc_mainloop_close(&descr);
err2:
if (ts && ts->sigfd != -1)
lxc_console_sigwinch_fini(ts);
lxc_console_signal_fini(ts);

This comment has been minimized.

@hallyn

hallyn Nov 13, 2017

Member

But... didn't you rename this in the previous patch? This is non-bisectable right?

@hallyn

hallyn Nov 13, 2017

Member

But... didn't you rename this in the previous patch? This is non-bisectable right?

This comment has been minimized.

@brauner

brauner Nov 13, 2017

Member

Right, squashing.

@brauner

brauner Nov 13, 2017

Member

Right, squashing.

}
if (isatty(ts->stdinfd))

This comment has been minimized.

@hallyn

hallyn Nov 13, 2017

Member

Seems fine. I would have liked for the patch description to mention that the ts->node will now only be added to if stdinfd is a tty, because in the future when doing a git log without -p that might be helpful.

@hallyn

hallyn Nov 13, 2017

Member

Seems fine. I would have liked for the patch description to mention that the ts->node will now only be added to if stdinfd is a tty, because in the future when doing a git log without -p that might be helpful.

@hallyn

This comment has been minimized.

Show comment
Hide comment
@hallyn

hallyn Nov 13, 2017

Member

I can hit merge now, but please let me know whether I'm right about this set being broken between patches 1 and 2. If so, I'd like to see them squashed so we can maintain bisectability.

Member

hallyn commented Nov 13, 2017

I can hit merge now, but please let me know whether I'm right about this set being broken between patches 1 and 2. If so, I'd like to see them squashed so we can maintain bisectability.

@hallyn hallyn closed this Nov 13, 2017

@hallyn hallyn reopened this Nov 13, 2017

@hallyn

This comment has been minimized.

Show comment
Hide comment
@hallyn

hallyn Nov 13, 2017

Member

(oops, wrong button)

Member

hallyn commented Nov 13, 2017

(oops, wrong button)

brauner added some commits Nov 12, 2017

console: prepare for generic signal handler
Non-functional changes to enable handling more signals.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
console: exit mainloop on SIGTERM
This allows cleanly exiting a console session without control sequences.

Relates to lxc/lxd#4001 .

Note that the existence of a signal handler now doesn't guarantee that ts->node
is allocated. Instead, ts->node will now only be added to if stdinfd is a tty.
New checks need to take that into account.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
@brauner

This comment has been minimized.

Show comment
Hide comment
@brauner

brauner Nov 13, 2017

Member

@hallyn, updated.

Member

brauner commented Nov 13, 2017

@hallyn, updated.

@hallyn hallyn merged commit 05e1745 into lxc:master Nov 13, 2017

4 checks passed

Branch target Branch target is correct
Details
Signed-off-by All commits signed-off
Details
Testsuite Testsuite passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment