petri: Add the ability to collect crash dumps from guests

.config/nextest.toml

@@ -72,4 +72,4 @@ slow-timeout = { period = "30s", terminate-after = 2 }
 filter = 'package(~vmm_tests)'
 # VMM tests contain their own watchdog timer, but keep an extra long timer
 # here as a backup.
-slow-timeout = { period = "15m", terminate-after = 1 }
+slow-timeout = { period = "10m", terminate-after = 2 }

.github/workflows/upload-petri-results.yml

@@ -22,6 +22,9 @@ jobs:
       BASE_URL: https://openvmmghtestresults.blob.core.windows.net/results
 
     steps:
+      - name: Free Disk Space (Ubuntu)
+        uses: jlumbroso/[email protected]
+
       - name: Authenticate to Azure
         uses: azure/login@v1
         with:

Cargo.lock

@@ -5625,6 +5625,7 @@ dependencies = [
  "chipset_resources",
  "clap",
  "diag_client",
+ "disk_backend",
  "disk_backend_resources",
  "disk_vhd1",
  "disk_vhdmp",

petri/Cargo.toml

@@ -14,6 +14,7 @@ petri_artifacts_core.workspace = true
 petri_artifacts_vmm_test.workspace = true
 chipset_resources.workspace = true
 diag_client.workspace = true
+disk_backend.workspace = true
 disk_vhd1.workspace = true
 hvlite_defs.workspace = true
 hvlite_helpers.workspace = true

petri/guest-bootstrap/user-data

@@ -35,6 +35,9 @@ bootcmd:
   # Mounts module is not configured in some (all?) distros, so mount /cidata manually.
   - [mkdir, -p, /cidata]
   - [mount, LABEL=cidata, /cidata, -o, ro]
+    # Mount the crash dump folder manually too
+  - [mkdir, -p, /crash]
+  - [mount, LABEL=crashdump, /crash, -o, rw]
   # Disable snapd, which takes a long time to start and is not needed.
   - systemctl disable snapd.service snapd.apparmor snapd.seeded.service || true
   - systemctl mask snapd.service || true

petri/guest-bootstrap/user-data-no-agent

@@ -8,9 +8,9 @@ users:
     groups: users, admin
 
 bootcmd:
-  # Mounts module is not configured in some (all?) distros, so mount /cidata manually.
-  - [mkdir, -p, /cidata]
-  - [mount, LABEL=cidata, /cidata, -o, ro]
+  # Mount the crash dump folder for if things go wrong
+  - [mkdir, -p, /crash]
+  - [mount, LABEL=crashdump, /crash, -o, rw]
   # Disable snapd, which takes a long time to start and is not needed.
   - systemctl disable snapd.service snapd.apparmor snapd.seeded.service || true
   - systemctl mask snapd.service || true

petri/make_imc_hive/src/windows/mod.rs

@@ -35,6 +35,19 @@ pub(crate) fn main() -> anyhow::Result<()> {
     )?;
     vmbus_key.set_dword("AllowAllDevicesWhenIsolated", 1)?;
 
+    // Enable kernel mode crash dump and info
+    let crash_control_key = create_subkeys(
+        &hive,
+        &["SYSTEM", "CurrentControlSet", "Control", "CrashControl"],
+    )?;
+    crash_control_key.set_dword("AutoReboot", 1)?;
+    crash_control_key.set_dword("AlwaysKeepMemoryDump", 1)?;
+    crash_control_key.set_dword("CrashDumpEnabled", 2)?; // kernel memory dump
+    crash_control_key.set_sz("DedicatedDumpFile", "E:\\dumpfile.dmp")?;
+    crash_control_key.set_expand_sz("DumpFile", "E:\\memory.dmp")?;
+    // Set the size to the largest possible size FAT32 lets us have
+    crash_control_key.set_dword("DumpFileSize", 4095)?; // in MB
+
     // Windows defaults to 1, so we need to set it to 2 to cause Windows to
     // apply the IMC changes on first boot.
     hive.set_dword("Sequence", 2)?;

petri/make_imc_hive/src/windows/offreg.rs

@@ -19,6 +19,7 @@ use windows_sys::Wdk::System::OfflineRegistry::ORHKEY;
 use windows_sys::Wdk::System::OfflineRegistry::ORSaveHive;
 use windows_sys::Wdk::System::OfflineRegistry::ORSetValue;
 use windows_sys::Win32::System::Registry::REG_DWORD;
+use windows_sys::Win32::System::Registry::REG_EXPAND_SZ;
 use windows_sys::Win32::System::Registry::REG_MULTI_SZ;
 use windows_sys::Win32::System::Registry::REG_SZ;
 
@@ -153,6 +154,23 @@ impl Key {
         Ok(())
     }
 
+    pub fn set_expand_sz(&self, name: &str, value: &str) -> std::io::Result<()> {
+        let name16 = name.encode_utf16().chain([0]).collect::<Vec<_>>();
+        let value16 = value.encode_utf16().chain([0]).collect::<Vec<_>>();
+        // SAFETY: calling as documented with owned key and null-terminated
+        // name and value.
+        unsafe {
+            chk(ORSetValue(
+                self.0,
+                name16.as_ptr(),
+                REG_EXPAND_SZ,
+                value16.as_ptr().cast(),
+                value16.len() as u32 * 2,
+            ))?;
+        }
+        Ok(())
+    }
+
     pub fn set_multi_sz<'a>(
         &self,
         name: &str,

petri/src/disk_image.rs

@@ -140,12 +140,14 @@ impl AgentImage {
     }
 }
 
-enum PathOrBinary<'a> {
+pub(crate) const SECTOR_SIZE: u64 = 512;
+
+pub(crate) enum PathOrBinary<'a> {
     Path(&'a Path),
     Binary(&'a [u8]),
 }
 
-fn build_fat32_disk_image(
+pub(crate) fn build_fat32_disk_image(
     file: &mut (impl Read + Write + Seek),
     gpt_name: &str,
     volume_label: &[u8; 11],
@@ -163,7 +165,6 @@ fn build_fat32_disk_image(
 }
 
 fn build_gpt(file: &mut (impl Read + Write + Seek), name: &str) -> anyhow::Result<Range<u64>> {
-    const SECTOR_SIZE: u64 = 512;
     let mut gpt = gptman::GPT::new_from(file, SECTOR_SIZE, Guid::new_random().into())?;
 
     // Set up the "Protective" Master Boot Record

petri/src/test.rs

@@ -152,7 +152,7 @@ impl Test {
 
         for hook in post_test_hooks {
             tracing::info!(name = hook.name(), "Running post-test hook");
-            if let Err(e) = hook.run() {
+            if let Err(e) = hook.run(r.is_ok()) {
                 tracing::error!(
                     error = e.as_ref() as &dyn std::error::Error,
                     "Post-test hook failed"
@@ -247,11 +247,11 @@ pub struct PetriPostTestHook {
     /// The name of the hook.
     name: String,
     /// The hook function.
-    hook: Box<dyn FnOnce() -> anyhow::Result<()> + Send>,
+    hook: Box<dyn FnOnce(bool) -> anyhow::Result<()>>,
 }
 
 impl PetriPostTestHook {
-    pub fn new(name: String, hook: impl FnOnce() -> anyhow::Result<()> + Send + 'static) -> Self {
+    pub fn new(name: String, hook: impl FnOnce(bool) -> anyhow::Result<()> + 'static) -> Self {
         Self {
             name,
             hook: Box::new(hook),
@@ -262,8 +262,8 @@ impl PetriPostTestHook {
         &self.name
     }
 
-    pub fn run(self) -> anyhow::Result<()> {
-        (self.hook)()
+    pub fn run(self, test_passed: bool) -> anyhow::Result<()> {
+        (self.hook)(test_passed)
     }
 }
 

petri/src/vm/hyperv/mod.rs

@@ -33,6 +33,8 @@ use crate::openhcl_diag::OpenHclDiagHandler;
 use crate::vm::append_cmdline;
 use anyhow::Context;
 use async_trait::async_trait;
+use disk_backend::sync_wrapper::BlockingDisk;
+use disk_vhdmp::VhdmpDisk;
 use get_resources::ged::FirmwareEvent;
 use pal_async::DefaultDriver;
 use pal_async::pipe::PolledPipe;
@@ -50,7 +52,9 @@ use std::io::ErrorKind;
 use std::io::Write;
 use std::path::Path;
 use std::path::PathBuf;
+use std::sync::Arc;
 use std::time::Duration;
+use tempfile::TempPath;
 use vm::HyperVVM;
 use vmgs_resources::GuestStateEncryptionPolicy;
 
@@ -159,6 +163,58 @@ impl PetriVmmBackend for HyperVPetriBackend {
         }
     }
 
+    fn create_guest_dump_disk() -> anyhow::Result<
+        Option<(
+            Arc<TempPath>,
+            Box<dyn FnOnce() -> anyhow::Result<Box<dyn fatfs::ReadWriteSeek>>>,
+        )>,
+    > {
+        // Make a 16 GiB dynamic VHD for guest crash dumps.
+        let crash_disk = tempfile::Builder::new()
+            .suffix(".vhdx")
+            .make(|path| disk_vhdmp::Vhd::create_dynamic(path, 16 * 1024, true))
+            .context("error creating crash dump vhdx")?;
+        let (crash_disk, crash_disk_path) = crash_disk.into_parts();
+        crash_disk
+            .attach_for_raw_access(false)
+            .context("error attaching crash dump vhdx")?;
+        let mut crash_disk = BlockingDisk::new(
+            disk_backend::Disk::new(
+                VhdmpDisk::new(crash_disk, false).context("failed opening vhdmp")?,
+            )
+            .unwrap(),
+        );
+
+        // Format the VHD with FAT32.
+        crate::disk_image::build_fat32_disk_image(
+            &mut crash_disk,
+            "CRASHDUMP",
+            b"crashdump  ",
+            &[],
+        )
+        .context("error writing empty crash disk filesystem")?;
+
+        // Prepare the hook to extract crash dumps after the test.
+        let crash_disk_path = Arc::new(crash_disk_path);
+        let hook_crash_disk = crash_disk_path.clone();
+        let disk_opener = Box::new(move || {
+            let mut vhd = Err(anyhow::Error::msg("haven't tried to open the vhd yet"));
+            // The VM may not be fully shut down immediately, do some retries
+            for _ in 0..5 {
+                vhd = VhdmpDisk::open_vhd(hook_crash_disk.as_ref(), true)
+                    .context("failed opening vhd");
+                if vhd.is_ok() {
+                    break;
+                } else {
+                    std::thread::sleep(Duration::from_secs(3));
+                }
+            }
+            let vhdmp = VhdmpDisk::new(vhd?, true).context("failed opening vhdmp")?;
+            Ok(Box::new(BlockingDisk::new(disk_backend::Disk::new(vhdmp).unwrap())) as _)
+        });
+        Ok(Some((crash_disk_path, disk_opener)))
+    }
+
     fn new(_resolver: &ArtifactResolver<'_>) -> Self {
         HyperVPetriBackend {}
     }
@@ -180,6 +236,7 @@ impl PetriVmmBackend for HyperVPetriBackend {
             boot_device_type,
             vmgs,
             tpm_state_persistence,
+            guest_crash_disk,
         } = config;
 
         let PetriVmResources { driver, log_source } = resources;
@@ -570,6 +627,16 @@ impl PetriVmmBackend for HyperVPetriBackend {
             }
         }
 
+        if let Some(guest_crash_disk) = guest_crash_disk {
+            vm.add_vhd(
+                &guest_crash_disk,
+                powershell::ControllerType::Scsi,
+                Some(super::PETRI_VTL0_SCSI_CRASH_LUN),
+                Some(petri_vtl0_scsi),
+            )
+            .await?;
+        }
+
         let serial_pipe_path = vm.set_vm_com_port(1).await?;
         let serial_log_file = log_source.log_file("guest")?;
         log_tasks.push(driver.spawn(

petri/src/vm/mod.rs

@@ -12,6 +12,7 @@ use crate::PetriLogSource;
 use crate::PetriTestParams;
 use crate::ShutdownKind;
 use crate::disk_image::AgentImage;
+use crate::disk_image::SECTOR_SIZE;
 use crate::openhcl_diag::OpenHclDiagHandler;
 use async_trait::async_trait;
 use get_resources::ged::FirmwareEvent;
@@ -37,7 +38,9 @@ use std::hash::Hash;
 use std::hash::Hasher;
 use std::path::Path;
 use std::path::PathBuf;
+use std::sync::Arc;
 use std::time::Duration;
+use tempfile::TempPath;
 use vmgs_resources::GuestStateEncryptionPolicy;
 
 /// The set of artifacts and resources needed to instantiate a
@@ -124,6 +127,8 @@ pub struct PetriVmConfig {
     pub agent_image: Option<AgentImage>,
     /// Agent to run in OpenHCL
     pub openhcl_agent_image: Option<AgentImage>,
+    /// Disk to use for guest crash dumps
+    pub guest_crash_disk: Option<Arc<TempPath>>,
     /// VM guest state
     pub vmgs: PetriVmgsResource,
     /// The boot device type for the VM
@@ -157,6 +162,15 @@ pub trait PetriVmmBackend {
     /// Get the default servicing flags (based on what this backend supports)
     fn default_servicing_flags() -> OpenHclServicingFlags;
 
+    /// Create a disk for guest crash dumps, and a post-test hook to open the disk
+    /// to allow for reading the dumps.
+    fn create_guest_dump_disk() -> anyhow::Result<
+        Option<(
+            Arc<TempPath>,
+            Box<dyn FnOnce() -> anyhow::Result<Box<dyn fatfs::ReadWriteSeek>>>,
+        )>,
+    >;
+
     /// Resolve any artifacts needed to use this backend
     fn new(resolver: &ArtifactResolver<'_>) -> Self;
 
@@ -171,6 +185,7 @@ pub trait PetriVmmBackend {
 
 pub(crate) const PETRI_VTL0_SCSI_BOOT_LUN: u8 = 0;
 pub(crate) const PETRI_VTL0_SCSI_PIPETTE_LUN: u8 = 1;
+pub(crate) const PETRI_VTL0_SCSI_CRASH_LUN: u8 = 2;
 
 /// A constructed Petri VM
 pub struct PetriVm<T: PetriVmmBackend> {
@@ -209,6 +224,55 @@ impl<T: PetriVmmBackend> PetriVmBuilder<T> {
             Firmware::Uefi { .. } | Firmware::OpenhclUefi { .. } => BootDeviceType::Scsi,
         };
 
+        let guest_crash_disk = if matches!(
+            artifacts.firmware.os_flavor(),
+            OsFlavor::Windows | OsFlavor::Linux
+        ) {
+            let (guest_crash_disk, guest_dump_disk_hook) = T::create_guest_dump_disk()?.unzip();
+            if let Some(guest_dump_disk_hook) = guest_dump_disk_hook {
+                let logger = params.logger.clone();
+                params
+                    .post_test_hooks
+                    .push(crate::test::PetriPostTestHook::new(
+                        "extract guest crash dumps".into(),
+                        move |test_passed| {
+                            if test_passed {
+                                return Ok(());
+                            }
+                            let mut disk = guest_dump_disk_hook()?;
+                            let gpt = gptman::GPT::read_from(&mut disk, SECTOR_SIZE)?;
+                            let partition = fscommon::StreamSlice::new(
+                                &mut disk,
+                                gpt[1].starting_lba * SECTOR_SIZE,
+                                gpt[1].ending_lba * SECTOR_SIZE,
+                            )?;
+                            let fs = fatfs::FileSystem::new(partition, fatfs::FsOptions::new())?;
+                            for entry in fs.root_dir().iter() {
+                                let Ok(entry) = entry else {
+                                    tracing::warn!(
+                                        ?entry,
+                                        "failed to read entry in guest crash dump disk"
+                                    );
+                                    continue;
+                                };
+                                if !entry.is_file() {
+                                    tracing::warn!(
+                                        ?entry,
+                                        "skipping non-file entry in guest crash dump disk"
+                                    );
+                                    continue;
+                                }
+                                logger.write_attachment(&entry.file_name(), entry.to_file())?;
+                            }
+                            Ok(())
+                        },
+                    ));
+            }
+            guest_crash_disk
+        } else {
+            None
+        };
+
         Ok(Self {
             backend: artifacts.backend,
             config: PetriVmConfig {
@@ -222,6 +286,7 @@ impl<T: PetriVmmBackend> PetriVmBuilder<T> {
                 openhcl_agent_image: artifacts.openhcl_agent_image,
                 vmgs: PetriVmgsResource::Ephemeral,
                 tpm_state_persistence: true,
+                guest_crash_disk,
             },
             modify_vmm_config: None,
             resources: PetriVmResources {