Fix behavior of krun_add_virtio_console_default

When using krun_add_virtio_console_default with file descriptors that weren't
normal STDIN/STDOUT/STDERR we could get weird behavior that wasn't in line with
the behavior specified in the header.

This also makes sure to set the change the terminal mode actually associated with
the provided file descriptors instead of changing the terminal mode associated with
normal stdin/stdout/stderr.

Signed-off-by: Matej Hrica <[email protected]>

fixups

Signed-off-by: Matej Hrica <[email protected]>

Author: mtjhrc

src/devices/src/virtio/console/device.rs

@@ -5,8 +5,6 @@ use std::mem::{size_of, size_of_val};
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::sync::Arc;
 
-use libc::TIOCGWINSZ;
-use nix::ioctl_read_bad;
 use utils::eventfd::EventFd;
 use vm_memory::{ByteValued, Bytes, GuestMemoryMmap};
 

src/vmm/src/builder.rs

@@ -52,7 +52,7 @@ use crate::device_manager;
 use crate::signal_handler::register_sigint_handler;
 #[cfg(target_os = "linux")]
 use crate::signal_handler::register_sigwinch_handler;
-use crate::terminal::term_set_raw_mode;
+use crate::terminal::{term_restore_mode, term_set_raw_mode};
 #[cfg(feature = "blk")]
 use crate::vmm_config::block::BlockBuilder;
 #[cfg(not(any(feature = "tee", feature = "nitro")))]
@@ -1892,64 +1892,70 @@ fn attach_console_devices(
             Some(c) => (c.input_fd, c.output_fd, c.err_fd),
             None => (STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO),
         };
-        let stdin_is_terminal =
-            isatty(unsafe { BorrowedFd::borrow_raw(input_fd) }).unwrap_or(false);
-        let stdout_is_terminal =
-            isatty(unsafe { BorrowedFd::borrow_raw(output_fd) }).unwrap_or(false);
-        let stderr_is_terminal = isatty(unsafe { BorrowedFd::borrow_raw(err_fd) }).unwrap_or(false);
-
-        if let Err(e) = term_set_raw_mode(!stdin_is_terminal) {
-            log::error!("Failed to set terminal to raw mode: {e}")
-        }
+        let input_is_terminal =
+            input_fd >= 0 && isatty(unsafe { BorrowedFd::borrow_raw(input_fd) }).unwrap_or(false);
+        let output_is_terminal =
+            output_fd >= 0 && isatty(unsafe { BorrowedFd::borrow_raw(output_fd) }).unwrap_or(false);
+        let error_is_terminal =
+            err_fd >= 0 && isatty(unsafe { BorrowedFd::borrow_raw(err_fd) }).unwrap_or(false);
+
+        let term_fd = if input_is_terminal {
+            Some(unsafe { BorrowedFd::borrow_raw(input_fd) })
+        } else if output_is_terminal {
+            Some(unsafe { BorrowedFd::borrow_raw(output_fd) })
+        } else if error_is_terminal {
+            Some(unsafe { BorrowedFd::borrow_raw(err_fd) })
+        } else {
+            None
+        };
 
-        let console_input = if stdin_is_terminal {
-            Some(port_io::stdin().unwrap())
-        } else if input_fd < 0 {
-            Some(port_io::input_empty().unwrap())
-        } else if input_fd != STDIN_FILENO {
+        let forwarding_sigint;
+        let console_input = if input_is_terminal && input_fd >= 0 {
+            forwarding_sigint = false;
             Some(port_io::input_to_raw_fd_dup(input_fd).unwrap())
         } else {
             #[cfg(target_os = "linux")]
             {
+                forwarding_sigint = true;
                 let sigint_input = port_io::PortInputSigInt::new();
                 let sigint_input_fd = sigint_input.sigint_evt().as_raw_fd();
                 register_sigint_handler(sigint_input_fd).map_err(RegisterFsSigwinch)?;
                 Some(Box::new(sigint_input) as _)
             }
             #[cfg(not(target_os = "linux"))]
-            Some(port_io::input_empty().unwrap())
+            {
+                forwarding_sigint = false;
+                Some(port_io::input_empty().unwrap())
+            }
         };
 
-        let console_output = if stdout_is_terminal {
-            Some(port_io::stdout().unwrap())
-        } else if output_fd != STDOUT_FILENO && output_fd > 0 {
+        let console_output = if output_is_terminal && output_fd >= 0 {
             Some(port_io::output_to_raw_fd_dup(output_fd).unwrap())
         } else {
             Some(port_io::output_to_log_as_err())
         };
 
         let mut ports = vec![PortDescription::console(console_input, console_output)];
 
-        let console_err = if stderr_is_terminal {
-            Some(port_io::stderr().unwrap())
-        } else if err_fd != STDERR_FILENO && err_fd > 0 {
-            Some(port_io::output_to_raw_fd_dup(err_fd).unwrap())
-        } else {
-            None
-        };
-
-        ports.push(PortDescription::console(None, console_err));
-
-        if !stdin_is_terminal && input_fd == STDIN_FILENO {
-            ports.push(PortDescription::input_pipe("krun-stdin", port_io::stdin().unwrap()));
+        if input_fd >= 0 && !input_is_terminal {
+            ports.push(PortDescription::input_pipe(
+                "krun-stdin",
+                port_io::input_to_raw_fd_dup(input_fd).unwrap(),
+            ));
         }
 
-        if !stdout_is_terminal && output_fd == STDOUT_FILENO {
-            ports.push(PortDescription::output_pipe("krun-stdout", port_io::stdout().unwrap()));
+        if output_fd >= 0 && !output_is_terminal {
+            ports.push(PortDescription::output_pipe(
+                "krun-stdout",
+                port_io::output_to_raw_fd_dup(output_fd).unwrap(),
+            ));
         };
 
-        if !stderr_is_terminal && err_fd == STDERR_FILENO {
-            ports.push(PortDescription::output_pipe("krun-stderr", port_io::stderr().unwrap()));
+        if err_fd >= 0 && !error_is_terminal {
+            ports.push(PortDescription::output_pipe(
+                "krun-stderr",
+                port_io::output_to_raw_fd_dup(err_fd).unwrap(),
+            ));
         }
 
         ports

src/vmm/src/lib.rs

@@ -47,7 +47,6 @@ use std::time::Duration;
 #[cfg(target_arch = "x86_64")]
 use crate::device_manager::legacy::PortIODeviceManager;
 use crate::device_manager::mmio::MMIODeviceManager;
-use crate::terminal::term_set_canonical_mode;
 #[cfg(target_os = "linux")]
 use crate::vstate::VcpuEvent;
 use crate::vstate::{Vcpu, VcpuHandle, VcpuResponse, Vm};
@@ -360,10 +359,6 @@ impl Vmm {
     pub fn stop(&mut self, exit_code: i32) {
         info!("Vmm is stopping.");
 
-        if let Err(e) = term_set_canonical_mode() {
-            log::error!("Failed to restore terminal to canonical mode: {e}")
-        }
-
         for observer in &self.exit_observers {
             observer
                 .lock()

src/vmm/src/terminal.rs

@@ -1,29 +1,16 @@
-use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
-use nix::sys::termios::{tcgetattr, tcsetattr, LocalFlags, SetArg};
-use nix::unistd::isatty;
+use nix::sys::termios::{tcgetattr, tcsetattr, LocalFlags, SetArg, Termios};
 use std::os::fd::BorrowedFd;
 
-pub fn term_set_raw_mode(handle_signals_by_terminal: bool) -> Result<(), nix::Error> {
-    if let Some(fd) = get_connected_term_fd() {
-        term_fd_set_raw_mode(fd, handle_signals_by_terminal)
-    } else {
-        Ok(())
-    }
-}
-
-pub fn term_set_canonical_mode() -> Result<(), nix::Error> {
-    if let Some(fd) = get_connected_term_fd() {
-        term_fd_set_canonical_mode(fd)
-    } else {
-        Ok(())
-    }
-}
+#[must_use]
+pub struct TerminalMode(Termios);
 
-pub fn term_fd_set_raw_mode(
+// Enable raw mode for the terminal and return the old state to be restored
+pub fn term_set_raw_mode(
     term: BorrowedFd,
     handle_signals_by_terminal: bool,
-) -> Result<(), nix::Error> {
+) -> Result<TerminalMode, nix::Error> {
     let mut termios = tcgetattr(term)?;
+    let old_state = termios.clone();
 
     let mut mask = LocalFlags::ECHO | LocalFlags::ICANON;
     if !handle_signals_by_terminal {
@@ -32,32 +19,9 @@ pub fn term_fd_set_raw_mode(
 
     termios.local_flags &= !mask;
     tcsetattr(term, SetArg::TCSANOW, &termios)?;
-    Ok(())
+    Ok(TerminalMode(old_state))
 }
 
-pub fn term_fd_set_canonical_mode(term: BorrowedFd) -> Result<(), nix::Error> {
-    let mut termios = tcgetattr(term)?;
-    termios.local_flags |= LocalFlags::ECHO | LocalFlags::ICANON | LocalFlags::ISIG;
-    tcsetattr(term, SetArg::TCSANOW, &termios)?;
-    Ok(())
-}
-
-pub fn get_connected_term_fd() -> Option<BorrowedFd<'static>> {
-    let (stdin, stdout, stderr) = unsafe {
-        (
-            BorrowedFd::borrow_raw(STDIN_FILENO),
-            BorrowedFd::borrow_raw(STDOUT_FILENO),
-            BorrowedFd::borrow_raw(STDERR_FILENO),
-        )
-    };
-
-    if isatty(stdin).unwrap_or(false) {
-        Some(stdin)
-    } else if isatty(stdout).unwrap_or(false) {
-        Some(stdout)
-    } else if isatty(stderr).unwrap_or(false) {
-        Some(stderr)
-    } else {
-        None
-    }
+pub fn term_restore_mode(term: BorrowedFd, restore: &TerminalMode) -> Result<(), nix::Error> {
+    tcsetattr(term, SetArg::TCSANOW, &restore.0)
 }