vmm: fix console resizing

DeviceManager::add_virtio_console_device used to create the console
resize pipe and assign it to self.console_resize_pipe, but when this
was changed to use console_info, that was deleted without replacement.
This meant that, even though the console resize pipe was created by
pre_create_console_devices, the DeviceManager never found out about
it, so console resize didn't work (at least for pty consoles).

To fix this, the console resize pipe needs to be passed to the Vm
initializer, which is already supported, it was just previously not
used for new VMs.

Since DeviceManager already stores the console resize pipe in an Arc,
and Vmm also needs a copy of it, the sensible thing to do is change
DeviceManager::new to take Arc, and then we don't need to dup the file
descriptor, which could fail.

Fixes: 52eebaf6 ("vmm: refactor DeviceManager to use console_info")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
This commit is contained in:
Alyssa Ross 2024-08-29 20:50:16 +02:00 committed by Rob Bradford
parent 6956306604
commit 4bfeba967b
4 changed files with 17 additions and 20 deletions

View File

@ -183,14 +183,14 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
console_info.console_main_fd = Some(main_fd.into_raw_fd()); console_info.console_main_fd = Some(main_fd.into_raw_fd());
set_raw_mode(&sub_fd.as_raw_fd(), vmm.original_termios_opt.clone())?; set_raw_mode(&sub_fd.as_raw_fd(), vmm.original_termios_opt.clone())?;
vmconfig.console.file = Some(path.clone()); vmconfig.console.file = Some(path.clone());
vmm.console_resize_pipe = Some( vmm.console_resize_pipe = Some(Arc::new(
listen_for_sigwinch_on_tty( listen_for_sigwinch_on_tty(
sub_fd, sub_fd,
&vmm.seccomp_action, &vmm.seccomp_action,
vmm.hypervisor.hypervisor_type(), vmm.hypervisor.hypervisor_type(),
) )
.map_err(ConsoleDeviceError::StartSigwinchListener)?, .map_err(ConsoleDeviceError::StartSigwinchListener)?,
); ));
} }
ConsoleOutputMode::Tty => { ConsoleOutputMode::Tty => {
// Duplicating the file descriptors like this is needed as otherwise // Duplicating the file descriptors like this is needed as otherwise
@ -206,14 +206,14 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
// SAFETY: FFI call. Trivially safe. // SAFETY: FFI call. Trivially safe.
if unsafe { libc::isatty(stdout.as_raw_fd()) } == 1 { if unsafe { libc::isatty(stdout.as_raw_fd()) } == 1 {
vmm.console_resize_pipe = Some( vmm.console_resize_pipe = Some(Arc::new(
listen_for_sigwinch_on_tty( listen_for_sigwinch_on_tty(
stdout.try_clone().unwrap(), stdout.try_clone().unwrap(),
&vmm.seccomp_action, &vmm.seccomp_action,
vmm.hypervisor.hypervisor_type(), vmm.hypervisor.hypervisor_type(),
) )
.map_err(ConsoleDeviceError::StartSigwinchListener)?, .map_err(ConsoleDeviceError::StartSigwinchListener)?,
); ));
} }
// Make sure stdout is in raw mode, if it's a terminal. // Make sure stdout is in raw mode, if it's a terminal.
@ -255,14 +255,14 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
// SAFETY: FFI call. Trivially safe. // SAFETY: FFI call. Trivially safe.
if unsafe { libc::isatty(stdout.as_raw_fd()) } == 1 { if unsafe { libc::isatty(stdout.as_raw_fd()) } == 1 {
vmm.console_resize_pipe = Some( vmm.console_resize_pipe = Some(Arc::new(
listen_for_sigwinch_on_tty( listen_for_sigwinch_on_tty(
stdout.try_clone().unwrap(), stdout.try_clone().unwrap(),
&vmm.seccomp_action, &vmm.seccomp_action,
vmm.hypervisor.hypervisor_type(), vmm.hypervisor.hypervisor_type(),
) )
.map_err(ConsoleDeviceError::StartSigwinchListener)?, .map_err(ConsoleDeviceError::StartSigwinchListener)?,
); ));
} }
// Make sure stdout is in raw mode, if it's a terminal. // Make sure stdout is in raw mode, if it's a terminal.

View File

@ -1248,7 +1248,7 @@ impl DeviceManager {
pub fn create_devices( pub fn create_devices(
&mut self, &mut self,
console_info: Option<ConsoleInfo>, console_info: Option<ConsoleInfo>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<Arc<File>>,
original_termios_opt: Arc<Mutex<Option<termios>>>, original_termios_opt: Arc<Mutex<Option<termios>>>,
) -> DeviceManagerResult<()> { ) -> DeviceManagerResult<()> {
trace_scoped!("create_devices"); trace_scoped!("create_devices");
@ -2006,7 +2006,7 @@ impl DeviceManager {
&mut self, &mut self,
virtio_devices: &mut Vec<MetaVirtioDevice>, virtio_devices: &mut Vec<MetaVirtioDevice>,
console_fd: Option<RawFd>, console_fd: Option<RawFd>,
resize_pipe: Option<File>, resize_pipe: Option<Arc<File>>,
) -> DeviceManagerResult<Option<Arc<virtio_devices::ConsoleResizer>>> { ) -> DeviceManagerResult<Option<Arc<virtio_devices::ConsoleResizer>>> {
let console_config = self.config.lock().unwrap().console.clone(); let console_config = self.config.lock().unwrap().console.clone();
let endpoint = match console_config.mode { let endpoint = match console_config.mode {
@ -2024,7 +2024,7 @@ impl DeviceManager {
// SAFETY: pty_fd is guaranteed to be a valid fd from // SAFETY: pty_fd is guaranteed to be a valid fd from
// pre_create_console_devices() in vmm/src/console_devices.rs // pre_create_console_devices() in vmm/src/console_devices.rs
let file = unsafe { File::from_raw_fd(pty_fd) }; let file = unsafe { File::from_raw_fd(pty_fd) };
self.console_resize_pipe = resize_pipe.map(Arc::new); self.console_resize_pipe = resize_pipe;
Endpoint::PtyPair(file.try_clone().unwrap(), file) Endpoint::PtyPair(file.try_clone().unwrap(), file)
} else { } else {
return Err(DeviceManagerError::InvalidConsoleFd); return Err(DeviceManagerError::InvalidConsoleFd);
@ -2113,7 +2113,7 @@ impl DeviceManager {
interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>, interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>,
virtio_devices: &mut Vec<MetaVirtioDevice>, virtio_devices: &mut Vec<MetaVirtioDevice>,
console_info: Option<ConsoleInfo>, console_info: Option<ConsoleInfo>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<Arc<File>>,
) -> DeviceManagerResult<Arc<Console>> { ) -> DeviceManagerResult<Arc<Console>> {
let serial_config = self.config.lock().unwrap().serial.clone(); let serial_config = self.config.lock().unwrap().serial.clone();
if console_info.is_none() { if console_info.is_none() {

View File

@ -576,7 +576,7 @@ pub struct Vmm {
signals: Option<Handle>, signals: Option<Handle>,
threads: Vec<thread::JoinHandle<()>>, threads: Vec<thread::JoinHandle<()>>,
original_termios_opt: Arc<Mutex<Option<termios>>>, original_termios_opt: Arc<Mutex<Option<termios>>>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<Arc<File>>,
console_info: Option<ConsoleInfo>, console_info: Option<ConsoleInfo>,
} }
@ -868,7 +868,7 @@ impl Vmm {
activate_evt, activate_evt,
timestamp, timestamp,
self.console_info.clone(), self.console_info.clone(),
None, self.console_resize_pipe.as_ref().map(Arc::clone),
Arc::clone(&self.original_termios_opt), Arc::clone(&self.original_termios_opt),
Some(snapshot), Some(snapshot),
) )
@ -1306,7 +1306,7 @@ impl RequestHandler for Vmm {
self.hypervisor.clone(), self.hypervisor.clone(),
activate_evt, activate_evt,
self.console_info.clone(), self.console_info.clone(),
None, self.console_resize_pipe.as_ref().map(Arc::clone),
Arc::clone(&self.original_termios_opt), Arc::clone(&self.original_termios_opt),
None, None,
None, None,
@ -1433,7 +1433,7 @@ impl RequestHandler for Vmm {
self.hypervisor.clone(), self.hypervisor.clone(),
activate_evt, activate_evt,
self.console_info.clone(), self.console_info.clone(),
None, self.console_resize_pipe.as_ref().map(Arc::clone),
Arc::clone(&self.original_termios_opt), Arc::clone(&self.original_termios_opt),
Some(snapshot), Some(snapshot),
Some(source_url), Some(source_url),
@ -1492,10 +1492,7 @@ impl RequestHandler for Vmm {
// First we stop the current VM // First we stop the current VM
let (config, console_resize_pipe) = if let Some(mut vm) = self.vm.take() { let (config, console_resize_pipe) = if let Some(mut vm) = self.vm.take() {
let config = vm.get_config(); let config = vm.get_config();
let console_resize_pipe = vm let console_resize_pipe = vm.console_resize_pipe();
.console_resize_pipe()
.as_ref()
.map(|pipe| pipe.try_clone().unwrap());
vm.shutdown()?; vm.shutdown()?;
(config, console_resize_pipe) (config, console_resize_pipe)
} else { } else {

View File

@ -493,7 +493,7 @@ impl Vm {
activate_evt: EventFd, activate_evt: EventFd,
timestamp: Instant, timestamp: Instant,
console_info: Option<ConsoleInfo>, console_info: Option<ConsoleInfo>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<Arc<File>>,
original_termios: Arc<Mutex<Option<termios>>>, original_termios: Arc<Mutex<Option<termios>>>,
snapshot: Option<Snapshot>, snapshot: Option<Snapshot>,
) -> Result<Self> { ) -> Result<Self> {
@ -801,7 +801,7 @@ impl Vm {
hypervisor: Arc<dyn hypervisor::Hypervisor>, hypervisor: Arc<dyn hypervisor::Hypervisor>,
activate_evt: EventFd, activate_evt: EventFd,
console_info: Option<ConsoleInfo>, console_info: Option<ConsoleInfo>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<Arc<File>>,
original_termios: Arc<Mutex<Option<termios>>>, original_termios: Arc<Mutex<Option<termios>>>,
snapshot: Option<Snapshot>, snapshot: Option<Snapshot>,
source_url: Option<&str>, source_url: Option<&str>,