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

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.

phd-tests/framework/src/guest_os/alpine.rs

@@ -15,6 +15,7 @@ impl GuestOs for Alpine {
             CommandSequenceEntry::write_str("root"),
             CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
+        .extend(super::linux::stty_enable_long_lines(self))
     }
 
     fn get_shell_prompt(&self) -> &'static str {

phd-tests/framework/src/guest_os/debian11_nocloud.rs

@@ -15,6 +15,7 @@ impl GuestOs for Debian11NoCloud {
             CommandSequenceEntry::write_str("root"),
             CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
+        .extend(super::linux::stty_enable_long_lines(self))
     }
 
     fn get_shell_prompt(&self) -> &'static str {

phd-tests/framework/src/guest_os/linux.rs

@@ -0,0 +1,18 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Helper functions for building guest OS adaptations for Linux OSes.
+
+use super::{CommandSequence, CommandSequenceEntry, GuestOs};
+
+/// Yields an `stty` command that tells the guest terminal to behave as though
+/// it is 9,999 columns wide.
+pub(super) fn stty_enable_long_lines<'a>(
+    guest_os: &impl GuestOs,
+) -> CommandSequence<'a> {
+    CommandSequence(vec![
+        CommandSequenceEntry::write_str("stty -F `tty` cols 9999"),
+        CommandSequenceEntry::wait_for(guest_os.get_shell_prompt()),
+    ])
+}

phd-tests/framework/src/guest_os/mod.rs

@@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
 
 mod alpine;
 mod debian11_nocloud;
+mod linux;
 mod shell_commands;
 mod ubuntu22_04;
 mod windows;
@@ -60,6 +61,13 @@ impl<'a> CommandSequenceEntry<'a> {
 
 pub(super) struct CommandSequence<'a>(pub Vec<CommandSequenceEntry<'a>>);
 
+impl<'a> CommandSequence<'a> {
+    fn extend(mut self, other: CommandSequence<'a>) -> CommandSequence<'a> {
+        self.0.extend(other.0);
+        self
+    }
+}
+
 pub(super) trait GuestOs: Send + Sync {
     /// Retrieves the command sequence used to wait for the OS to boot and log
     /// into it.

phd-tests/framework/src/guest_os/ubuntu22_04.rs

@@ -18,6 +18,7 @@ impl GuestOs for Ubuntu2204 {
             CommandSequenceEntry::write_str("1!Passw0rd"),
             CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
+        .extend(super::linux::stty_enable_long_lines(self))
     }
 
     fn get_shell_prompt(&self) -> &'static str {

phd-tests/framework/src/lib.rs

@@ -35,6 +35,7 @@ use camino::Utf8PathBuf;
 
 use disk::DiskFactory;
 use futures::{stream::FuturesUnordered, StreamExt};
+use guest_os::GuestOsKind;
 use port_allocator::PortAllocator;
 use server_log_mode::ServerLogMode;
 pub use test_vm::TestVm;
@@ -304,6 +305,16 @@ impl Framework {
         &self.default_guest_os_artifact
     }
 
+    /// Yields the guest OS adapter corresponding to the default guest OS
+    /// artifact.
+    pub async fn default_guest_os_kind(&self) -> anyhow::Result<GuestOsKind> {
+        Ok(self
+            .artifact_store
+            .get_guest_os_image(&self.default_guest_os_artifact)
+            .await?
+            .1)
+    }
+
     /// Indicates whether the disk factory in this framework supports the
     /// creation of Crucible disks. This can be used to skip tests that require
     /// Crucible support.

phd-tests/testcase_macro/src/lib.rs

@@ -85,7 +85,7 @@ pub fn phd_skip(args: TokenStream) -> TokenStream {
     let args = if args.is_empty() {
         None
     } else {
-        let lit = parse_macro_input!(args as syn::Lit);
+        let lit = parse_macro_input!(args as proc_macro2::TokenStream);
         Some(lit)
     };
 

phd-tests/tests/src/boot_order.rs

@@ -2,7 +2,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use anyhow::{bail, Error};
+use anyhow::bail;
 use phd_framework::{
     disk::{fat::FatFilesystem, DiskSource},
     test_vm::{DiskBackend, DiskInterface},
@@ -20,37 +20,6 @@ use efi_utils::{
     EDK2_FIRMWARE_VOL_GUID, EDK2_UI_APP_GUID,
 };
 
-pub(crate) async fn run_long_command(
-    vm: &phd_framework::TestVm,
-    cmd: &str,
-) -> Result<String, Error> {
-    // Ok, this is a bit whacky: something about the line wrapping for long
-    // commands causes `run_shell_command` to hang instead of ever actually
-    // seeing a response prompt.
-    //
-    // I haven't gone and debugged that; instead, chunk the input command up
-    // into segments short enough to not wrap when input, put them all in a
-    // file, then run the file.
-    vm.run_shell_command("rm cmd").await?;
-    let mut offset = 0;
-    // Escape any internal `\`. This isn't comprehensive escaping (doesn't
-    // handle \n, for example)..
-    let cmd = cmd.replace("\\", "\\\\");
-    while offset < cmd.len() {
-        let lim = std::cmp::min(cmd.len() - offset, 50);
-        let chunk = &cmd[offset..][..lim];
-        offset += lim;
-
-        // Catch this before it causes weird issues in half-executed commands.
-        //
-        // Could escape these here, but right now that's not really necessary.
-        assert!(!chunk.contains("\n"));
-
-        vm.run_shell_command(&format!("echo -n \'{}\' >>cmd", chunk)).await?;
-    }
-    vm.run_shell_command(". cmd").await
-}
-
 // This test checks that with a specified boot order, the guest boots whichever
 // disk we wanted to come first. This is simple enough, until you want to know
 // "what you booted from"..
@@ -305,7 +274,8 @@ async fn guest_can_adjust_boot_order(ctx: &Framework) {
 
     // Try adding a few new boot options, then add them to the boot order,
     // reboot, and make sure they're all as we set them.
-    if !run_long_command(&vm, &format!("ls {}", efipath(&bootvar(0xffff))))
+    if !vm
+        .run_shell_command(&format!("ls {}", efipath(&bootvar(0xffff))))
         .await?
         .is_empty()
     {

phd-tests/tests/src/boot_order/efi_utils.rs

@@ -20,8 +20,6 @@ use std::fmt::Write;
 use std::io::{Cursor, Read};
 use tracing::{info, trace, warn};
 
-use super::run_long_command;
-
 // First, some GUIDs. These GUIDs come from EDK2, and OVMF reuses them. Notably
 // these are the raw bytes of the GUID: textual values will have slightly
 // different ordering of bytes.
@@ -326,7 +324,7 @@ pub(crate) async fn read_efivar(
         efipath(varname)
     );
 
-    let hex = run_long_command(vm, &cmd).await?;
+    let hex = vm.run_shell_command(&cmd).await?;
 
     Ok(unhex(&hex))
 }
@@ -345,7 +343,7 @@ pub(crate) async fn write_efivar(
         efipath(varname)
     );
 
-    let attr_read_bytes = run_long_command(vm, &attr_cmd).await?;
+    let attr_read_bytes = vm.run_shell_command(&attr_cmd).await?;
     let attrs = if attr_read_bytes.ends_with(": No such file or directory") {
         // Default attributes if the variable does not exist yet. We expect it
         // to be non-volatile because we are writing it, we expect it to be
@@ -390,7 +388,7 @@ pub(crate) async fn write_efivar(
         efipath(varname)
     );
 
-    let res = run_long_command(vm, &cmd).await?;
+    let res = vm.run_shell_command(&cmd).await?;
     // If something went sideways and the write failed with something like
     // `invalid argument`...
     if !res.is_empty() {

phd-tests/tests/src/framework.rs

@@ -2,9 +2,9 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-//! This module contains tests whose primary goal is to verify the correctness
-//! of the PHD framework itself.
+//! Tests that primarily exercise the PHD framework itself.
 
+use phd_framework::guest_os::GuestOsKind;
 use phd_testcase::*;
 
 #[phd_testcase]
@@ -16,3 +16,38 @@ async fn multiline_serial_test(ctx: &Framework) {
     let out = vm.run_shell_command("echo \\\nhello \\\nworld").await?;
     assert_eq!(out, "hello world");
 }
+
+#[phd_testcase]
+async fn long_line_serial_test(ctx: &Framework) {
+    let os = ctx.default_guest_os_kind().await?;
+    if matches!(
+        os,
+        GuestOsKind::WindowsServer2016 | GuestOsKind::WindowsServer2019
+    ) {
+        phd_skip!(format!(
+            "long serial lines not supported for guest OS {os:?}"
+        ));
+    }
+
+    let mut vm = ctx.spawn_default_vm("long_line_serial_test").await?;
+    vm.launch().await?;
+    vm.wait_to_boot().await?;
+
+    let long_str = "In my younger and more vulnerable years my father gave \
+    me some advice that I've been turning over in my mind ever since. \
+    \"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.\"";
+
+    let out = vm
+        .run_shell_command(&format!(
+            "echo '{}'",
+            // Fitzgerald didn't have to deal with nested Bash quotes, but this
+            // test does. Replace apostrophes in the input string with a
+            // string-terminating `'`, followed by an escaped single quote that
+            // serves as the apostrophe, followed by a string-opening `'`.
+            long_str.replace("'", "'\\''")
+        ))
+        .await?;
+    assert_eq!(out, long_str);
+}