Try to restore pty configuration on reboot

When a vm is created with a pty device, on reboot the pty fd (sub
only) will only be associated with the vmm through the epoll event
loop. The fd being polled will have been closed due to the vm itself
dropping the pty files (and potentially reopening the fd index to a
different item making things quite confusing) and new pty fds will be
opened but not polled on for input.

This change creates a structure to encapsulate the information about
the pty fd (main File, sub File and the path to the sub File). On
reboot, a copy of the console and serial pty structs is then passed
down to the new Vm  instance which will be used instead of creating a
new pty device.

This resolves the underlying issue from #2316.

Signed-off-by: William Douglas <william.r.douglas@gmail.com>
This commit is contained in:
William Douglas 2021-03-04 23:34:45 +00:00 committed by Sebastien Boeuf
parent 933d41cf2f
commit 56028fb214
3 changed files with 88 additions and 39 deletions

View File

@ -797,6 +797,23 @@ impl DeviceInfoForFDT for MMIODeviceInfo {
} }
} }
#[derive(Debug)]
pub struct PtyPair {
pub main: File,
pub sub: File,
pub path: PathBuf,
}
impl PtyPair {
fn clone(&self) -> Self {
PtyPair {
main: self.main.try_clone().unwrap(),
sub: self.sub.try_clone().unwrap(),
path: self.path.clone(),
}
}
}
pub struct DeviceManager { pub struct DeviceManager {
// Manage address space related to devices // Manage address space related to devices
address_manager: Arc<AddressManager>, address_manager: Arc<AddressManager>,
@ -805,10 +822,10 @@ pub struct DeviceManager {
console: Arc<Console>, console: Arc<Console>,
// console PTY // console PTY
console_pty: Option<Arc<Mutex<(File, File)>>>, console_pty: Option<Arc<Mutex<PtyPair>>>,
// serial PTY // serial PTY
serial_pty: Option<Arc<Mutex<(File, File)>>>, serial_pty: Option<Arc<Mutex<PtyPair>>>,
// Interrupt controller // Interrupt controller
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
@ -1010,19 +1027,23 @@ impl DeviceManager {
Ok(device_manager) Ok(device_manager)
} }
pub fn serial_pty(&self) -> Option<File> { pub fn serial_pty(&self) -> Option<PtyPair> {
self.serial_pty self.serial_pty
.as_ref() .as_ref()
.map(|pty| pty.lock().unwrap().0.try_clone().unwrap()) .map(|pty| pty.lock().unwrap().clone())
} }
pub fn console_pty(&self) -> Option<File> { pub fn console_pty(&self) -> Option<PtyPair> {
self.console_pty self.console_pty
.as_ref() .as_ref()
.map(|pty| pty.lock().unwrap().0.try_clone().unwrap()) .map(|pty| pty.lock().unwrap().clone())
} }
pub fn create_devices(&mut self) -> DeviceManagerResult<()> { pub fn create_devices(
&mut self,
serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>,
) -> DeviceManagerResult<()> {
let mut virtio_devices: Vec<(VirtioDeviceArc, bool, String)> = Vec::new(); let mut virtio_devices: Vec<(VirtioDeviceArc, bool, String)> = Vec::new();
let interrupt_controller = self.add_interrupt_controller()?; let interrupt_controller = self.add_interrupt_controller()?;
@ -1071,7 +1092,12 @@ impl DeviceManager {
)?; )?;
} }
self.console = self.add_console_device(&legacy_interrupt_manager, &mut virtio_devices)?; self.console = self.add_console_device(
&legacy_interrupt_manager,
&mut virtio_devices,
serial_pty,
console_pty,
)?;
// Reserve some IRQs for PCI devices in case they need to support INTx. // Reserve some IRQs for PCI devices in case they need to support INTx.
self.reserve_legacy_interrupts_for_pci_devices()?; self.reserve_legacy_interrupts_for_pci_devices()?;
@ -1664,6 +1690,8 @@ impl DeviceManager {
&mut self, &mut self,
interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>, interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>,
virtio_devices: &mut Vec<(VirtioDeviceArc, bool, String)>, virtio_devices: &mut Vec<(VirtioDeviceArc, bool, String)>,
serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>,
) -> DeviceManagerResult<Arc<Console>> { ) -> DeviceManagerResult<Arc<Console>> {
let serial_config = self.config.lock().unwrap().serial.clone(); let serial_config = self.config.lock().unwrap().serial.clone();
let serial_writer: Option<Box<dyn io::Write + Send>> = match serial_config.mode { let serial_writer: Option<Box<dyn io::Write + Send>> = match serial_config.mode {
@ -1672,16 +1700,21 @@ impl DeviceManager {
.map_err(DeviceManagerError::SerialOutputFileOpen)?, .map_err(DeviceManagerError::SerialOutputFileOpen)?,
)), )),
ConsoleOutputMode::Pty => { ConsoleOutputMode::Pty => {
let (main, mut sub, path) = if let Some(pty) = serial_pty {
create_pty().map_err(DeviceManagerError::SerialPtyOpen)?; self.config.lock().unwrap().serial.file = Some(pty.path.clone());
self.set_raw_mode(&mut sub) let writer = pty.main.try_clone().unwrap();
.map_err(DeviceManagerError::SetPtyRaw)?; self.serial_pty = Some(Arc::new(Mutex::new(pty)));
self.serial_pty = Some(Arc::new(Mutex::new(( Some(Box::new(writer))
main.try_clone().unwrap(), } else {
sub.try_clone().unwrap(), let (main, mut sub, path) =
)))); create_pty().map_err(DeviceManagerError::SerialPtyOpen)?;
self.config.lock().unwrap().serial.file = Some(path); self.set_raw_mode(&mut sub)
Some(Box::new(main.try_clone().unwrap())) .map_err(DeviceManagerError::SetPtyRaw)?;
self.config.lock().unwrap().serial.file = Some(path.clone());
let writer = main.try_clone().unwrap();
self.serial_pty = Some(Arc::new(Mutex::new(PtyPair { main, sub, path })));
Some(Box::new(writer))
}
} }
ConsoleOutputMode::Tty => Some(Box::new(stdout())), ConsoleOutputMode::Tty => Some(Box::new(stdout())),
ConsoleOutputMode::Off | ConsoleOutputMode::Null => None, ConsoleOutputMode::Off | ConsoleOutputMode::Null => None,
@ -1700,16 +1733,21 @@ impl DeviceManager {
.map_err(DeviceManagerError::ConsoleOutputFileOpen)?, .map_err(DeviceManagerError::ConsoleOutputFileOpen)?,
)), )),
ConsoleOutputMode::Pty => { ConsoleOutputMode::Pty => {
let (main, mut sub, path) = if let Some(pty) = console_pty {
create_pty().map_err(DeviceManagerError::SerialPtyOpen)?; self.config.lock().unwrap().console.file = Some(pty.path.clone());
self.set_raw_mode(&mut sub) let writer = pty.main.try_clone().unwrap();
.map_err(DeviceManagerError::SetPtyRaw)?; self.console_pty = Some(Arc::new(Mutex::new(pty)));
self.console_pty = Some(Arc::new(Mutex::new(( Some(Box::new(writer))
main.try_clone().unwrap(), } else {
sub.try_clone().unwrap(), let (main, mut sub, path) =
)))); create_pty().map_err(DeviceManagerError::ConsolePtyOpen)?;
self.config.lock().unwrap().console.file = Some(path); self.set_raw_mode(&mut sub)
Some(Box::new(main.try_clone().unwrap())) .map_err(DeviceManagerError::SetPtyRaw)?;
self.config.lock().unwrap().console.file = Some(path.clone());
let writer = main.try_clone().unwrap();
self.console_pty = Some(Arc::new(Mutex::new(PtyPair { main, sub, path })));
Some(Box::new(writer))
}
} }
ConsoleOutputMode::Tty => Some(Box::new(stdout())), ConsoleOutputMode::Tty => Some(Box::new(stdout())),
ConsoleOutputMode::Null => Some(Box::new(sink())), ConsoleOutputMode::Null => Some(Box::new(sink())),
@ -3802,7 +3840,7 @@ impl Snapshottable for DeviceManager {
// Now that DeviceManager is updated with the right states, it's time // Now that DeviceManager is updated with the right states, it's time
// to create the devices based on the configuration. // to create the devices based on the configuration.
self.create_devices() self.create_devices(None, None)
.map_err(|e| MigratableError::Restore(anyhow!("Could not create devices {:?}", e)))?; .map_err(|e| MigratableError::Restore(anyhow!("Could not create devices {:?}", e)))?;
// Finally, restore all devices associated with the DeviceManager. // Finally, restore all devices associated with the DeviceManager.

View File

@ -359,15 +359,17 @@ impl Vmm {
&self.seccomp_action, &self.seccomp_action,
self.hypervisor.clone(), self.hypervisor.clone(),
activate_evt, activate_evt,
None,
None,
)?; )?;
if let Some(ref serial_pty) = vm.serial_pty() { if let Some(serial_pty) = vm.serial_pty() {
self.epoll self.epoll
.add_event(serial_pty, EpollDispatch::Pty) .add_event(&serial_pty.main, EpollDispatch::Pty)
.map_err(VmError::EventfdError)?; .map_err(VmError::EventfdError)?;
}; };
if let Some(ref console_pty) = vm.console_pty() { if let Some(console_pty) = vm.console_pty() {
self.epoll self.epoll
.add_event(console_pty, EpollDispatch::Pty) .add_event(&console_pty.main, EpollDispatch::Pty)
.map_err(VmError::EventfdError)?; .map_err(VmError::EventfdError)?;
}; };
self.vm = Some(vm); self.vm = Some(vm);
@ -477,6 +479,8 @@ impl Vmm {
// First we stop the current VM and create a new one. // First we stop the current VM and create a new one.
if let Some(ref mut vm) = self.vm { if let Some(ref mut vm) = self.vm {
let config = vm.get_config(); let config = vm.get_config();
let serial_pty = vm.serial_pty();
let console_pty = vm.console_pty();
self.vm_shutdown()?; self.vm_shutdown()?;
let exit_evt = self.exit_evt.try_clone().map_err(VmError::EventFdClone)?; let exit_evt = self.exit_evt.try_clone().map_err(VmError::EventFdClone)?;
@ -499,6 +503,8 @@ impl Vmm {
&self.seccomp_action, &self.seccomp_action,
self.hypervisor.clone(), self.hypervisor.clone(),
activate_evt, activate_evt,
serial_pty,
console_pty,
)?); )?);
} }

View File

@ -29,7 +29,9 @@ use crate::config::{
VmConfig, VsockConfig, VmConfig, VsockConfig,
}; };
use crate::cpu; use crate::cpu;
use crate::device_manager::{self, get_win_size, Console, DeviceManager, DeviceManagerError}; use crate::device_manager::{
self, get_win_size, Console, DeviceManager, DeviceManagerError, PtyPair,
};
use crate::device_tree::DeviceTree; use crate::device_tree::DeviceTree;
use crate::memory_manager::{Error as MemoryManagerError, MemoryManager}; use crate::memory_manager::{Error as MemoryManagerError, MemoryManager};
use crate::migration::{get_vm_snapshot, url_to_path, VM_SNAPSHOT_FILE}; use crate::migration::{get_vm_snapshot, url_to_path, VM_SNAPSHOT_FILE};
@ -650,6 +652,7 @@ impl Vm {
Ok(numa_nodes) Ok(numa_nodes)
} }
#[allow(clippy::too_many_arguments)]
pub fn new( pub fn new(
config: Arc<Mutex<VmConfig>>, config: Arc<Mutex<VmConfig>>,
exit_evt: EventFd, exit_evt: EventFd,
@ -657,6 +660,8 @@ impl Vm {
seccomp_action: &SeccompAction, seccomp_action: &SeccompAction,
hypervisor: Arc<dyn hypervisor::Hypervisor>, hypervisor: Arc<dyn hypervisor::Hypervisor>,
activate_evt: EventFd, activate_evt: EventFd,
serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>,
) -> Result<Self> { ) -> Result<Self> {
#[cfg(all(feature = "kvm", target_arch = "x86_64"))] #[cfg(all(feature = "kvm", target_arch = "x86_64"))]
hypervisor.check_required_extensions().unwrap(); hypervisor.check_required_extensions().unwrap();
@ -702,7 +707,7 @@ impl Vm {
.device_manager .device_manager
.lock() .lock()
.unwrap() .unwrap()
.create_devices() .create_devices(serial_pty, console_pty)
.map_err(Error::DeviceManager)?; .map_err(Error::DeviceManager)?;
Ok(new_vm) Ok(new_vm)
} }
@ -1068,11 +1073,11 @@ impl Vm {
Ok(()) Ok(())
} }
pub fn serial_pty(&self) -> Option<File> { pub fn serial_pty(&self) -> Option<PtyPair> {
self.device_manager.lock().unwrap().serial_pty() self.device_manager.lock().unwrap().serial_pty()
} }
pub fn console_pty(&self) -> Option<File> { pub fn console_pty(&self) -> Option<PtyPair> {
self.device_manager.lock().unwrap().console_pty() self.device_manager.lock().unwrap().console_pty()
} }
@ -1574,7 +1579,7 @@ impl Vm {
let dm = self.device_manager.lock().unwrap(); let dm = self.device_manager.lock().unwrap();
let mut out = [0u8; 64]; let mut out = [0u8; 64];
if let Some(mut pty) = dm.serial_pty() { if let Some(mut pty) = dm.serial_pty() {
let count = pty.read(&mut out).map_err(Error::PtyConsole)?; let count = pty.main.read(&mut out).map_err(Error::PtyConsole)?;
let console = dm.console(); let console = dm.console();
if console.input_enabled() { if console.input_enabled() {
console console
@ -1583,7 +1588,7 @@ impl Vm {
} }
}; };
let count = match dm.console_pty() { let count = match dm.console_pty() {
Some(mut pty) => pty.read(&mut out).map_err(Error::PtyConsole)?, Some(mut pty) => pty.main.read(&mut out).map_err(Error::PtyConsole)?,
None => return Ok(()), None => return Ok(()),
}; };
let console = dm.console(); let console = dm.console();