pid1: try to initialize terminal dimensions from data gathered via ANSI sequences + many clean-ups/refactorings#33707
Merged
poettering merged 46 commits intosystemd:mainfrom Jul 19, 2024
Conversation
9a6c102 to
a6f73ee
Compare
YHNdnzj
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
daandemeyer
reviewed
Jul 12, 2024
a6f73ee to
7bcaa70
Compare
… exec_context_determine_size() And while we are at it, merge exec_context_determine_tty_size() + exec_context_apply_tty_size(). Let's simplify things, and merge the two funcs, since the latter just does one more call. At the same time, let's make sure we actually allow passing separate input/output fds.
We turn off the flag anyway when we install them back as stdin/stdout later (via dup2()). let's hence follow our usual rules, and turn on O_CLOEXEC.
…fd() like we usually do
Let's make sure to first issue the non-destructive operations, then issue the hangup (for which we need the fd), then try to disallocate the device (for which we don't need it anymore).
It's a bit confusing, but we actually initialize the terminal twice for each service, potentially. One earlier time, where we might end up firing vhangup() and vt_disallocate(), which is a pretty brutal way to reset things, by disconnecting and possibly invalidating the tty completely. When we do this we do not keep any fd open afterwards, since it quite likely points to a dead connection of a tty. The 2nd time we initialize things when we actually want to use it. The first initialization is hence "destructive" (killing any left-overs from previous uses) the 2nd one "constructive" (preparing things for our new use), if you so will. Let's document this distinction in comments, and let's also move both initializations to exec_invoke(), so that they are easier to see in their symmetric behaviour. Moreover, let's run the tty initialization after we opened both input and output, since we need both for doing the fancy dimension auto init stuff now. Oh, and of course, one thing to mention: we nowadays initialize terminals both with ioctl() and with ansi sequences. But the latter means we need an fd that is open for *write* (since we are *writing* those ansi sequences to the tty). Hence, resetting via the input fd is conceptually wrong, it worked only so far if we had O_RDWR open mode selected)
Let's always rely on our own TTY reset logic and tty disallocation/clear screen logic, thus always pass --noclear and --noreset. Also, bring the list of baud rates to try into sync for console-getty and serial-getty (the former might or might not be connected to rs232, we can't know, hence assume the worst, and copy what serial-getty@.service does)
…garding screen clearing
It doesn't really make sense to have that in dev-setup.c, which is mostly about setting up /dev/, creating device nodes and stuff. let's move it to the other stuff that deals with /dev/console's peculiarities.
…v/console being a symlink /dev/console is sometimes a symlink in container managers. Let's handle that correctly, and resolve the symlink, and not consider the data from /sys/ in that case.
…n get_default_background_color()
I managed to hit the timeout a couple of times inside of slow qemu. Let's increase it a bit to 1/3s
…ly not initialized Various tty types come up with cols/rows not initialized (i.e. set to zero). Let's detect these cases, and return a better error than EIO, simply to make things easier to debug.
In PID 1 we write status information to /dev/console regularly, but we cannot keep it open continously, due to the kernel's SAK logic (which would kill PID 1 if user hits SAK). But closing/reopening it all the time really sucks for tty types that have no window size management (such as serial terminals/hvc0 and suchlike), because it also means the TTY is fully closed most of the time, and that resets the window sizes to 0/0. Now, we reinitialize the window size on every reopen, but that is a bit expensive for simple status output. Hence, cache the window size in the usualy $COLUMNS/$ROWS environment variables. We don't inherit these to our payloads anyway, hence these are free to us to use.
This way, we can work around the fact that "struct winsize" for /dev/console might not be initialized the moment we open the device.
72aa3be to
1604427
Compare
Member
|
Hmm, #33707 (comment)? |
Member
Author
|
lemme merge this first, now that it is passed CIs (mostly), will submit a fix with a FIXME right-after in a separate PR |
poettering
added a commit
to poettering/systemd
that referenced
this pull request
Jul 19, 2024
As requested by @YHNdnzj: systemd#33707 (comment)
poettering
added a commit
that referenced
this pull request
Jul 19, 2024
As requested by @YHNdnzj: #33707 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This started out as an attempt to improve the situation around serial TTY dimension configuration, but ended up with quite a log of additional clean-ups.
The primary goal of this is making the configuration of TTY dimensions fully automatic wherever possible, by initializing the TTY's window metrics (i.e.
struct winsize) from data gathered via ANSI sequences. This is primarily relevant to make serial terminals work nicely, as there's no handshake, sidechannel or similar where the screen dimensions could be propagated. Traditionally this meant that kernel defaults are used, i.e. 80x24 which almost always is simply wrong on modern setups. This annoys the hell out of me, when using things like "systemd-vmspawn"'s serial support, because tools such as "less" get very confused by this.At strategic places (i.e. when we start making use of a tty for a service or at boot) we'll now issue an ANSI sequence to place the cursor "very far" down and "very far" to the right, and then read back the position where the cursor actually was placed. Since cursor positions are clipped to terminal dimensions this will reveal to us how large the terminal actually is. (It would be nice if there were direct ANSI sequences to determine all this, but this works fine and we are not the first project to implement this). Once we learnt the dimensions we'll tell the local tty driver via TIOCSWINSZ.
This makes serial terminals work almost as nicely as local graphical terminals in man ways, but not entirely: if a graphical terminal changes window size it can inform clients of that via the SIGWINCH process signal. This signal is even propagated by ssh and things. However, there's no mechanism for propagating this via serial ttys. This basically means, that with this PR the serial terminal dimensions are initialized correctly at login time, but then never updated again, regardless if the user then changes the terminal window size.