Allow qemu test to work under Windows without WSL (#463)

* xtask: Add ChildWrapper

This wraps the QEMU child process so that the process is killed on
drop. This avoids leaving a dangling qemu behind if an unexpected error
happens during the test.

Also add a context message to the spawn error, makes it easier to see
what's wrong if for example qemu isn't in $PATH.

* xtask: Improve echo service lifecycle

If the program ends while the echo service is still blocked in
`recv_from`, it panics on Windows:

```
thread '<unnamed>' panicked at 'failed to receive packet: Os { code: 10004, kind: Uncategorized, message: "A blocking operation was interrupted by a call to WSACancelBlockingCall." }', xtask\src\net.rs:17:14
```

Fix by using adding a timeout to the reads so that a shutdown flag can
be checked. Also wrap the service in a struct to ensure join on drop.

* xtask: Add platform module

This module contains functions for determining the host platform. These
are intended to be used instead of `#[cfg(...)]` blocks. Such blocks can
hide a compilation problem on a different platform, whereas using these
functions will allow stuff to be compiled for all platforms.

* xtask: Restrict KVM to Linux

KVM is a Linux feature, as far as I know not available on any other OSes.

* xtask: Enable QEMU under Windows without WSL

This updates the `Pipe` struct to work with the Windows version of named
pipes as well as Unix pipes. The `Io` struct has also been modified a
little bit so that JSON can be sent without a trailing newline, as that
causes QEMU to hang on Windows.

Fixes #461

Author: nicholasbishop

xtask/Cargo.toml

@@ -6,7 +6,6 @@ publish = false
 
 [dependencies]
 anyhow = "1.0.51"
-cfg-if = "1.0.0"
 clap = { version = "3.2.1", features = ["derive"] }
 # The latest fatfs release (0.3.5) is old, use git instead to pick up some fixes.
 fatfs = { git = "https://github.com/rafalh/rust-fatfs.git", rev = "87fc1ed5074a32b4e0344fcdde77359ef9e75432" }

xtask/src/disk.rs

@@ -1,5 +1,3 @@
-#![cfg(unix)]
-
 use anyhow::Result;
 use fatfs::{Date, DateTime, FileSystem, FormatVolumeOptions, FsOptions, StdIoWrapper, Time};
 use mbrman::{MBRPartitionEntry, CHS, MBR};

xtask/src/main.rs

@@ -3,12 +3,13 @@ mod cargo;
 mod disk;
 mod net;
 mod opt;
+mod pipe;
+mod platform;
 mod qemu;
 mod util;
 
 use anyhow::Result;
 use cargo::{fix_nested_cargo_env, Cargo, CargoAction, Feature, Package};
-use cfg_if::cfg_if;
 use clap::Parser;
 use opt::{Action, BuildOpt, ClippyOpt, DocOpt, MiriOpt, Opt, QemuOpt};
 use std::process::Command;
@@ -87,7 +88,11 @@ fn run_miri(opt: &MiriOpt) -> Result<()> {
 /// Build uefi-test-runner and run it in QEMU.
 fn run_vm_tests(opt: &QemuOpt) -> Result<()> {
     let mut features = vec![Feature::Qemu];
-    if opt.ci {
+
+    // Always enable the ci feature when not building on Linux so that
+    // the MP test is skipped. That test doesn't work with kvm disabled
+    // (see https://github.com/rust-osdev/uefi-rs/issues/103).
+    if opt.ci || !platform::is_linux() {
         features.push(Feature::Ci);
     }
 
@@ -103,13 +108,7 @@ fn run_vm_tests(opt: &QemuOpt) -> Result<()> {
     };
     run_cmd(cargo.command()?)?;
 
-    cfg_if! {
-        if #[cfg(unix)] {
-            qemu::run_qemu(*opt.target, opt)
-        } else {
-            panic!("vm tests are only supported on unix targets");
-        }
-    }
+    qemu::run_qemu(*opt.target, opt)
 }
 
 /// Run unit tests and doctests on the host. Most of uefi-rs is tested

xtask/src/net.rs

@@ -1,22 +1,68 @@
-#![cfg(unix)]
-
 use std::net::UdpSocket;
+use std::sync::{Arc, Mutex};
+use std::thread::{self, JoinHandle};
+use std::time::Duration;
+
+/// Run a simple echo service that listens on UDP port 21572 and
+/// reverses the incoming messages.
+pub struct EchoService {
+    stop_requested: Arc<Mutex<bool>>,
+
+    // `JoinHandle::join` consumes the handle, so put it in an option so
+    // that we can call `join` in `drop`.
+    join_handle: Option<JoinHandle<()>>,
+}
+
+impl Drop for EchoService {
+    fn drop(&mut self) {
+        self.stop();
+        self.join_handle
+            .take()
+            .unwrap()
+            .join()
+            .expect("failed to join echo service thread");
+    }
+}
+
+impl EchoService {
+    /// Start the server.
+    pub fn start() -> Self {
+        let stop_requested = Arc::new(Mutex::new(false));
+        let stop_requested_copy = stop_requested.clone();
+        let join_handle = thread::spawn(|| reverse_echo_service(stop_requested_copy));
+        Self {
+            stop_requested,
+            join_handle: Some(join_handle),
+        }
+    }
 
-/// Start a thread that listens on UDP port 21572 and simulates a simple echo
-/// service that reverses the incoming messages.
-pub fn start_reverse_echo_service() {
-    std::thread::spawn(reverse_echo_service);
+    /// Request that the server stop.
+    pub fn stop(&self) {
+        let mut guard = self.stop_requested.lock().unwrap();
+        *guard = true;
+    }
 }
 
-fn reverse_echo_service() {
+fn reverse_echo_service(stop_requested: Arc<Mutex<bool>>) {
     let socket = UdpSocket::bind(("127.0.0.1", 21572)).expect("failed to bind to UDP socket");
 
+    // Set a timeout so that the service can periodically check if a
+    // stop has been requested.
+    socket
+        .set_read_timeout(Some(Duration::from_millis(100)))
+        .expect("failed to set read timeout");
+
     let mut buffer = [0; 257];
     loop {
+        if *stop_requested.lock().unwrap() {
+            break;
+        }
+
         // Receive a packet.
-        let (len, addr) = socket
-            .recv_from(&mut buffer)
-            .expect("failed to receive packet");
+        let (len, addr) = match socket.recv_from(&mut buffer) {
+            Ok((len, addr)) => (len, addr),
+            Err(_) => continue,
+        };
         let buffer = &mut buffer[..len];
 
         // Extract header information.

xtask/src/pipe.rs

@@ -0,0 +1,105 @@
+use crate::platform;
+use crate::qemu::Io;
+use anyhow::Result;
+use fs_err::{File, OpenOptions};
+use std::path::{Path, PathBuf};
+use std::thread;
+use std::time::Duration;
+
+pub struct Pipe {
+    qemu_arg: String,
+    input_path: PathBuf,
+    output_path: PathBuf,
+}
+
+impl Pipe {
+    /// Prepare to set up a two-way communication pipe. This is called
+    /// before launching QEMU. On Unix this uses `mkfifo` to create two
+    /// pipes; on Windows QEMU itself will create the duplex pipe.
+    pub fn new(dir: &Path, base_name: &'static str) -> Result<Self> {
+        if platform::is_unix() {
+            let qemu_arg = format!("pipe:{}", dir.join(base_name).to_str().unwrap());
+            let input_path = dir.join(format!("{}.in", base_name));
+            let output_path = dir.join(format!("{}.out", base_name));
+
+            // This part has to be conditionally compiled because the
+            // `nix` interfaces don't exist when compiling under
+            // Windows.
+            #[cfg(unix)]
+            {
+                let mode = nix::sys::stat::Mode::from_bits(0o666).unwrap();
+                nix::unistd::mkfifo(&input_path, mode)?;
+                nix::unistd::mkfifo(&output_path, mode)?;
+            }
+
+            Ok(Self {
+                qemu_arg,
+                input_path,
+                output_path,
+            })
+        } else if platform::is_windows() {
+            // No need to call the equivalent of `mkfifo` here; QEMU acts as
+            // the named pipe server and creates the pipe itself.
+
+            Ok(Self {
+                // QEMU adds the "\\.\pipe\" prefix automatically.
+                qemu_arg: format!("pipe:{}", base_name),
+
+                // On Windows the pipe is duplex, so only one path
+                // needed.
+                input_path: format!(r"\\.\pipe\{}", base_name).into(),
+                output_path: PathBuf::new(),
+            })
+        } else {
+            unimplemented!();
+        }
+    }
+
+    pub fn qemu_arg(&self) -> &str {
+        &self.qemu_arg
+    }
+
+    /// Create an `Io` object for performing reads and writes.
+    pub fn open_io(&self) -> Result<Io<File, File>> {
+        let reader;
+        let writer;
+
+        if platform::is_unix() {
+            reader = File::open(&self.output_path)?;
+            writer = OpenOptions::new().write(true).open(&self.input_path)?;
+        } else if platform::is_windows() {
+            // Connect to the pipe, then clone the resulting `File` so
+            // that we can wrap the read side in a `BufReader`. The
+            // reader and writer must share the same underlying
+            // `Handle`, so this is different than opening the pipe
+            // twice.
+            writer = windows_open_pipe(&self.input_path)?;
+            reader = writer.try_clone()?;
+        } else {
+            unimplemented!();
+        }
+
+        Ok(Io::new(reader, writer))
+    }
+}
+
+/// Attempt to connect to a duplex named pipe in byte mode.
+fn windows_open_pipe(path: &Path) -> Result<File> {
+    let max_attempts = 100;
+    let mut attempt = 0;
+    loop {
+        attempt += 1;
+
+        match OpenOptions::new().read(true).write(true).open(&path) {
+            Ok(file) => return Ok(file),
+            Err(err) => {
+                if attempt >= max_attempts {
+                    return Err(err)?;
+                } else {
+                    // Sleep before trying again.
+                    thread::sleep(Duration::from_millis(100));
+                }
+            }
+        }
+    }
+}

xtask/src/platform.rs

@@ -0,0 +1,18 @@
+//! Functions to determine the host platform.
+//!
+//! Use the functions where possible instead of `#[cfg(...)]` so that
+//! code for all platforms gets checked at compile time.
+
+use std::env::consts;
+
+pub fn is_linux() -> bool {
+    consts::OS == "linux"
+}
+
+pub fn is_unix() -> bool {
+    consts::FAMILY == "unix"
+}
+
+pub fn is_windows() -> bool {
+    consts::FAMILY == "windows"
+}