From cb81f8be5bd993c715399714984b824c294b91ab Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Wed, 10 Jul 2019 15:41:46 +0100 Subject: [PATCH] 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 --- src/main.rs | 114 +++++++++++++++++++++++++++++++++++++++++++--- vmm/src/config.rs | 40 ++++++++++++++++ vmm/src/vm.rs | 88 +++++++++++++++++++++-------------- 3 files changed, 201 insertions(+), 41 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5d3ef4814..8530ab6f0 100755 --- a/src/main.rs +++ b/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> = cmd_arguments.values_of("disk").map(|x| x.collect()); - let net: Option> = 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> = cmd_arguments.values_of("fs").map(|x| x.collect()); - let pmem: Option> = 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::() + .unwrap(), + 0 + ); + + // Further test that we're MSI only now + aver_eq!( + tb, + ssh_command("cat /proc/interrupts | grep -c 'IO-APIC'") + .trim() + .parse::() + .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::() + .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(()) + }); + } } diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 2ff9b8bf3..bee9f8016 100644 --- a/vmm/src/config.rs +++ b/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>; @@ -63,6 +65,7 @@ pub struct VmParams<'a> { pub rng: &'a str, pub fs: Option>, pub pmem: Option>, + pub serial: &'a str, } fn parse_size(size: &str) -> Result { @@ -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 { + 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(¶m[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>>, pub pmem: Option>>, + 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)?, }) } } diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index e919061df..09d5722b9 100755 --- a/vmm/src/vm.rs +++ b/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 = result::Result; @@ -463,7 +466,7 @@ struct DeviceManager { mmio_bus: devices::Bus, // Serial port on 0x3f8 - serial: Arc>, + serial: Option>>, // i8042 device for exit i8042: Arc>, @@ -500,24 +503,35 @@ impl DeviceManager { ioapic: &ioapic, }; - // Serial is tied to IRQ #4 - let serial_irq = 4; - let interrupt: Box = 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)?; - - Box::new(KernelIoapicIrq::new(serial_evt)) + let serial_writer: Option> = match vm_cfg.serial.mode { + SerialOutputMode::File => Some(Box::new( + File::create(vm_cfg.serial.file.unwrap()) + .map_err(DeviceManagerError::SerialOutputFileOpen)?, + )), + SerialOutputMode::Tty => Some(Box::new(stdout())), + SerialOutputMode::Off => None, }; + let serial = if serial_writer.is_some() { + // Serial is tied to IRQ #4 + let serial_irq = 4; + let interrupt: Box = 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 - let serial = Arc::new(Mutex::new(devices::legacy::Serial::new_out( - interrupt, - Box::new(stdout()), - ))); + Box::new(KernelIoapicIrq::new(serial_evt)) + }; + + 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 - self.io_bus - .insert(self.serial.clone(), 0x3f8, 0x8) - .map_err(Error::BusError)?; + if self.serial.is_some() { + // Insert serial device + self.io_bus + .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]; - let count = io::stdin() - .lock() - .read_raw(&mut out) - .map_err(Error::Serial)?; + if self.devices.serial.is_some() { + let mut out = [0u8; 64]; + let count = io::stdin() + .lock() + .read_raw(&mut out) + .map_err(Error::Serial)?; - self.devices - .serial - .lock() - .expect("Failed to process stdin event due to poisoned lock") - .queue_input_bytes(&out[..count]) - .map_err(Error::Serial)?; + self.devices + .serial + .as_ref() + .unwrap() + .lock() + .expect("Failed to process stdin event due to poisoned lock") + .queue_input_bytes(&out[..count]) + .map_err(Error::Serial)?; + } } } }