xtask: Rework the OVMF search to make it more robust

Prior to this commit we assumed that OVMF files for a given guest arch
have consistent file names across host operating systems, varying only
in the directory path. This turns out not to be a good assumption
though; the locations and names of OVMF files installed by system
packages very a lot between operating systems.

To make it easier to figure out where stuff is, add a number of
OS-specific functions that provide the fulls OVMF code and vars
paths. For now these functions cover Arch, CentOS, Debian/Ubuntu,
Fedora, and Windows. A selection of candidate paths is then collected
(one set of paths for Linux, a different set for Windows) and searched.

Along with the above changes, the `--ovmf-dir` option has been replaced
with separate `--ovmf-code` and `--ovmf-vars` options. When set, these
paths will be used instead of searching the candidate paths.

The `uefi-test-runner` directory is no longer automatically searched for
OVMF files since we no longer assume we know what these files would be
named. Hopefully the improved candidate search makes this moot, and the
`--ovmf-code`/`--ovmf-vars` options can still be used to search there if
desired.

CI changes:
* The aarch64 job has been simplified a bit since the system location of
  the files is now sufficient for `cargo xtask run` to work.
* The ia32 job now explicitly sets the location of the downloaded OVMF
  files.

Fixes #469

Author: nicholasbishop

.github/workflows/rust.yml

@@ -23,9 +23,6 @@ jobs:
         sudo add-apt-repository ppa:canonical-server/server-backports
         sudo apt-get update
         sudo apt-get install qemu-system-arm qemu-efi-aarch64 -y
-        # Copy the files so that the vars file isn't read-only.
-        cp /usr/share/AAVMF/AAVMF_CODE.fd uefi-test-runner/QEMU_EFI-pflash.raw
-        cp /usr/share/AAVMF/AAVMF_VARS.fd uefi-test-runner/vars-template-pflash.raw
 
     - name: Install stable
       uses: actions-rs/toolchain@v1
@@ -102,8 +99,8 @@ jobs:
         # guard against external changes breaking the CI.
         EDK2_NIGHTLY_COMMIT: 'ebb83e5475d49418afc32857f66111949928bcdc'
       run: |
-        curl -o uefi-test-runner/OVMF32_CODE.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_CODE.fd
-        curl -o uefi-test-runner/OVMF32_VARS.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_VARS.fd
+        curl -o OVMF32_CODE.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_CODE.fd
+        curl -o OVMF32_VARS.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_VARS.fd
 
     - name: Install stable
       uses: actions-rs/toolchain@v1
@@ -121,7 +118,7 @@ jobs:
       run: cargo xtask build --target ia32
 
     - name: Run VM tests
-      run: cargo xtask run --target ia32 --headless --ci
+      run: cargo xtask run --target ia32 --headless --ci --ovmf-code OVMF32_CODE.fd --ovmf-vars OVMF32_VARS.fd
       timeout-minutes: 2
 
   test:

README.md

@@ -87,7 +87,8 @@ Available commands:
                      Especially useful if you want to run the tests under
                      [WSL](https://docs.microsoft.com/en-us/windows/wsl) on Windows.
   - `--headless`: run QEMU without a GUI
-  - `--ovmf-dir <PATH>`: directory in which to look for OVMF files
+  - `--ovmf-code <PATH>`: path of an OVMF code file
+  - `--ovmf-vars <PATH>`: path of an OVMF vars file
   - `--release`: build in release mode
   - `--target {x86_64,ia32,aarch64}`: choose target UEFI arch
 - `test`: run unit tests and doctests on the host

xtask/src/opt.rs

@@ -140,9 +140,13 @@ pub struct QemuOpt {
     #[clap(long, action)]
     pub headless: bool,
 
-    /// Directory in which to look for OVMF files.
+    /// Path of an OVMF code file.
     #[clap(long, action)]
-    pub ovmf_dir: Option<PathBuf>,
+    pub ovmf_code: Option<PathBuf>,
+
+    /// Path of an OVMF vars file.
+    #[clap(long, action)]
+    pub ovmf_vars: Option<PathBuf>,
 }
 
 /// Build uefi-test-runner and run it in QEMU.

xtask/src/qemu.rs

@@ -14,67 +14,206 @@ use std::process::{Child, Command, Stdio};
 use std::{env, thread};
 use tempfile::TempDir;
 
+#[derive(Clone, Copy, Debug)]
+enum OvmfFileType {
+    Code,
+    Vars,
+}
+
+impl OvmfFileType {
+    fn as_str(&self) -> &'static str {
+        match self {
+            Self::Code => "code",
+            Self::Vars => "vars",
+        }
+    }
+}
+
 struct OvmfPaths {
     code: PathBuf,
     vars: PathBuf,
 }
 
 impl OvmfPaths {
-    fn from_dir(dir: &Path, arch: UefiArch) -> Self {
+    fn get_path(&self, file_type: OvmfFileType) -> &Path {
+        match file_type {
+            OvmfFileType::Code => &self.code,
+            OvmfFileType::Vars => &self.vars,
+        }
+    }
+
+    /// Get the Arch Linux OVMF paths for the given guest arch.
+    fn arch_linux(arch: UefiArch) -> Self {
         match arch {
+            // Package "edk2-armvirt".
             UefiArch::AArch64 => Self {
-                code: dir.join("QEMU_EFI-pflash.raw"),
-                vars: dir.join("vars-template-pflash.raw"),
+                code: "/usr/share/edk2-armvirt/aarch64/QEMU_CODE.fd".into(),
+                vars: "/usr/share/edk2-armvirt/aarch64/QEMU_VARS.fd".into(),
             },
+            // Package "edk2-ovmf".
             UefiArch::IA32 => Self {
-                code: dir.join("OVMF32_CODE.fd"),
-                vars: dir.join("OVMF32_VARS.fd"),
+                code: "/usr/share/edk2-ovmf/ia32/OVMF_CODE.fd".into(),
+                vars: "/usr/share/edk2-ovmf/ia32/OVMF_VARS.fd".into(),
             },
+            // Package "edk2-ovmf".
             UefiArch::X86_64 => Self {
-                code: dir.join("OVMF_CODE.fd"),
-                vars: dir.join("OVMF_VARS.fd"),
+                code: "/usr/share/edk2-ovmf/x64/OVMF_CODE.fd".into(),
+                vars: "/usr/share/edk2-ovmf/x64/OVMF_VARS.fd".into(),
             },
         }
     }
 
-    fn exists(&self) -> bool {
-        self.code.exists() && self.vars.exists()
+    /// Get the CentOS OVMF paths for the given guest arch.
+    fn centos_linux(arch: UefiArch) -> Option<Self> {
+        match arch {
+            // Package "edk2-aarch64".
+            UefiArch::AArch64 => Some(Self {
+                code: "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw".into(),
+                vars: "/usr/share/edk2/aarch64/vars-template-pflash.raw".into(),
+            }),
+            // There's no official ia32 package.
+            UefiArch::IA32 => None,
+            // Package "edk2-ovmf".
+            UefiArch::X86_64 => Some(Self {
+                // Use the `.secboot` variant because the CentOS package
+                // doesn't have a plain "OVMF_CODE.fd".
+                code: "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd".into(),
+                vars: "/usr/share/edk2/ovmf/OVMF_VARS.fd".into(),
+            }),
+        }
     }
 
-    /// Find path to OVMF files.
-    fn find(opt: &QemuOpt, arch: UefiArch) -> Result<Self> {
-        // If the path is specified in the settings, use it.
-        if let Some(ovmf_dir) = &opt.ovmf_dir {
-            let ovmf_paths = Self::from_dir(ovmf_dir, arch);
-            if ovmf_paths.exists() {
-                return Ok(ovmf_paths);
-            }
-            bail!("OVMF files not found in {}", ovmf_dir.display());
+    /// Get the Debian OVMF paths for the given guest arch. These paths
+    /// also work on Ubuntu.
+    fn debian_linux(arch: UefiArch) -> Self {
+        match arch {
+            // Package "qemu-efi-aarch64".
+            UefiArch::AArch64 => Self {
+                code: "/usr/share/AAVMF/AAVMF_CODE.fd".into(),
+                vars: "/usr/share/AAVMF/AAVMF_VARS.fd".into(),
+            },
+            // Package "ovmf-ia32".
+            UefiArch::IA32 => Self {
+                code: "/usr/share/OVMF/OVMF32_CODE_4M.secboot.fd".into(),
+                vars: "/usr/share/OVMF/OVMF32_VARS_4M.fd".into(),
+            },
+            // Package "ovmf".
+            UefiArch::X86_64 => Self {
+                code: "/usr/share/OVMF/OVMF_CODE.fd".into(),
+                vars: "/usr/share/OVMF/OVMF_VARS.fd".into(),
+            },
+        }
+    }
+
+    /// Get the Fedora OVMF paths for the given guest arch.
+    fn fedora_linux(arch: UefiArch) -> Self {
+        match arch {
+            // Package "edk2-aarch64".
+            UefiArch::AArch64 => Self {
+                code: "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw".into(),
+                vars: "/usr/share/edk2/aarch64/vars-template-pflash.raw".into(),
+            },
+            // Package "edk2-ovmf-ia32".
+            UefiArch::IA32 => Self {
+                code: "/usr/share/edk2/ovmf-ia32/OVMF_CODE.fd".into(),
+                vars: "/usr/share/edk2/ovmf-ia32/OVMF_VARS.fd".into(),
+            },
+            // Package "edk2-ovmf".
+            UefiArch::X86_64 => Self {
+                code: "/usr/share/edk2/ovmf/OVMF_CODE.fd".into(),
+                vars: "/usr/share/edk2/ovmf/OVMF_VARS.fd".into(),
+            },
         }
+    }
 
-        // Check whether the test runner directory contains the files.
-        let ovmf_dir = Path::new("uefi-test-runner");
-        let ovmf_paths = Self::from_dir(ovmf_dir, arch);
-        if ovmf_paths.exists() {
-            return Ok(ovmf_paths);
+    /// Get the Windows OVMF paths for the given guest arch.
+    fn windows(arch: UefiArch) -> Self {
+        match arch {
+            UefiArch::AArch64 => Self {
+                code: r"C:\Program Files\qemu\share\edk2-aarch64-code.fd".into(),
+                vars: r"C:\Program Files\qemu\share\edk2-arm-vars.fd".into(),
+            },
+            UefiArch::IA32 => Self {
+                code: r"C:\Program Files\qemu\share\edk2-i386-code.fd".into(),
+                vars: r"C:\Program Files\qemu\share\edk2-i386-vars.fd".into(),
+            },
+            UefiArch::X86_64 => Self {
+                code: r"C:\Program Files\qemu\share\edk2-x86_64-code.fd".into(),
+                // There's no x86_64 vars file, but the i386 one works.
+                vars: r"C:\Program Files\qemu\share\edk2-i386-vars.fd".into(),
+            },
         }
+    }
 
+    /// Get candidate paths where OVMF code/vars might exist for the
+    /// given guest arch and host platform.
+    fn get_candidate_paths(arch: UefiArch) -> Vec<Self> {
+        let mut candidates = Vec::new();
         if platform::is_linux() {
-            let possible_paths = [
-                // Most distros, including CentOS, Fedora, Debian, and Ubuntu.
-                Path::new("/usr/share/OVMF"),
-                // Arch Linux.
-                Path::new("/usr/share/ovmf/x64"),
-            ];
-            for path in possible_paths {
-                let ovmf_paths = Self::from_dir(path, arch);
-                if ovmf_paths.exists() {
-                    return Ok(ovmf_paths);
+            candidates.push(Self::arch_linux(arch));
+            if let Some(candidate) = Self::centos_linux(arch) {
+                candidates.push(candidate);
+            }
+            candidates.push(Self::debian_linux(arch));
+            candidates.push(Self::fedora_linux(arch));
+        }
+        if platform::is_windows() {
+            candidates.push(Self::windows(arch));
+        }
+        candidates
+    }
+
+    /// Search for an OVMF file (either code or vars).
+    ///
+    /// If `user_provided_path` is not None, it is always used. An error
+    /// is returned if the path does not exist.
+    ///
+    /// Otherwise, the paths in `candidates` are searched to find one
+    /// that exists. If none of them exist, an error is returned.
+    fn find_ovmf_file(
+        file_type: OvmfFileType,
+        user_provided_path: &Option<PathBuf>,
+        candidates: &[Self],
+    ) -> Result<PathBuf> {
+        if let Some(path) = user_provided_path {
+            // The user provided an exact path to use; verify that it
+            // exists.
+            if path.exists() {
+                Ok(path.to_owned())
+            } else {
+                bail!(
+                    "ovmf {} file does not exist: {}",
+                    file_type.as_str(),
+                    path.display()
+                );
+            }
+        } else {
+            for candidate in candidates {
+                let path = candidate.get_path(file_type);
+                if path.exists() {
+                    return Ok(path.to_owned());
                 }
             }
+
+            bail!(
+                "no ovmf {} file found in candidates: {:?}",
+                file_type.as_str(),
+                candidates
+                    .iter()
+                    .map(|c| c.get_path(file_type))
+                    .collect::<Vec<_>>(),
+            );
         }
+    }
+
+    /// Find path to OVMF files.
+    fn find(opt: &QemuOpt, arch: UefiArch) -> Result<Self> {
+        let candidates = Self::get_candidate_paths(arch);
+
+        let code = Self::find_ovmf_file(OvmfFileType::Code, &opt.ovmf_code, &candidates)?;
+        let vars = Self::find_ovmf_file(OvmfFileType::Vars, &opt.ovmf_vars, &candidates)?;
 
-        bail!("OVMF files not found anywhere");
+        Ok(Self { code, vars })
     }
 }