wip

Author: mattkur

petri/src/vm/hyperv/hyperv.psm1

@@ -479,33 +479,34 @@ function Set-Vtl2Settings {
         [Parameter(Mandatory = $true)]
         [string]$SettingsFile,
 
-        [string]$ClientName = 'Petri',
-
-        [int]$ConvertDepth = 8
+        [string]$ClientName = 'Petri'
     )
 
-    # TODO: Support protobuf in addition to JSON formatted settings.
-    # (Should be easy, right? Just have the serde code write the pb output directly and read it
-    # into a byte array here instead of reading the JSON file.)
-    $settingsJson = Get-Content -Raw -Path $SettingsFile| ConvertTo-Json -Depth $ConvertDepth -Compress
+    $settingsContent = Get-Content -Raw -Path $SettingsFile
 
     $guestManagement = Get-VmGuestManagementService
 
     $options = New-Object Microsoft.Management.Infrastructure.Options.CimOperationOptions
     $options.SetCustomOption("ClientName", $ClientName, $false)
 
+    Write-Host "aaaa"
+
     # Parameter - VmId
     $p1 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("VmId", $VmId.ToString(), [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In)
 
+    Write-Host "bbbb"
+
     # Parameter - Namespace
     $p2 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("Namespace", $Namespace, [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In)
 
+    Write-Host "cccc"
+
     # Parameter - Settings
     # The input is a byte buffer with the size prepended.
     # Size is a uint32 in network byte order (i.e. Big Endian)
-    # Size includes the itself and the payload.
+    # Size includes the size itself and the payload.
 
-    $bytes = [system.Text.Encoding]::UTF8.GetBytes($settingsJson)
+    $bytes = [system.Text.Encoding]::UTF8.GetBytes($settingsContent)
 
     $header = [System.BitConverter]::GetBytes([uint32]($bytes.Length + 4))
     if ([System.BitConverter]::IsLittleEndian) {

petri/src/vm/hyperv/mod.rs

@@ -69,12 +69,21 @@ pub struct HyperVPetriRuntime {
     is_isolated: bool,
 }
 
+pub struct HyperVPetriConfig {
+    pub initial_vtl2_settings: Option<vtl2_settings_proto::Vtl2Settings>,
+}
+
 impl HyperVPetriRuntime {
     pub async fn add_openhcl_nvme_storage<P: AsRef<Path>>(
         &self,
         instance_id: Option<&guid::Guid>,
         vhd_paths: &[P],
     ) -> anyhow::Result<()> {
+        for vhd_path in vhd_paths {
+            acl_for_vm(vhd_path, Some(*self.vm.vmid()), VmFileAccess::FullControl)
+                .context("grant VM access to VHD")?;
+        }
+
         self.vm
             .add_openhcl_nvme_storage(instance_id, vhd_paths)
             .await
@@ -90,7 +99,7 @@ impl HyperVPetriRuntime {
 
 #[async_trait]
 impl PetriVmmBackend for HyperVPetriBackend {
-    type VmmConfig = ();
+    type VmmConfig = HyperVPetriConfig;
     type VmRuntime = HyperVPetriRuntime;
 
     fn check_compat(firmware: &Firmware, arch: MachineArch) -> bool {
@@ -113,10 +122,6 @@ impl PetriVmmBackend for HyperVPetriBackend {
         modify_vmm_config: Option<impl FnOnce(Self::VmmConfig) -> Self::VmmConfig + Send>,
         resources: &PetriVmResources,
     ) -> anyhow::Result<Self::VmRuntime> {
-        if modify_vmm_config.is_some() {
-            panic!("specified modify_vmm_config, but that is not supported for hyperv");
-        }
-
         let PetriVmConfig {
             name,
             arch,
@@ -413,7 +418,7 @@ impl PetriVmmBackend for HyperVPetriBackend {
             // Hyper-V (e.g., if it is in a WSL filesystem).
             let igvm_file = temp_dir.path().join("igvm.bin");
             fs_err::copy(src_igvm_file, &igvm_file).context("failed to copy igvm file")?;
-            acl_read_for_vm(&igvm_file, Some(*vm.vmid()))
+            acl_for_vm(&igvm_file, Some(*vm.vmid()), VmFileAccess::Read)
                 .context("failed to set ACL for igvm file")?;
 
             // TODO: only increase VTL2 memory on debug builds
@@ -511,6 +516,21 @@ impl PetriVmmBackend for HyperVPetriBackend {
             hyperv_serial_log_task(driver.clone(), serial_pipe_path, serial_log_file),
         ));
 
+        // todo mattkur
+        let initial_vtl2_settings = if let Some(f) = modify_vmm_config {
+            f(HyperVPetriConfig {
+                initial_vtl2_settings: None,
+            })
+            .initial_vtl2_settings
+        } else {
+            None
+        };
+
+        if let Some(settings) = initial_vtl2_settings {
+            tracing::info!(?settings, "applying initial VTL2 settings");
+            vm.set_base_vtl2_settings(&settings).await?;
+        }
+
         vm.start().await?;
 
         Ok(HyperVPetriRuntime {
@@ -635,17 +655,36 @@ impl PetriVmRuntime for HyperVPetriRuntime {
     }
 }
 
-fn acl_read_for_vm(path: &Path, id: Option<guid::Guid>) -> anyhow::Result<()> {
+/// The type of access to grant the VM for a file.
+#[expect(missing_docs)]
+enum VmFileAccess {
+    Read,
+    FullControl,
+}
+
+/// The Hyper-V security model requires that the VM be granted explicit access to any
+/// resources assigned. Hyper-V takes care of this when you use the admin facing
+/// PowerShell cmdlets and some WMI flows. But, some tests use lower level APIs that don't
+/// do this automatically.
+fn acl_for_vm<P: AsRef<Path>>(
+    path: P,
+    id: Option<guid::Guid>,
+    access: VmFileAccess,
+) -> anyhow::Result<()> {
     let sid_arg = format!(
-        "NT VIRTUAL MACHINE\\{name}:R",
+        "NT VIRTUAL MACHINE\\{name}:{perm}",
         name = if let Some(id) = id {
             format!("{id:X}")
         } else {
             "Virtual Machines".to_string()
+        },
+        perm = match access {
+            VmFileAccess::Read => "R",
+            VmFileAccess::FullControl => "F",
         }
     );
     let output = std::process::Command::new("icacls.exe")
-        .arg(path)
+        .arg(path.as_ref())
         .arg("/grant")
         .arg(sid_arg)
         .output()
@@ -654,6 +693,7 @@ fn acl_read_for_vm(path: &Path, id: Option<guid::Guid>) -> anyhow::Result<()> {
         let stderr = String::from_utf8_lossy(&output.stderr);
         anyhow::bail!("icacls failed: {stderr}");
     }
+
     Ok(())
 }
 

petri/src/vm/hyperv/powershell.rs

@@ -1128,6 +1128,10 @@ pub async fn run_check_vm_host_supports_hyperv_storage() -> anyhow::Result<bool>
 ///
 /// This is true if there are Microsoft-internal utilties installed on the
 /// host that support this feature.
+///
+/// TODO: Check that the resource is successfully added to the VM.
+/// e.g. if there is a signature issue, it seems that this call completes
+/// yet there was an error.
 pub async fn run_configure_microsoft_internal_nvme_storage<P: AsRef<Path>>(
     vmid: &Guid,
     instance_id: Option<&Guid>,
@@ -1171,6 +1175,8 @@ pub async fn run_set_base_vtl2_settings(
         .write_all(serde_json::to_string(vtl2_settings)?.as_bytes())
         .context("writing settings to tempfile")?;
 
+    tracing::info!(?tempfile, ?vtl2_settings, ?vmid, "set base vtl2 settings");
+
     run_host_cmd(
         PowerShellBuilder::new()
             .cmdlet("Import-Module")

petri/src/vm/mod.rs

@@ -323,7 +323,7 @@ impl<T: PetriVmmBackend> PetriVmBuilder<T> {
         let mut tasks = Vec::new();
 
         {
-            const TIMEOUT_DURATION_MINUTES: u64 = 7;
+            const TIMEOUT_DURATION_MINUTES: u64 = 20; // DO NOT MERGE
             const TIMER_DURATION: Duration = Duration::from_secs(TIMEOUT_DURATION_MINUTES * 60);
             let log_source = resources.log_source.clone();
             let inspect_task =

vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs

@@ -18,6 +18,7 @@ use mesh::rpc::RpcSend;
 use nvme_resources::NamespaceDefinition;
 use nvme_resources::NvmeControllerHandle;
 use petri::PetriVmBuilder;
+#[cfg(windows)]
 use petri::hyperv::HyperVPetriBackend;
 use petri::openvmm::OpenVmmPetriBackend;
 use petri::pipette::PipetteClient;
@@ -37,10 +38,11 @@ use storvsp_resources::ScsiControllerHandle;
 use storvsp_resources::ScsiDeviceAndPath;
 use storvsp_resources::ScsiPath;
 use vm_resource::IntoResource;
+#[cfg(windows)]
 use vmm_test_macros::hyperv_test;
 use vmm_test_macros::openvmm_test;
+#[cfg(windows)]
 use vtl2_settings_proto::Vtl2Settings;
-use vtl2_settings_proto::Vtl2SettingsFixed;
 
 /// Create a VPCI device config for an NVMe controller assigned to VTL2, with a single namespace.
 /// The namespace will be backed by either a file or a ramdisk, depending on whether
@@ -624,12 +626,16 @@ async fn openhcl_linux_storvsp_dvd_nvme(
     Ok(())
 }
 
+#[cfg(windows)]
 #[hyperv_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))]
 pub async fn add_openhcl_nvme_storage(
     config: PetriVmBuilder<HyperVPetriBackend>,
 ) -> anyhow::Result<()> {
-    let instance_id = Guid::new_random();
+    let vtl0_instance_id = Guid::new_random();
+    let vtl2_instance_id = Guid::new_random();
+    const VHD_SIZE: u64 = 200 * 1024 * 1024; // 200 MiB
 
+    // todo: generate a better temp file name
     let temp_dir = tempfile::tempdir()?;
     let vhd_path = temp_dir.path().join("test.vhd");
     let file = File::options()
@@ -641,43 +647,78 @@ pub async fn add_openhcl_nvme_storage(
         .open(&vhd_path)
         .context("create file")?;
 
-    file.set_len(32 * 1024 * 1024).context("set file length")?;
+    file.set_len(VHD_SIZE).context("set file length")?;
     disk_vhd1::Vhd1Disk::make_fixed(&file).context("make fixed")?;
 
-    let (mut vm, agent) = config.run().await?;
-    vm.backend()
-        .add_openhcl_nvme_storage(Some(&instance_id), &[vhd_path.to_str().unwrap()])
-        .await?;
-
-    let mut vtl2_settings = Vtl2Settings {
+    let initial_vtl2_settings = Vtl2Settings {
         version: vtl2_settings_proto::vtl2_settings_base::Version::V1.into(),
-        dynamic: Some(Default::default()),
-        fixed: Some(Default::default()),
+        dynamic: Some(vtl2_settings_proto::Vtl2SettingsDynamic {
+            storage_controllers: vec![
+                Vtl2StorageControllerBuilder::scsi()
+                    .with_instance_id(vtl0_instance_id)
+                    .build(),
+            ],
+            ..Default::default()
+        }),
+        fixed: None,
         namespace_settings: Default::default(),
     };
-    vtl2_settings
-        .dynamic
-        .as_mut()
-        .unwrap()
-        .storage_controllers
+
+    // This is a bit ugly; ideally the HyperV backend would munge and set any
+    // VTL2 settings to the modify call (just like happens for OpenVMM).
+    // For now, just take the expedient path to get this test stood up.
+    let mut vtl2_settings = initial_vtl2_settings.clone();
+
+    let (mut vm, agent) = config
+        .with_vmbus_redirect(true)
+        .modify_backend(move |b| {
+            assert!(b.initial_vtl2_settings.is_none());
+            petri::hyperv::HyperVPetriConfig {
+                initial_vtl2_settings: Some(initial_vtl2_settings),
+            }
+        })
+        .run()
+        .await?;
+
+    // Runtime add the NVMe device and update the VTL2 settings to
+    // include it.
+    //
+    // This _could_ happen before the VM is started, but do it at runtime
+    // for test simplicity.
+
+    vm.backend()
+        .add_openhcl_nvme_storage(Some(&vtl2_instance_id), &[vhd_path.to_str().unwrap()])
+        .await?;
+
+    vtl2_settings.dynamic.as_mut().unwrap().storage_controllers[0]
+        .luns
         .push(
-            Vtl2StorageControllerBuilder::scsi()
-                .with_instance_id(Guid::new_random())
-                .add_lun(
-                    Vtl2LunBuilder::disk()
-                        .with_location(0)
-                        .with_physical_device(Vtl2StorageBackingDeviceBuilder::new(
-                            ControllerType::Nvme,
-                            instance_id,
-                            1,
-                        )),
-                )
+            Vtl2LunBuilder::disk()
+                .with_location(0)
+                .with_physical_device(Vtl2StorageBackingDeviceBuilder::new(
+                    ControllerType::Nvme,
+                    vtl2_instance_id,
+                    1,
+                ))
                 .build(),
         );
     vm.backend().set_base_vtl2_settings(&vtl2_settings).await?;
 
-    // todo: configure vtl2 settings
-    // todo: make sure the disk shows up in the VTL0 guest
+    match vm.inspect_openhcl("vm/nvme/devices", None, None).await {
+        Err(e) => tracing::error!(?e, "Failed to inspect NVMe devices"),
+        Ok(devices) => tracing::info!(devices = %devices.json(), "NVMe devices"),
+    }
+
+    test_storage_linux(
+        &agent,
+        vec![ExpectedGuestDevice {
+            controller_guid: vtl0_instance_id,
+            lun: 0,
+            disk_size_sectors: (VHD_SIZE / 512) as usize,
+            friendly_name: "nvme".to_string(),
+        }],
+    )
+    .await?;
 
     agent.power_off().await?;
     vm.wait_for_clean_teardown().await?;