vmm: Make serial port controllable via command line

Add a "--serial" command line that takes as input either "off", "tty"
(default and current behaviour) and "file=/path/to/file".

When "--serial off" is used the serial device is not added to the VM
configuration at all.

Integration tests added that check for interrupts present (or not) and
that when sending to a file the file contains the expected serial
output.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>

src/main.rs

@@ -88,27 +88,28 @@ fn main() {
                 .takes_value(true)
                 .min_values(1),
         )
+        .arg(
+            Arg::with_name("serial")
+                .long("serial")
+                .help("Control serial port: off|tty|file=/path/to/a/file")
+                .default_value("tty"),
+        )
         .get_matches();
 
     // These .unwrap()s cannot fail as there is a default value defined
     let cpus = cmd_arguments.value_of("cpus").unwrap();
     let memory = cmd_arguments.value_of("memory").unwrap();
+    let rng = cmd_arguments.value_of("rng").unwrap();
+    let serial = cmd_arguments.value_of("serial").unwrap();
 
     let kernel = cmd_arguments
         .value_of("kernel")
         .expect("Missing argument: kernel");
-
     let cmdline = cmd_arguments.value_of("cmdline");
 
     let disks: Option<Vec<&str>> = cmd_arguments.values_of("disk").map(|x| x.collect());
-
     let net: Option<Vec<&str>> = cmd_arguments.values_of("net").map(|x| x.collect());
-
-    // This .unwrap() cannot fail as there is a default value defined
-    let rng = cmd_arguments.value_of("rng").unwrap();
-
     let fs: Option<Vec<&str>> = cmd_arguments.values_of("fs").map(|x| x.collect());
-
     let pmem: Option<Vec<&str>> = cmd_arguments.values_of("pmem").map(|x| x.collect());
 
     let vm_config = match config::VmConfig::parse(config::VmParams {
@@ -121,6 +122,7 @@ fn main() {
         rng,
         fs,
         pmem,
+        serial,
     }) {
         Ok(config) => config,
         Err(e) => {
@@ -760,4 +762,102 @@ mod tests {
             Ok(())
         });
     }
+
+    #[test]
+    fn test_serial_disable() {
+        test_block!(tb, "", {
+            let (disks, fw_path) = prepare_files();
+            let mut child = Command::new("target/debug/cloud-hypervisor")
+                .args(&["--cpus", "1"])
+                .args(&["--memory", "size=512M"])
+                .args(&["--kernel", fw_path.as_str()])
+                .args(&["--disk", disks[0], disks[1]])
+                .args(&[
+                    "--net",
+                    "tap=,mac=12:34:56:78:90:ab,ip=192.168.2.1,mask=255.255.255.0",
+                ])
+                .args(&["--serial", "off"])
+                .spawn()
+                .unwrap();
+
+            thread::sleep(std::time::Duration::new(10, 0));
+
+            // Test that there is no ttyS0
+            aver_eq!(
+                tb,
+                ssh_command("cat /proc/interrupts | grep 'IO-APIC' | grep -c 'ttyS0'")
+                    .trim()
+                    .parse::<u32>()
+                    .unwrap(),
+                0
+            );
+
+            // Further test that we're MSI only now
+            aver_eq!(
+                tb,
+                ssh_command("cat /proc/interrupts | grep -c 'IO-APIC'")
+                    .trim()
+                    .parse::<u32>()
+                    .unwrap(),
+                0
+            );
+
+            ssh_command("sudo reboot");
+            thread::sleep(std::time::Duration::new(10, 0));
+            let _ = child.kill();
+            let _ = child.wait();
+            Ok(())
+        });
+    }
+
+    #[test]
+    fn test_serial_file() {
+        test_block!(tb, "", {
+            let serial_path = std::path::Path::new("/tmp/serial-output");
+            let (disks, fw_path) = prepare_files();
+            let mut child = Command::new("target/debug/cloud-hypervisor")
+                .args(&["--cpus", "1"])
+                .args(&["--memory", "size=512M"])
+                .args(&["--kernel", fw_path.as_str()])
+                .args(&["--disk", disks[0], disks[1]])
+                .args(&[
+                    "--net",
+                    "tap=,mac=12:34:56:78:90:ab,ip=192.168.2.1,mask=255.255.255.0",
+                ])
+                .args(&[
+                    "--serial",
+                    format!("file={}", serial_path.to_str().unwrap()).as_str(),
+                ])
+                .spawn()
+                .unwrap();
+
+            thread::sleep(std::time::Duration::new(10, 0));
+
+            // Test that there is a ttyS0
+            aver_eq!(
+                tb,
+                ssh_command("cat /proc/interrupts | grep 'IO-APIC' | grep -c 'ttyS0'")
+                    .trim()
+                    .parse::<u32>()
+                    .unwrap(),
+                1
+            );
+
+            ssh_command("sudo reboot");
+            thread::sleep(std::time::Duration::new(10, 0));
+
+            // Do this check after shutdown of the VM as an easy way to ensure
+            // all writes are flushed to disk
+            let mut f = std::fs::File::open(serial_path).unwrap();
+            let mut buf = String::new();
+            f.read_to_string(&mut buf).unwrap();
+            aver!(tb, buf.contains("cloud login:"));
+            std::fs::remove_file(serial_path).unwrap();
+
+            let _ = child.kill();
+            let _ = child.wait();
+
+            Ok(())
+        });
+    }
 }

vmm/src/config.rs

@@ -50,6 +50,8 @@ pub enum Error<'a> {
     ParsePmemFileParam,
     /// Failed parsing size parameter.
     ParseSizeParam(std::num::ParseIntError),
+    /// Failed parsing serial parameter.
+    ParseSerialParam,
 }
 pub type Result<'a, T> = result::Result<T, Error<'a>>;
 
@@ -63,6 +65,7 @@ pub struct VmParams<'a> {
     pub rng: &'a str,
     pub fs: Option<Vec<&'a str>>,
     pub pmem: Option<Vec<&'a str>>,
+    pub serial: &'a str,
 }
 
 fn parse_size(size: &str) -> Result<u64> {
@@ -338,6 +341,41 @@ impl<'a> PmemConfig<'a> {
     }
 }
 
+#[derive(PartialEq)]
+pub enum SerialOutputMode {
+    Off,
+    Tty,
+    File,
+}
+
+pub struct SerialConfig<'a> {
+    pub file: Option<&'a Path>,
+    pub mode: SerialOutputMode,
+}
+
+impl<'a> SerialConfig<'a> {
+    pub fn parse(param: &'a str) -> Result<Self> {
+        if param == "off" {
+            Ok(Self {
+                mode: SerialOutputMode::Off,
+                file: None,
+            })
+        } else if param == "tty" {
+            Ok(Self {
+                mode: SerialOutputMode::Tty,
+                file: None,
+            })
+        } else if param.starts_with("file=") {
+            Ok(Self {
+                mode: SerialOutputMode::File,
+                file: Some(Path::new(&param[5..])),
+            })
+        } else {
+            Err(Error::ParseSerialParam)
+        }
+    }
+}
+
 pub struct VmConfig<'a> {
     pub cpus: CpusConfig,
     pub memory: MemoryConfig<'a>,
@@ -348,6 +386,7 @@ pub struct VmConfig<'a> {
     pub rng: RngConfig<'a>,
     pub fs: Option<Vec<FsConfig<'a>>>,
     pub pmem: Option<Vec<PmemConfig<'a>>>,
+    pub serial: SerialConfig<'a>,
 }
 
 impl<'a> VmConfig<'a> {
@@ -398,6 +437,7 @@ impl<'a> VmConfig<'a> {
             rng: RngConfig::parse(vm_params.rng)?,
             fs,
             pmem,
+            serial: SerialConfig::parse(vm_params.serial)?,
         })
     }
 }

vmm/src/vm.rs

@@ -21,7 +21,7 @@ extern crate vm_memory;
 extern crate vm_virtio;
 extern crate vmm_sys_util;
 
-use crate::config::VmConfig;
+use crate::config::{SerialOutputMode, VmConfig};
 use devices::ioapic;
 use kvm_bindings::{
     kvm_enable_cap, kvm_msi, kvm_pit_config, kvm_userspace_memory_region, KVM_CAP_SPLIT_IRQCHIP,
@@ -222,6 +222,9 @@ pub enum DeviceManagerError {
 
     /// Cannot find a memory range for persistent memory
     PmemRangeAllocation,
+
+    /// Error creating serial output file
+    SerialOutputFileOpen(io::Error),
 }
 pub type DeviceManagerResult<T> = result::Result<T, DeviceManagerError>;
 
@@ -463,7 +466,7 @@ struct DeviceManager {
     mmio_bus: devices::Bus,
 
     // Serial port on 0x3f8
-    serial: Arc<Mutex<devices::legacy::Serial>>,
+    serial: Option<Arc<Mutex<devices::legacy::Serial>>>,
 
     // i8042 device for exit
     i8042: Arc<Mutex<devices::legacy::I8042Device>>,
@@ -500,24 +503,35 @@ impl DeviceManager {
             ioapic: &ioapic,
         };
 
-        // Serial is tied to IRQ #4
+        let serial_writer: Option<Box<io::Write + Send>> = match vm_cfg.serial.mode {
-        let serial_irq = 4;
+            SerialOutputMode::File => Some(Box::new(
-        let interrupt: Box<devices::Interrupt> = if let Some(ioapic) = &ioapic {
+                File::create(vm_cfg.serial.file.unwrap())
-            Box::new(UserIoapicIrq::new(ioapic.clone(), serial_irq))
+                    .map_err(DeviceManagerError::SerialOutputFileOpen)?,
-        } else {
+            )),
-            let serial_evt = EventFd::new(EFD_NONBLOCK).map_err(DeviceManagerError::EventFd)?;
+            SerialOutputMode::Tty => Some(Box::new(stdout())),
-            vm_fd
+            SerialOutputMode::Off => None,
-                .register_irqfd(serial_evt.as_raw_fd(), serial_irq as u32)
-                .map_err(DeviceManagerError::Irq)?;
-
-            Box::new(KernelIoapicIrq::new(serial_evt))
         };
+        let serial = if serial_writer.is_some() {
+            // Serial is tied to IRQ #4
+            let serial_irq = 4;
+            let interrupt: Box<devices::Interrupt> = if let Some(ioapic) = &ioapic {
+                Box::new(UserIoapicIrq::new(ioapic.clone(), serial_irq))
+            } else {
+                let serial_evt = EventFd::new(EFD_NONBLOCK).map_err(DeviceManagerError::EventFd)?;
+                vm_fd
+                    .register_irqfd(serial_evt.as_raw_fd(), serial_irq as u32)
+                    .map_err(DeviceManagerError::Irq)?;
 
-        // Add serial device
+                Box::new(KernelIoapicIrq::new(serial_evt))
-        let serial = Arc::new(Mutex::new(devices::legacy::Serial::new_out(
+            };
-            interrupt,
+
-            Box::new(stdout()),
+            Some(Arc::new(Mutex::new(devices::legacy::Serial::new_out(
-        )));
+                interrupt,
+                serial_writer.unwrap(),
+            ))))
+        } else {
+            None
+        };
 
         // Add a shutdown device (i8042)
         let exit_evt = EventFd::new(EFD_NONBLOCK).map_err(DeviceManagerError::EventFd)?;
@@ -821,10 +835,12 @@ impl DeviceManager {
     }
 
     pub fn register_devices(&mut self) -> Result<()> {
-        // Insert serial device
+        if self.serial.is_some() {
-        self.io_bus
+            // Insert serial device
-            .insert(self.serial.clone(), 0x3f8, 0x8)
+            self.io_bus
-            .map_err(Error::BusError)?;
+                .insert(self.serial.as_ref().unwrap().clone(), 0x3f8, 0x8)
+                .map_err(Error::BusError)?;
+        }
 
         // Insert i8042 device
         self.io_bus
@@ -1155,7 +1171,7 @@ impl<'a> Vm<'a> {
         let mut events = vec![epoll::Event::new(epoll::Events::empty(), 0); EPOLL_EVENTS_LEN];
         let epoll_fd = self.epoll.as_raw_fd();
 
-        if self.on_tty {
+        if self.devices.serial.is_some() && self.on_tty {
             io::stdin()
                 .lock()
                 .set_raw_mode()
@@ -1178,18 +1194,22 @@ impl<'a> Vm<'a> {
                             break 'outer;
                         }
                         EpollDispatch::Stdin => {
-                            let mut out = [0u8; 64];
+                            if self.devices.serial.is_some() {
-                            let count = io::stdin()
+                                let mut out = [0u8; 64];
-                                .lock()
+                                let count = io::stdin()
-                                .read_raw(&mut out)
+                                    .lock()
-                                .map_err(Error::Serial)?;
+                                    .read_raw(&mut out)
+                                    .map_err(Error::Serial)?;
 
-                            self.devices
+                                self.devices
-                                .serial
+                                    .serial
-                                .lock()
+                                    .as_ref()
-                                .expect("Failed to process stdin event due to poisoned lock")
+                                    .unwrap()
-                                .queue_input_bytes(&out[..count])
+                                    .lock()
-                                .map_err(Error::Serial)?;
+                                    .expect("Failed to process stdin event due to poisoned lock")
+                                    .queue_input_bytes(&out[..count])
+                                    .map_err(Error::Serial)?;
+                            }
                         }
                     }
                 }