From 4bfeba967b3a00b144d6f302c8b7d023b6a9c1ff Mon Sep 17 00:00:00 2001
From: Alyssa Ross <hi@alyssa.is>
Date: Thu, 29 Aug 2024 20:50:16 +0200
Subject: [PATCH] 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>
---
 vmm/src/console_devices.rs | 12 ++++++------
 vmm/src/device_manager.rs  |  8 ++++----
 vmm/src/lib.rs             | 13 +++++--------
 vmm/src/vm.rs              |  4 ++--
 4 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/vmm/src/console_devices.rs b/vmm/src/console_devices.rs
index e48dc311e..cdadf1e95 100644
--- a/vmm/src/console_devices.rs
+++ b/vmm/src/console_devices.rs
@@ -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());
             set_raw_mode(&sub_fd.as_raw_fd(), vmm.original_termios_opt.clone())?;
             vmconfig.console.file = Some(path.clone());
-            vmm.console_resize_pipe = Some(
+            vmm.console_resize_pipe = Some(Arc::new(
                 listen_for_sigwinch_on_tty(
                     sub_fd,
                     &vmm.seccomp_action,
                     vmm.hypervisor.hypervisor_type(),
                 )
                 .map_err(ConsoleDeviceError::StartSigwinchListener)?,
-            );
+            ));
         }
         ConsoleOutputMode::Tty => {
             // 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.
             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(
                         stdout.try_clone().unwrap(),
                         &vmm.seccomp_action,
                         vmm.hypervisor.hypervisor_type(),
                     )
                     .map_err(ConsoleDeviceError::StartSigwinchListener)?,
-                );
+                ));
             }
 
             // 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.
             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(
                         stdout.try_clone().unwrap(),
                         &vmm.seccomp_action,
                         vmm.hypervisor.hypervisor_type(),
                     )
                     .map_err(ConsoleDeviceError::StartSigwinchListener)?,
-                );
+                ));
             }
 
             // Make sure stdout is in raw mode, if it's a terminal.
diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs
index 99ad52088..1b097159f 100644
--- a/vmm/src/device_manager.rs
+++ b/vmm/src/device_manager.rs
@@ -1248,7 +1248,7 @@ impl DeviceManager {
     pub fn create_devices(
         &mut self,
         console_info: Option<ConsoleInfo>,
-        console_resize_pipe: Option<File>,
+        console_resize_pipe: Option<Arc<File>>,
         original_termios_opt: Arc<Mutex<Option<termios>>>,
     ) -> DeviceManagerResult<()> {
         trace_scoped!("create_devices");
@@ -2006,7 +2006,7 @@ impl DeviceManager {
         &mut self,
         virtio_devices: &mut Vec<MetaVirtioDevice>,
         console_fd: Option<RawFd>,
-        resize_pipe: Option<File>,
+        resize_pipe: Option<Arc<File>>,
     ) -> DeviceManagerResult<Option<Arc<virtio_devices::ConsoleResizer>>> {
         let console_config = self.config.lock().unwrap().console.clone();
         let endpoint = match console_config.mode {
@@ -2024,7 +2024,7 @@ impl DeviceManager {
                     // SAFETY: pty_fd is guaranteed to be a valid fd from
                     // pre_create_console_devices() in vmm/src/console_devices.rs
                     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)
                 } else {
                     return Err(DeviceManagerError::InvalidConsoleFd);
@@ -2113,7 +2113,7 @@ impl DeviceManager {
         interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>,
         virtio_devices: &mut Vec<MetaVirtioDevice>,
         console_info: Option<ConsoleInfo>,
-        console_resize_pipe: Option<File>,
+        console_resize_pipe: Option<Arc<File>>,
     ) -> DeviceManagerResult<Arc<Console>> {
         let serial_config = self.config.lock().unwrap().serial.clone();
         if console_info.is_none() {
diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs
index 973ed2056..922121655 100644
--- a/vmm/src/lib.rs
+++ b/vmm/src/lib.rs
@@ -576,7 +576,7 @@ pub struct Vmm {
     signals: Option<Handle>,
     threads: Vec<thread::JoinHandle<()>>,
     original_termios_opt: Arc<Mutex<Option<termios>>>,
-    console_resize_pipe: Option<File>,
+    console_resize_pipe: Option<Arc<File>>,
     console_info: Option<ConsoleInfo>,
 }
 
@@ -868,7 +868,7 @@ impl Vmm {
             activate_evt,
             timestamp,
             self.console_info.clone(),
-            None,
+            self.console_resize_pipe.as_ref().map(Arc::clone),
             Arc::clone(&self.original_termios_opt),
             Some(snapshot),
         )
@@ -1306,7 +1306,7 @@ impl RequestHandler for Vmm {
                         self.hypervisor.clone(),
                         activate_evt,
                         self.console_info.clone(),
-                        None,
+                        self.console_resize_pipe.as_ref().map(Arc::clone),
                         Arc::clone(&self.original_termios_opt),
                         None,
                         None,
@@ -1433,7 +1433,7 @@ impl RequestHandler for Vmm {
             self.hypervisor.clone(),
             activate_evt,
             self.console_info.clone(),
-            None,
+            self.console_resize_pipe.as_ref().map(Arc::clone),
             Arc::clone(&self.original_termios_opt),
             Some(snapshot),
             Some(source_url),
@@ -1492,10 +1492,7 @@ impl RequestHandler for Vmm {
         // First we stop the current VM
         let (config, console_resize_pipe) = if let Some(mut vm) = self.vm.take() {
             let config = vm.get_config();
-            let console_resize_pipe = vm
-                .console_resize_pipe()
-                .as_ref()
-                .map(|pipe| pipe.try_clone().unwrap());
+            let console_resize_pipe = vm.console_resize_pipe();
             vm.shutdown()?;
             (config, console_resize_pipe)
         } else {
diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs
index a9f7ce211..2013dda28 100644
--- a/vmm/src/vm.rs
+++ b/vmm/src/vm.rs
@@ -493,7 +493,7 @@ impl Vm {
         activate_evt: EventFd,
         timestamp: Instant,
         console_info: Option<ConsoleInfo>,
-        console_resize_pipe: Option<File>,
+        console_resize_pipe: Option<Arc<File>>,
         original_termios: Arc<Mutex<Option<termios>>>,
         snapshot: Option<Snapshot>,
     ) -> Result<Self> {
@@ -801,7 +801,7 @@ impl Vm {
         hypervisor: Arc<dyn hypervisor::Hypervisor>,
         activate_evt: EventFd,
         console_info: Option<ConsoleInfo>,
-        console_resize_pipe: Option<File>,
+        console_resize_pipe: Option<Arc<File>>,
         original_termios: Arc<Mutex<Option<termios>>>,
         snapshot: Option<Snapshot>,
         source_url: Option<&str>,