virtio-console, builder: Attach terminal information to each port

mtjhrc · mtjhrc · commit cf56ee9bc8bc · 2025-10-08T14:30:44.000+02:00
Previosly we were assuming stdin is terminal using it to determine the terminal
size. Attach a PortTerminalProperties trait object to each port for querying the
correct terminal size.

Note that this still only supports dynamically resizing the first port
(upon SIGWINCH).

Signed-off-by: Matej Hrica &lt;mhrica@redhat.com&gt;
diff --git a/src/devices/src/virtio/console/device.rs b/src/devices/src/virtio/console/device.rs
@@ -29,34 +29,6 @@ pub(crate) const AVAIL_FEATURES: u64 = (1 << uapi::VIRTIO_CONSOLE_F_SIZE as u64)
     | (1 << uapi::VIRTIO_CONSOLE_F_MULTIPORT as u64)
     | (1 << uapi::VIRTIO_F_VERSION_1 as u64);
 
-#[repr(C)]
-#[derive(Default)]
-struct WS {
-    rows: u16,
-    cols: u16,
-    xpixel: u16,
-    ypixel: u16,
-}
-ioctl_read_bad!(tiocgwinsz, TIOCGWINSZ, WS);
-
-pub(crate) fn get_win_size() -> (u16, u16) {
-    let mut ws: WS = WS::default();
-
-    let ret = unsafe { tiocgwinsz(0, &mut ws) };
-
-    if let Err(err) = ret {
-        match err {
-            // If the port isn't a TTY, this is expected to fail. Avoid logging
-            // an error in that case.
-            nix::errno::Errno::ENOTTY => {}
-            _ => error!("Couldn't get terminal dimensions: {err}"),
-        }
-        (0, 0)
-    } else {
-        (ws.cols, ws.rows)
-    }
-}
-
 #[derive(Copy, Clone, Debug, Default)]
 #[repr(C, packed)]
 pub struct VirtioConsoleConfig {
@@ -100,7 +72,7 @@ pub struct Console {
 impl Console {
     pub fn new(ports: Vec<PortDescription>) -> super::Result<Console> {
         assert!(!ports.is_empty(), "Expected at least 1 port");
-        assert!(ports[0].represents_console, "First port must be a console");
+        assert!(ports[0].terminal.is_some(), "First port must be a console");
 
         let num_queues = num_queues(ports.len());
         let queues = vec![VirtQueue::new(QUEUE_SIZE); num_queues];
@@ -111,12 +83,16 @@ impl Console {
                 .push(EventFd::new(utils::eventfd::EFD_NONBLOCK).map_err(ConsoleError::EventFd)?);
         }
 
-        let (cols, rows) = get_win_size();
-        let config = VirtioConsoleConfig::new(cols, rows, ports.len() as u32);
-        let ports = zip(0u32.., ports)
+        let ports: Vec<Port> = zip(0u32.., ports)
             .map(|(port_id, description)| Port::new(port_id, description))
             .collect();
 
+        let (cols, rows) = ports[0]
+            .terminal()
+            .expect("Port 0 should always be a terminal")
+            .get_win_size();
+        let config = VirtioConsoleConfig::new(cols, rows, ports.len() as u32);
+
         Ok(Console {
             control: ConsoleControl::new(),
             ports,
@@ -228,10 +204,10 @@ impl Console {
                         continue;
                     }
 
-                    if self.ports[cmd.id as usize].is_console() {
+                    if let Some(term) = self.ports[cmd.id as usize].terminal() {
                         self.control.mark_console_port(mem, cmd.id);
                         self.control.port_open(cmd.id, true);
-                        let (cols, rows) = get_win_size();
+                        let (cols, rows) = term.get_win_size();
                         self.control
                             .console_resize(cmd.id, VirtioConsoleResize { cols, rows });
                     } else {
diff --git a/src/devices/src/virtio/console/event_handler.rs b/src/devices/src/virtio/console/event_handler.rs
@@ -3,7 +3,7 @@ use std::os::unix::io::AsRawFd;
 use polly::event_manager::{EventManager, Subscriber};
 use utils::epoll::{EpollEvent, EventSet};
 
-use super::device::{get_win_size, Console};
+use super::device::Console;
 use crate::virtio::console::device::{CONTROL_RXQ_INDEX, CONTROL_TXQ_INDEX};
 use crate::virtio::console::port_queue_mapping::{queue_idx_to_port_id, QueueDirection};
 use crate::virtio::device::VirtioDevice;
@@ -88,8 +88,10 @@ impl Console {
             error!("Failed to read the sigwinch event: {e:?}");
         }
 
-        let (cols, rows) = get_win_size();
-        self.update_console_size(cols, rows);
+        if let Some(term) = self.ports[0].terminal() {
+            let (cols, rows) = term.get_win_size();
+            self.update_console_size(cols, rows);
+        }
     }
 
     fn read_control_queue_event(&mut self, event: &EpollEvent) {
diff --git a/src/devices/src/virtio/console/port.rs b/src/devices/src/virtio/console/port.rs
@@ -10,25 +10,27 @@ use crate::virtio::console::console_control::ConsoleControl;
 use crate::virtio::console::port_io::{PortInput, PortOutput};
 use crate::virtio::console::process_rx::process_rx;
 use crate::virtio::console::process_tx::process_tx;
+use crate::virtio::port_io::PortTerminalProperties;
 use crate::virtio::{InterruptTransport, Queue};
 
 pub struct PortDescription {
     pub name: Cow<'static, str>,
-    pub represents_console: bool,
     pub input: Option<Box<dyn PortInput + Send>>,
     pub output: Option<Box<dyn PortOutput + Send>>,
+    pub terminal: Option<Box<dyn PortTerminalProperties>>,
 }
 
 impl PortDescription {
     pub fn console(
         input: Option<Box<dyn PortInput + Send>>,
         output: Option<Box<dyn PortOutput + Send>>,
+        terminal: Box<dyn PortTerminalProperties>,
     ) -> Self {
         Self {
             name: "".into(),
-            represents_console: true,
             input,
             output,
+            terminal: Some(terminal),
         }
     }
 
@@ -38,9 +40,9 @@ impl PortDescription {
     ) -> Self {
         Self {
             name: name.into(),
-            represents_console: false,
             input: None,
             output: Some(output),
+            terminal: None,
         }
     }
 
@@ -50,9 +52,9 @@ impl PortDescription {
     ) -> Self {
         Self {
             name: name.into(),
-            represents_console: false,
             input: Some(input),
             output: None,
+            terminal: None,
         }
     }
 }
@@ -71,32 +73,32 @@ pub(crate) struct Port {
     port_id: u32,
     /// Empty if no name given
     name: Cow<'static, str>,
-    represents_console: bool,
     state: PortState,
     input: Option<Arc<Mutex<Box<dyn PortInput + Send>>>>,
     output: Option<Arc<Mutex<Box<dyn PortOutput + Send>>>>,
+    terminal: Option<Box<dyn PortTerminalProperties>>,
 }
 
 impl Port {
     pub(crate) fn new(port_id: u32, description: PortDescription) -> Self {
         Self {
             port_id,
             name: description.name,
-            represents_console: description.represents_console,
             state: PortState::Inactive,
             input: description.input.map(|input| Arc::new(Mutex::new(input))),
             output: description
                 .output
                 .map(|output| Arc::new(Mutex::new(output))),
+            terminal: description.terminal,
         }
     }
 
     pub fn name(&self) -> &str {
         &self.name
     }
 
-    pub fn is_console(&self) -> bool {
-        self.represents_console
+    pub fn terminal(&self) -> Option<&dyn PortTerminalProperties> {
+        self.terminal.as_deref()
     }
 
     pub fn notify_rx(&self) {
diff --git a/src/devices/src/virtio/console/port_io.rs b/src/devices/src/virtio/console/port_io.rs
@@ -1,12 +1,14 @@
-use std::fs::File;
-use std::io::{self, ErrorKind};
-use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd};
-
-use libc::{fcntl, F_GETFL, F_SETFL, O_NONBLOCK, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
+use libc::{
+    fcntl, F_GETFL, F_SETFL, O_NONBLOCK, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO, TIOCGWINSZ,
+};
 use log::Level;
 use nix::errno::Errno;
+use nix::ioctl_read_bad;
 use nix::poll::{poll, PollFd, PollFlags, PollTimeout};
-use nix::unistd::dup;
+use nix::unistd::{dup, isatty};
+use std::fs::File;
+use std::io::{self, ErrorKind};
+use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd};
 use utils::eventfd::EventFd;
 use utils::eventfd::EFD_NONBLOCK;
 use vm_memory::bitmap::Bitmap;
@@ -24,6 +26,11 @@ pub trait PortOutput {
     fn wait_until_writable(&self);
 }
 
+/// Terminal properties associated with this port
+pub trait PortTerminalProperties: Send + Sync {
+    fn get_win_size(&self) -> (u16, u16);
+}
+
 pub fn stdin() -> Result<Box<dyn PortInput + Send>, nix::Error> {
     let fd = dup_raw_fd_into_owned(STDIN_FILENO)?;
     make_non_blocking(&fd)?;
@@ -44,6 +51,21 @@ pub fn stderr() -> Result<Box<dyn PortOutput + Send>, nix::Error> {
     output_to_raw_fd_dup(STDERR_FILENO)
 }
 
+pub fn term_fd(
+    term_fd: RawFd,
+) -> Result<Box<dyn PortTerminalProperties + Send + Sync>, nix::Error> {
+    let fd = dup_raw_fd_into_owned(term_fd)?;
+    assert!(
+        isatty(&fd).is_ok_and(|v| v),
+        "Expected fd {fd:?}, to be a tty, to query the window size!"
+    );
+    Ok(Box::new(PortTerminalPropertiesFd(fd)))
+}
+
+pub fn term_fixed_size(width: u16, height: u16) -> Box<dyn PortTerminalProperties + Send + Sync> {
+    Box::new(PortTerminalPropertiesFixed((width, height)))
+}
+
 pub fn input_empty() -> Result<Box<dyn PortInput + Send>, nix::Error> {
     Ok(Box::new(PortInputEmpty {}))
 }
@@ -278,3 +300,35 @@ impl PortInput for PortInputEmpty {
         }
     }
 }
+
+struct PortTerminalPropertiesFixed((u16, u16));
+
+impl PortTerminalProperties for PortTerminalPropertiesFixed {
+    fn get_win_size(&self) -> (u16, u16) {
+        self.0
+    }
+}
+
+struct PortTerminalPropertiesFd(OwnedFd);
+
+impl PortTerminalProperties for PortTerminalPropertiesFd {
+    fn get_win_size(&self) -> (u16, u16) {
+        let mut ws: WS = WS::default();
+
+        if let Err(err) = unsafe { tiocgwinsz(self.0.as_raw_fd(), &mut ws) } {
+            error!("Couldn't get terminal dimensions: {err}");
+            return (0, 0);
+        }
+        (ws.cols, ws.rows)
+    }
+}
+
+#[repr(C)]
+#[derive(Default)]
+struct WS {
+    rows: u16,
+    cols: u16,
+    xpixel: u16,
+    ypixel: u16,
+}
+ioctl_read_bad!(tiocgwinsz, TIOCGWINSZ, WS);
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
@@ -1870,22 +1870,32 @@ fn attach_console_devices(
 ) -> std::result::Result<(), StartMicrovmError> {
     use self::StartMicrovmError::*;
 
+    let creating_implicit_console = cfg.is_none();
     let mut console_output_path: Option<PathBuf> = None;
 
     // we don't care about the console output that was set for the vm_resources
     // if the implicit console is disabled
     if let Some(path) = vm_resources.console_output.clone() {
         // only set the console output for the implicit console
-        if !vm_resources.disable_implicit_console && cfg.is_none() {
+        if !vm_resources.disable_implicit_console && creating_implicit_console {
             console_output_path = Some(path)
         }
     }
 
     let ports = if console_output_path.is_some() {
         let file = File::create(console_output_path.unwrap()).map_err(OpenConsoleFile)?;
+        // Manually emulate our Legacy behavior: In the case of output_path we have always used the
+        // stdin to determine the console size
+        let stdin_fd = unsafe { BorrowedFd::borrow_raw(STDIN_FILENO) };
+        let term_fd = if isatty(stdin_fd).is_ok_and(|v| v) {
+            port_io::term_fd(stdin_fd.as_raw_fd()).unwrap()
+        } else {
+            port_io::term_fixed_size(0, 0)
+        };
         vec![PortDescription::console(
             Some(port_io::input_empty().unwrap()),
             Some(port_io::output_file(file).unwrap()),
+            term_fd,
         )]
     } else {
         let (input_fd, output_fd, err_fd) = match cfg {
@@ -1935,7 +1945,30 @@ fn attach_console_devices(
             Some(port_io::output_to_log_as_err())
         };
 
-        let mut ports = vec![PortDescription::console(console_input, console_output)];
+        let terminal_properties = term_fd
+            .map(|fd| port_io::term_fd(fd.as_raw_fd()).unwrap())
+            .unwrap_or_else(|| port_io::term_fixed_size(0, 0));
+
+        if let Some(term_fd) = term_fd {
+            match term_set_raw_mode(term_fd, forwarding_sigint) {
+                Ok(old_mode) => {
+                    vmm.exit_observers.push(Arc::new(Mutex::new(move || {
+                        if let Err(e) = term_restore_mode(term_fd, &old_mode) {
+                            log::error!("Failed to restore terminal mode: {e}")
+                        }
+                    })));
+                }
+                Err(e) => {
+                    log::error!("Failed to set terminal to raw mode: {e}")
+                }
+            };
+        }
+
+        let mut ports = vec![PortDescription::console(
+            console_input,
+            console_output,
+            terminal_properties,
+        )];
 
         if input_fd >= 0 && !input_is_terminal {
             ports.push(PortDescription::input_pipe(
diff --git a/src/vmm/src/terminal.rs b/src/vmm/src/terminal.rs
@@ -13,7 +13,7 @@ pub fn term_set_raw_mode(
     let old_state = termios.clone();
 
     let mut mask = LocalFlags::ECHO | LocalFlags::ICANON;
-    if !handle_signals_by_terminal {
+    if handle_signals_by_terminal {
         mask |= LocalFlags::ISIG
     }
 

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ pub fn term_set_raw_mode(`
`13`	`13`	`let old_state = termios.clone();`
`14`	`14`
`15`	`15`	`let mut mask = LocalFlags::ECHO \| LocalFlags::ICANON;`
`16`		`- if !handle_signals_by_terminal {`
	`16`	`+ if handle_signals_by_terminal {`
`17`	`17`	`mask \|= LocalFlags::ISIG`
`18`	`18`	`}`
`19`	`19`