From 46f6d9597d69993f58b97686668af7eb3463bf7b Mon Sep 17 00:00:00 2001 From: William Douglas Date: Thu, 16 Sep 2021 19:47:36 +0000 Subject: [PATCH] vmm: Switch to using the serial_manager for serial input This change switches from handling serial input in the VMM thread to its own thread controlled by the SerialManager. The motivation for this change is to avoid the VMM thread being unable to process events while serial input is happening and vice versa. The change also makes future work flushing the serial buffer on PTY connections easier. Signed-off-by: William Douglas --- vmm/src/device_manager.rs | 61 ++++++++++++++++++++++----------------- vmm/src/lib.rs | 47 ++++-------------------------- vmm/src/vm.rs | 53 ++-------------------------------- 3 files changed, 43 insertions(+), 118 deletions(-) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 055d9e73d..e2879a2e9 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -24,6 +24,7 @@ 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; use crate::PciDeviceInfo; @@ -198,6 +199,12 @@ pub enum DeviceManagerError { /// Cannot open qcow disk path QcowDeviceCreate(qcow::Error), + /// Cannot create serial manager + CreateSerialManager(SerialManagerError), + + /// Cannot spawn the serial manager thread + SpawnSerialManager(SerialManagerError), + /// Cannot open tap interface OpenTap(net_util::TapError), @@ -509,27 +516,10 @@ pub fn create_pty(non_blocking: bool) -> io::Result<(File, File, PathBuf)> { #[derive(Default)] pub struct Console { - #[cfg(target_arch = "x86_64")] - // Serial port on 0x3f8 - serial: Option>>, - #[cfg(target_arch = "aarch64")] - serial: Option>>, console_resizer: Option>, } impl Console { - pub fn queue_input_bytes_serial(&self, out: &[u8]) -> vmm_sys_util::errno::Result<()> { - if self.serial.is_some() { - self.serial - .as_ref() - .unwrap() - .lock() - .unwrap() - .queue_input_bytes(out)?; - } - Ok(()) - } - pub fn update_console_size(&self) { if let Some(resizer) = self.console_resizer.as_ref() { resizer.update_console_size() @@ -796,6 +786,9 @@ pub struct DeviceManager { // serial PTY serial_pty: Option>>, + // Serial Manager + serial_manager: Option>, + // pty foreground status, console_resize_pipe: Option>, @@ -990,6 +983,7 @@ impl DeviceManager { #[cfg(feature = "acpi")] acpi_address, serial_pty: None, + serial_manager: None, console_pty: None, console_resize_pipe: None, virtio_mem_devices: Vec::new(), @@ -1808,19 +1802,34 @@ impl DeviceManager { ConsoleOutputMode::Tty => Some(Box::new(stdout())), ConsoleOutputMode::Off | ConsoleOutputMode::Null => None, }; - let serial = if serial_config.mode != ConsoleOutputMode::Off { - Some(self.add_serial_device(interrupt_manager, serial_writer)?) - } else { - None - }; + if serial_config.mode != ConsoleOutputMode::Off { + let serial = self.add_serial_device(interrupt_manager, serial_writer)?; + self.serial_manager = match serial_config.mode { + ConsoleOutputMode::Pty | ConsoleOutputMode::Tty => { + let serial_manager = + SerialManager::new(serial, self.serial_pty.clone(), serial_config.mode) + .map_err(DeviceManagerError::CreateSerialManager)?; + if let Some(mut serial_manager) = serial_manager { + serial_manager + .start_thread( + self.exit_evt + .try_clone() + .map_err(DeviceManagerError::EventFd)?, + ) + .map_err(DeviceManagerError::SpawnSerialManager)?; + Some(Arc::new(serial_manager)) + } else { + None + } + } + _ => None, + }; + } let console_resizer = self.add_virtio_console_device(virtio_devices, console_pty, console_resize_pipe)?; - Ok(Arc::new(Console { - serial, - console_resizer, - })) + Ok(Arc::new(Console { console_resizer })) } fn make_virtio_devices(&mut self) -> DeviceManagerResult> { diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 1939dc175..b4d1cb0ad 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -62,6 +62,7 @@ pub mod vm; #[cfg(feature = "acpi")] mod acpi; mod serial_buffer; +mod serial_manager; type GuestMemoryMmap = vm_memory::GuestMemoryMmap; type GuestRegionMmap = vm_memory::GuestRegionMmap; @@ -149,10 +150,8 @@ pub type Result = result::Result; pub enum EpollDispatch { Exit = 0, Reset = 1, - Stdin = 2, - Api = 3, - ActivateVirtioDevices = 4, - SerialPty = 5, + Api = 2, + ActivateVirtioDevices = 3, Unknown, } @@ -162,10 +161,8 @@ impl From for EpollDispatch { match v { 0 => Exit, 1 => Reset, - 2 => Stdin, - 3 => Api, - 4 => ActivateVirtioDevices, - 5 => SerialPty, + 2 => Api, + 3 => ActivateVirtioDevices, _ => Unknown, } } @@ -184,18 +181,6 @@ impl EpollContext { Ok(EpollContext { epoll_file }) } - pub fn add_stdin(&mut self) -> result::Result<(), io::Error> { - let dispatch_index = EpollDispatch::Stdin as u64; - epoll::ctl( - self.epoll_file.as_raw_fd(), - epoll::ControlOptions::EPOLL_CTL_ADD, - libc::STDIN_FILENO, - epoll::Event::new(epoll::Events::EPOLLIN, dispatch_index), - )?; - - Ok(()) - } - fn add_event(&mut self, fd: &T, token: EpollDispatch) -> result::Result<(), io::Error> where T: AsRawFd, @@ -409,18 +394,6 @@ impl Vmm { None, None, )?; - if let Some(serial_pty) = vm.serial_pty() { - self.epoll - .add_event(&serial_pty.main, EpollDispatch::SerialPty) - .map_err(VmError::EventfdError)?; - }; - if matches!( - vm_config.lock().unwrap().serial.mode, - config::ConsoleOutputMode::Tty - ) && unsafe { libc::isatty(libc::STDIN_FILENO as i32) } != 0 - { - self.epoll.add_stdin().map_err(VmError::EventfdError)?; - } self.vm = Some(vm); } @@ -1308,11 +1281,6 @@ impl Vmm { self.reset_evt.read().map_err(Error::EventFdRead)?; self.vm_reboot().map_err(Error::VmReboot)?; } - EpollDispatch::Stdin => { - if let Some(ref vm) = self.vm { - vm.handle_stdin().map_err(Error::Stdin)?; - } - } EpollDispatch::ActivateVirtioDevices => { if let Some(ref vm) = self.vm { let count = self.activate_evt.read().map_err(Error::EventFdRead)?; @@ -1324,11 +1292,6 @@ impl Vmm { .map_err(Error::ActivateVirtioDevices)?; } } - event @ EpollDispatch::SerialPty => { - if let Some(ref vm) = self.vm { - vm.handle_pty(event).map_err(Error::Pty)?; - } - } EpollDispatch::Api => { // Consume the event. self.api_evt.read().map_err(Error::EventFdRead)?; diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index d0550cd96..9539b23d8 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -14,16 +14,16 @@ #[cfg(any(target_arch = "aarch64", feature = "acpi"))] use crate::config::NumaConfig; use crate::config::{ - ConsoleOutputMode, DeviceConfig, DiskConfig, FsConfig, HotplugMethod, NetConfig, PmemConfig, - UserDeviceConfig, ValidationError, VmConfig, VsockConfig, + DeviceConfig, DiskConfig, FsConfig, HotplugMethod, NetConfig, PmemConfig, UserDeviceConfig, + ValidationError, VmConfig, VsockConfig, }; +use crate::cpu; use crate::device_manager::{self, Console, DeviceManager, DeviceManagerError, PtyPair}; use crate::device_tree::DeviceTree; use crate::memory_manager::{Error as MemoryManagerError, MemoryManager}; use crate::migration::{get_vm_snapshot, url_to_path, VM_SNAPSHOT_FILE}; use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::GuestMemoryMmap; -use crate::{cpu, EpollDispatch}; use crate::{ PciDeviceInfo, CPU_MANAGER_SNAPSHOT_ID, DEVICE_MANAGER_SNAPSHOT_ID, MEMORY_MANAGER_SNAPSHOT_ID, }; @@ -1930,53 +1930,6 @@ impl Vm { Ok(()) } - pub fn handle_pty(&self, event: EpollDispatch) -> Result<()> { - // Could be a little dangerous, picks up a lock on device_manager - // and goes into a blocking read. If the epoll loops starts to be - // services by multiple threads likely need to revist this. - let dm = self.device_manager.lock().unwrap(); - - if matches!(event, EpollDispatch::SerialPty) { - if let Some(mut pty) = dm.serial_pty() { - let mut out = [0u8; 64]; - let count = pty.main.read(&mut out).map_err(Error::PtyConsole)?; - let console = dm.console(); - console - .queue_input_bytes_serial(&out[..count]) - .map_err(Error::Console)?; - }; - } - - Ok(()) - } - - pub fn handle_stdin(&self) -> Result<()> { - let mut out = [0u8; 64]; - let count = io::stdin() - .lock() - .read_raw(&mut out) - .map_err(Error::Console)?; - - // Replace "\n" with "\r" to deal with Windows SAC (#1170) - if count == 1 && out[0] == 0x0a { - out[0] = 0x0d; - } - - if matches!( - self.config.lock().unwrap().serial.mode, - ConsoleOutputMode::Tty - ) { - self.device_manager - .lock() - .unwrap() - .console() - .queue_input_bytes_serial(&out[..count]) - .map_err(Error::Console)?; - } - - Ok(()) - } - /// Gets a thread-safe reference counted pointer to the VM configuration. pub fn get_config(&self) -> Arc> { Arc::clone(&self.config)