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

PHD: use stty to widen the effective terminal for Linux guests #818

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

gjcolombo
Copy link
Contributor

All of PHD's supported Linux guests' serial console drivers assume by default that they are writing to an 80-character-wide terminal. This breaks calls to run_shell_commands that are longer than 80 characters, because guests will insert extra spaces and carriage returns to deal with terminal wrapping. Suppress this by adding an stty command to these guests' login sequences that tells them the terminal is 10,000 characters wide. Add a framework test to validate this.

The new test also works (with no additional changes) for Windows Server
2022. It won't currently work with Server 2016 or 2019, which use the 80x24 terminal backend, so skip this test on these OSes (and add an affordance that allows a test to ask the framework for the default guest OS adapter without actually setting up any VM configuration).

With this change, the boot order tests' run_long_command function is obsolete, so remove it.

Finally, tweak phd_skip! so that it can take an arbitrary token sequence as an argument instead of only taking a string literal. This allows the use of format! in a skip macro.

Tested by running this test with all supported guest OS flavors, running the boot order tests on Alpine and Debian 11, and running a full PHD test suite against Debian 11.

Related to #773 (but doesn't totally fix it, since earlier Windows Server versions still need some work).

All of PHD's supported Linux guests' serial console drivers assume by
default that they are writing to an 80-character-wide terminal. This
breaks calls to `run_shell_commands` that are longer than 80 characters,
because guests will insert extra spaces and carriage returns to deal
with terminal wrapping. Suppress this by adding an `stty` command to
these guests' login sequences that tells them the terminal is 10,000
characters wide. Add a framework test to validate this.

The new test also works (with no additional changes) for Windows Server
2022. It won't currently work with Server 2016 or 2019, which use the
80x24 terminal backend, so skip this test on these OSes (and add an
affordance that allows a test to ask the framework for the default guest
OS adapter without actually setting up any VM configuration).

With this change, the boot order tests' `run_long_command` function is
obsolete, so remove it.

Finally, tweak `phd_skip!` so that it can take an arbitrary token
sequence as an argument instead of only taking a string literal. This
allows the use of `format!` in a skip macro.

Tested by running this test with all supported guest OS flavors.
@gjcolombo gjcolombo requested a review from iximeow November 27, 2024 21:17
Copy link
Member

@iximeow iximeow left a comment

Choose a reason for hiding this comment

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

hurray! thanks for this.

Comment on lines +38 to +40
\"Whenever you feel like sending a long serial console line,\" he told me, \
\"just remember that all the guest OSes in this world haven't had the tty \
settings you've had.\"";
Copy link
Member

Choose a reason for hiding this comment

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

amazing

@gjcolombo gjcolombo merged commit fca969a into master Nov 28, 2024
11 checks passed
@gjcolombo gjcolombo deleted the gjcolombo/phd-long-lines-linux branch November 28, 2024 00:44
@dancrossnyc
Copy link
Contributor

Just FYI, something that Ixi and I discovered at OxCon is that stty rows 0 will set the terminal to the max permissible width (not that 10,000 isn't acceptably large for any reasonable output). I'm afraid terminal types are a slightly reddish-brown herring; they were only relevant in so far as being useful for setting the initial terminal width at login).

@iximeow
Copy link
Member

iximeow commented Dec 2, 2024

i'd had it in my head that we also found stty rows 0 didn't actually change the terminal width in some case, but now you have me questioning my memory. i don't recall if that was the case (maybe Alpine?), or if it was a different stty operation that had an Alpine/other split, or if i was fully munging my memory of trying to configure Windows terminal dimensions into this.

if it was me misremembering the Windows part of this, Greg also showed me that we're already doing an equivalent for Windows Server 2022 with mode con .... there was definitely a period i thought stty was interesting but not great here because it would be an unfortunate wrinkle if we could only handle long lines on Linuxes 😁

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.

3 participants