From a8f063db7c21ded03d5ca06f0bd8940cb1a89655 Mon Sep 17 00:00:00 2001 From: William Douglas Date: Fri, 24 Sep 2021 05:32:41 +0000 Subject: [PATCH] vmm: Refactor serial buffer to allow flush on PTY when writable Refactor the serial buffer handling in order to write the serial buffer's output to a PTY connected after the serial device stops being written to by the guest. This change moves the serial buffer initialization inside the serial manager. That is done to allow the serial buffer to be made aware of the PTY and epoll fds needed in order to modify the EpollDispatch::File trigger. These are then used by the serial buffer to trigger an epoll event when the PTY fd is writable and the buffer has content in it. They are also used to remove the trigger when the buffer is emptied in order to avoid unnecessary wake-ups. Signed-off-by: William Douglas --- vmm/src/device_manager.rs | 8 +----- vmm/src/serial_buffer.rs | 58 +++++++++++++++++++++++++++++++++++++-- vmm/src/serial_manager.rs | 49 ++++++++++++++++++++++++--------- 3 files changed, 93 insertions(+), 22 deletions(-) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index a941e3d1e..d3421a0f6 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -23,7 +23,6 @@ use crate::interrupt::LegacyUserspaceInterruptManager; use crate::memory_manager::MEMORY_MANAGER_ACPI_SIZE; use crate::memory_manager::{Error as MemoryManagerError, MemoryManager}; use crate::seccomp_filters::{get_seccomp_filter, Thread}; -use crate::serial_buffer::SerialBuffer; use crate::serial_manager::{Error as SerialManagerError, SerialManager}; use crate::sigwinch_listener::start_sigwinch_listener; use crate::GuestRegionMmap; @@ -1818,21 +1817,16 @@ impl DeviceManager { ConsoleOutputMode::Pty => { if let Some(pty) = serial_pty { self.config.lock().unwrap().serial.file = Some(pty.path.clone()); - let writer = pty.main.try_clone().unwrap(); - let buffer = SerialBuffer::new(Box::new(writer)); self.serial_pty = Some(Arc::new(Mutex::new(pty))); - Some(Box::new(buffer)) } else { let (main, mut sub, path) = create_pty(true).map_err(DeviceManagerError::SerialPtyOpen)?; self.set_raw_mode(&mut sub) .map_err(DeviceManagerError::SetPtyRaw)?; self.config.lock().unwrap().serial.file = Some(path.clone()); - let writer = main.try_clone().unwrap(); - let buffer = SerialBuffer::new(Box::new(writer)); self.serial_pty = Some(Arc::new(Mutex::new(PtyPair { main, sub, path }))); - Some(Box::new(buffer)) } + None } ConsoleOutputMode::Tty => Some(Box::new(stdout())), ConsoleOutputMode::Off | ConsoleOutputMode::Null => None, diff --git a/vmm/src/serial_buffer.rs b/vmm/src/serial_buffer.rs index c9b4b13be..94b9dc59d 100644 --- a/vmm/src/serial_buffer.rs +++ b/vmm/src/serial_buffer.rs @@ -3,7 +3,10 @@ // SPDX-License-Identifier: Apache-2.0 // +use crate::serial_manager::EpollDispatch; + use std::io::Write; +use std::os::unix::io::RawFd; // Circular buffer implementation for serial output. // Read from head; push to tail @@ -12,6 +15,9 @@ pub(crate) struct SerialBuffer { head: usize, tail: usize, out: Box, + buffering: bool, + out_fd: Option, + epoll_fd: Option, } const MAX_BUFFER_SIZE: usize = 16 << 10; @@ -23,10 +29,21 @@ impl SerialBuffer { head: 0, tail: 0, out, + buffering: false, + out_fd: None, + epoll_fd: None, } } - pub(crate) fn flush_buffer(&mut self) -> Result<(), std::io::Error> { + pub(crate) fn add_out_fd(&mut self, out_fd: RawFd) { + self.out_fd = Some(out_fd); + } + + pub(crate) fn add_epoll_fd(&mut self, epoll_fd: RawFd) { + self.epoll_fd = Some(epoll_fd); + } + + pub fn flush_buffer(&mut self) -> Result<(), std::io::Error> { if self.tail <= self.head { // The buffer to be written is in two parts let buf = &self.buffer[self.head..]; @@ -45,6 +62,7 @@ impl SerialBuffer { if !matches!(e.kind(), std::io::ErrorKind::WouldBlock) { return Err(e); } + self.add_out_poll()?; return Ok(()); } } @@ -58,6 +76,7 @@ impl SerialBuffer { self.buffer.shrink_to_fit(); self.head = 0; self.tail = 0; + self.remove_out_poll()?; } else { self.head += bytes_written; } @@ -67,11 +86,45 @@ impl SerialBuffer { if !matches!(e.kind(), std::io::ErrorKind::WouldBlock) { return Err(e); } + self.add_out_poll()?; } } Ok(()) } + + fn add_out_poll(&mut self) -> Result<(), std::io::Error> { + if self.out_fd.is_some() && self.epoll_fd.is_some() && !self.buffering { + self.buffering = true; + let out_fd = self.out_fd.as_ref().unwrap(); + let epoll_fd = self.epoll_fd.as_ref().unwrap(); + epoll::ctl( + *epoll_fd, + epoll::ControlOptions::EPOLL_CTL_MOD, + *out_fd, + epoll::Event::new( + epoll::Events::EPOLLIN | epoll::Events::EPOLLOUT, + EpollDispatch::File as u64, + ), + )?; + } + Ok(()) + } + + fn remove_out_poll(&mut self) -> Result<(), std::io::Error> { + if self.out_fd.is_some() && self.epoll_fd.is_some() && self.buffering { + self.buffering = false; + let out_fd = self.out_fd.as_ref().unwrap(); + let epoll_fd = self.epoll_fd.as_ref().unwrap(); + epoll::ctl( + *epoll_fd, + epoll::ControlOptions::EPOLL_CTL_MOD, + *out_fd, + epoll::Event::new(epoll::Events::EPOLLIN, EpollDispatch::File as u64), + )?; + } + Ok(()) + } } impl Write for SerialBuffer { fn write(&mut self, buf: &[u8]) -> Result { @@ -83,6 +136,7 @@ impl Write for SerialBuffer { if !matches!(e.kind(), std::io::ErrorKind::WouldBlock) { return Err(e); } + self.add_out_poll()?; self.buffer.push(*v); self.tail += 1; } else { @@ -116,6 +170,6 @@ impl Write for SerialBuffer { } fn flush(&mut self) -> Result<(), std::io::Error> { - Ok(()) + self.flush_buffer() } } diff --git a/vmm/src/serial_manager.rs b/vmm/src/serial_manager.rs index 444fc8831..a208c5c42 100644 --- a/vmm/src/serial_manager.rs +++ b/vmm/src/serial_manager.rs @@ -5,6 +5,7 @@ use crate::config::ConsoleOutputMode; use crate::device_manager::PtyPair; +use crate::serial_buffer::SerialBuffer; #[cfg(target_arch = "aarch64")] use devices::legacy::Pl011; #[cfg(target_arch = "x86_64")] @@ -37,6 +38,10 @@ pub enum Error { #[error("Error queuing input to the serial device: {0}")] QueueInput(#[source] vmm_sys_util::errno::Error), + /// Cannot flush output on the serial buffer. + #[error("Error flushing serial device's output buffer: {0}")] + FlushOutput(#[source] io::Error), + /// Cannot make the file descriptor non-blocking. #[error("Error making input file descriptor non-blocking: {0}")] SetNonBlocking(#[source] io::Error), @@ -142,6 +147,14 @@ impl SerialManager { ) .map_err(Error::Epoll)?; + if mode == ConsoleOutputMode::Pty { + let writer = in_file.try_clone().map_err(Error::FileClone)?; + let mut buffer = SerialBuffer::new(Box::new(writer)); + buffer.add_out_fd(in_file.as_raw_fd()); + buffer.add_epoll_fd(epoll_fd); + serial.as_ref().lock().unwrap().set_out(Box::new(buffer)); + } + // Use 'File' to enforce closing on 'epoll_fd' let epoll_file = unsafe { File::from_raw_fd(epoll_fd) }; @@ -201,21 +214,31 @@ impl SerialManager { warn!("Unknown serial manager loop event: {}", event); } EpollDispatch::File => { - let mut input = [0u8; 64]; - let count = - in_file.read(&mut input).map_err(Error::ReadInput)?; - - // Replace "\n" with "\r" to deal with Windows SAC (#1170) - if count == 1 && input[0] == 0x0a { - input[0] = 0x0d; + if event.events & libc::EPOLLOUT as u32 != 0 { + serial + .as_ref() + .lock() + .unwrap() + .flush_output() + .map_err(Error::FlushOutput)?; } + if event.events & libc::EPOLLIN as u32 != 0 { + let mut input = [0u8; 64]; + let count = + in_file.read(&mut input).map_err(Error::ReadInput)?; - serial - .as_ref() - .lock() - .unwrap() - .queue_input_bytes(&input[..count]) - .map_err(Error::QueueInput)?; + // Replace "\n" with "\r" to deal with Windows SAC (#1170) + if count == 1 && input[0] == 0x0a { + input[0] = 0x0d; + } + + serial + .as_ref() + .lock() + .unwrap() + .queue_input_bytes(&input[..count]) + .map_err(Error::QueueInput)?; + } } EpollDispatch::Kill => { info!("KILL event received, stopping epoll loop");