Add rollback for Hyper-V DDA assignment failures

Track each stage of Hyper-V DDA assignment so a later failure can unwind earlier host-side changes. The assignment path now records disabled PnP devices, successfully dismounted devices, and devices already attached to the VM.

On failure, rollback removes assigned devices from the VM, mounts dismounted devices back to the host, re-enables the PnP devices that were disabled, and reports rollback errors alongside the original assignment error.

Add a focused selftest that simulates a second-device dismount failure and verifies rollback removes, mounts, and re-enables devices in the expected order.

Author: vyadavmsft

lisa/microsoft/testsuites/device_passthrough/functional_tests.py

@@ -71,19 +71,31 @@ def verify_device_passthrough_on_guest(
     ) -> None:
         lspci = node.tools[Lspci]
         platform = environment.platform
+        if platform is None:
+            raise SkippedException(
+                "Device passthrough validation requires a LISA platform context. "
+                "Verify the runbook uses cloud-hypervisor or hyperv."
+            )
         platform_name = platform.type_name()
 
         if platform_name == CLOUD_HYPERVISOR:
             # Import at runtime to avoid libvirt dependency on other platforms.
-            from lisa.sut_orchestrator.libvirt.context import get_node_context
+            from lisa.sut_orchestrator.libvirt.context import (
+                get_node_context as get_libvirt_node_context,
+            )
+
+            node_context = get_libvirt_node_context(node)
         elif platform_name == HYPERV:
-            from lisa.sut_orchestrator.hyperv.context import get_node_context
+            from lisa.sut_orchestrator.hyperv.context import (
+                get_node_context as get_hyperv_node_context,
+            )
+
+            node_context = get_hyperv_node_context(node)
         else:
             raise SkippedException(
                 f"Device passthrough validation is not supported on '{platform_name}'"
             )
 
-        node_context = get_node_context(node)
         if not node_context.passthrough_devices:
             raise SkippedException("No passthrough devices are assigned to node")
 

lisa/microsoft/testsuites/performance/networkperf_passthrough.py

@@ -715,13 +715,17 @@ def _configure_passthrough_nic_for_node(
 
     def _get_passthrough_node_context(self, node: Node) -> Any:
         try:
-            from lisa.sut_orchestrator.libvirt.context import get_node_context
+            from lisa.sut_orchestrator.libvirt.context import (
+                get_node_context as get_libvirt_node_context,
+            )
 
-            return get_node_context(node)
+            return get_libvirt_node_context(node)
         except AssertionError:
-            from lisa.sut_orchestrator.hyperv.context import get_node_context
+            from lisa.sut_orchestrator.hyperv.context import (
+                get_node_context as get_hyperv_node_context,
+            )
 
-            return get_node_context(node)
+            return get_hyperv_node_context(node)
 
     def _get_device_bdf(self, device_addr_obj: Any) -> str:
         bus = getattr(device_addr_obj, "bus", "")
@@ -1184,27 +1188,30 @@ def _perf_ntttcp_with_windows_server(
                     f"Stderr: {receiver_result.stderr[:2000]}"
                 ) from parse_error
             if udp_mode:
-                perf_message = client_ntttcp.create_ntttcp_udp_performance_message(
-                    parsed_server_result,
-                    parsed_client_result,
-                    str(test_thread),
-                    buffer_size,
-                    test_case_name,
-                    test_result,
-                    client_mtu,
+                notifier.notify(
+                    client_ntttcp.create_ntttcp_udp_performance_message(
+                        parsed_server_result,
+                        parsed_client_result,
+                        str(test_thread),
+                        buffer_size,
+                        test_case_name,
+                        test_result,
+                        client_mtu,
+                    )
                 )
             else:
-                perf_message = client_ntttcp.create_ntttcp_tcp_performance_message(
-                    parsed_server_result,
-                    parsed_client_result,
-                    Decimal(0),
-                    str(test_thread),
-                    buffer_size,
-                    test_case_name,
-                    test_result,
-                    client_mtu,
+                notifier.notify(
+                    client_ntttcp.create_ntttcp_tcp_performance_message(
+                        parsed_server_result,
+                        parsed_client_result,
+                        Decimal(0),
+                        str(test_thread),
+                        buffer_size,
+                        test_case_name,
+                        test_result,
+                        client_mtu,
+                    )
                 )
-            notifier.notify(perf_message)
 
     def _get_host_nic_name(self, node: RemoteNode) -> str:
         ip = node.connection_info[constants.ENVIRONMENTS_NODES_REMOTE_ADDRESS]

lisa/sut_orchestrator/hyperv/hyperv_device_pool.py

@@ -408,29 +408,149 @@ def _assign_devices_to_vm(
     ) -> None:
         # Assign the devices to the VM
         escaped_vm_name = vm_name.replace("'", "''")
-        config_commands: List[str] = []
-        for device in devices:
-            escaped_instance_id = device.instance_id.replace("'", "''")
+        disabled_devices: List[DeviceAddressSchema] = []
+        dismounted_devices: List[DeviceAddressSchema] = []
+        assigned_devices: List[DeviceAddressSchema] = []
+        powershell = self._server.tools[PowerShell]
+
+        try:
+            for device in devices:
+                escaped_instance_id = device.instance_id.replace("'", "''")
+                escaped_location_path = device.location_path.replace("'", "''")
+                powershell.run_cmdlet(
+                    cmdlet=(
+                        f"Disable-PnpDevice -InstanceId '{escaped_instance_id}' "
+                        "-Confirm:$false"
+                    ),
+                    force_run=True,
+                )
+                disabled_devices.append(device)
+
+                powershell.run_cmdlet(
+                    cmdlet=(
+                        f"Dismount-VMHostAssignableDevice -Force "
+                        f"-LocationPath '{escaped_location_path}'"
+                    ),
+                    force_run=True,
+                )
+                dismounted_devices.append(device)
+
+                powershell.run_cmdlet(
+                    cmdlet=(
+                        f"Add-VMAssignableDevice "
+                        f"-LocationPath '{escaped_location_path}' "
+                        f"-VMName '{escaped_vm_name}'"
+                    ),
+                    force_run=True,
+                )
+                assigned_devices.append(device)
+        except LisaException as err:
+            rollback_errors = self._rollback_dda_assignment(
+                vm_name=vm_name,
+                disabled_devices=disabled_devices,
+                dismounted_devices=dismounted_devices,
+                assigned_devices=assigned_devices,
+            )
+            if rollback_errors:
+                raise LisaException(
+                    "Failed to assign Hyper-V DDA device(s) to VM "
+                    f"'{vm_name}': {err}. Rollback also failed: "
+                    f"{'; '.join(rollback_errors)}"
+                ) from err
+            raise
+
+    def _rollback_dda_assignment(
+        self,
+        vm_name: str,
+        disabled_devices: List[DeviceAddressSchema],
+        dismounted_devices: List[DeviceAddressSchema],
+        assigned_devices: List[DeviceAddressSchema],
+    ) -> List[str]:
+        powershell = self._server.tools[PowerShell]
+        escaped_vm_name = vm_name.replace("'", "''")
+        rollback_errors: List[str] = []
+
+        self.log.info(f"Rolling back Hyper-V DDA assignment for VM '{vm_name}'")
+
+        for device in reversed(assigned_devices):
             escaped_location_path = device.location_path.replace("'", "''")
-            config_commands.append(
-                f"Disable-PnpDevice -InstanceId '{escaped_instance_id}' "
-                "-Confirm:$false"
+            error = self._run_dda_rollback_cmdlet(
+                powershell=powershell,
+                description=(
+                    f"Remove DDA device '{device.location_path}' from VM '{vm_name}'"
+                ),
+                cmdlet=(
+                    f"Remove-VMAssignableDevice "
+                    f"-LocationPath '{escaped_location_path}' "
+                    f"-VMName '{escaped_vm_name}'"
+                ),
             )
-            config_commands.append(
-                f"Dismount-VMHostAssignableDevice -Force "
-                f"-LocationPath '{escaped_location_path}'"
+            if error:
+                rollback_errors.append(error)
+
+        for device in reversed(dismounted_devices):
+            escaped_location_path = device.location_path.replace("'", "''")
+            error = self._run_dda_rollback_cmdlet(
+                powershell=powershell,
+                description=f"Mount DDA device '{device.location_path}' on host",
+                cmdlet=(
+                    f"Mount-VMHostAssignableDevice "
+                    f"-LocationPath '{escaped_location_path}'"
+                ),
             )
-            config_commands.append(
-                f"Add-VMAssignableDevice -LocationPath '{escaped_location_path}' "
-                f"-VMName '{escaped_vm_name}'"
+            if error:
+                rollback_errors.append(error)
+
+        for device in reversed(disabled_devices):
+            escaped_instance_id = device.instance_id.replace("'", "''")
+            error = self._run_dda_rollback_cmdlet(
+                powershell=powershell,
+                description=(
+                    f"Enable PnP device '{device.instance_id}' for location path "
+                    f"'{device.location_path}'"
+                ),
+                cmdlet=(
+                    f"Enable-PnpDevice -InstanceId '{escaped_instance_id}' "
+                    "-Confirm:$false"
+                ),
             )
+            if error:
+                rollback_errors.append(error)
+                continue
 
-        powershell = self._server.tools[PowerShell]
-        for cmd in config_commands:
+            try:
+                self._wait_for_pnp_device_enabled(
+                    device.instance_id,
+                    device.location_path,
+                )
+            except LisaException as err:
+                rollback_errors.append(
+                    f"PnP device '{device.instance_id}' for location path "
+                    f"'{device.location_path}' was not enabled during rollback: "
+                    f"{err}"
+                )
+
+        for error in rollback_errors:
+            self.log.warning(error)
+
+        return rollback_errors
+
+    def _run_dda_rollback_cmdlet(
+        self,
+        powershell: PowerShell,
+        description: str,
+        cmdlet: str,
+    ) -> Optional[str]:
+        self.log.info(description)
+        try:
             powershell.run_cmdlet(
-                cmdlet=cmd,
+                cmdlet=cmdlet,
                 force_run=True,
             )
+        except LisaException as err:
+            return f"{description} failed: {err}"
+
+        return None
 
     def _set_device_passthrough_node_context(
         self,

lisa/tools/ntttcp.py

@@ -1009,7 +1009,7 @@ def create_ntttcp_result(
 
 class WindowsNtttcp(Ntttcp):
     _download_url = (
-        "https://github.com/microsoft/ntttcp/releases/latest/download/" "ntttcp.exe"
+        "https://github.com/microsoft/ntttcp/releases/latest/download/ntttcp.exe"
     )
     _total_mbps_pattern = re.compile(
         r"(?im)^\s*TOTAL\s+(?P<throughput>[0-9]+(?:\.[0-9]+)?)\s*$"
@@ -1033,7 +1033,7 @@ def dependencies(self) -> List[Type[Tool]]:
 
     def setup_system(self, udp_mode: bool = False, set_task_max: bool = True) -> None:
         self.node.tools[PowerShell].run_cmdlet(
-            "Set-NetFirewallProfile -Profile Domain,Public,Private " "-Enabled False",
+            "Set-NetFirewallProfile -Profile Domain,Public,Private -Enabled False",
             fail_on_error=False,
         )
 

selftests/test_hyperv_device_pool.py

@@ -0,0 +1,85 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT license.
+
+from types import SimpleNamespace
+from typing import Any, List, cast
+from unittest import TestCase
+from unittest.mock import MagicMock, patch
+
+from lisa.sut_orchestrator.hyperv.hyperv_device_pool import HyperVDevicePool
+from lisa.sut_orchestrator.hyperv.schema import (
+    DeviceAddressSchema,
+    HypervPlatformSchema,
+)
+from lisa.tools import PowerShell
+from lisa.util import LisaException
+
+
+class HyperVDevicePoolTestCase(TestCase):
+    def test_assign_devices_rolls_back_on_dismount_failure(self) -> None:
+        first_device = DeviceAddressSchema(
+            instance_id="PCI\\FIRST",
+            location_path="PCIROOT(1)#PCI(0000)",
+        )
+        second_device = DeviceAddressSchema(
+            instance_id="PCI\\SECOND",
+            location_path="PCIROOT(1)#PCI(0001)",
+        )
+        commands: List[str] = []
+
+        def run_cmdlet(cmdlet: str, **_: Any) -> str:
+            commands.append(cmdlet)
+            if (
+                "Dismount-VMHostAssignableDevice" in cmdlet
+                and second_device.location_path in cmdlet
+            ):
+                raise LisaException("pcip failed")
+            return ""
+
+        powershell = SimpleNamespace(run_cmdlet=MagicMock(side_effect=run_cmdlet))
+        node = SimpleNamespace(tools={PowerShell: powershell})
+        pool = HyperVDevicePool(
+            node=cast(Any, node),
+            runbook=HypervPlatformSchema(),
+            log=MagicMock(),
+        )
+
+        with patch.object(pool, "_wait_for_pnp_device_enabled") as wait_enabled:
+            with self.assertRaises(LisaException):
+                pool._assign_devices_to_vm(
+                    vm_name="vm1",
+                    devices=[first_device, second_device],
+                )
+
+        remove_index = next(
+            index
+            for index, command in enumerate(commands)
+            if "Remove-VMAssignableDevice" in command
+            and first_device.location_path in command
+        )
+        mount_index = next(
+            index
+            for index, command in enumerate(commands)
+            if "Mount-VMHostAssignableDevice" in command
+            and first_device.location_path in command
+        )
+        enable_second_index = next(
+            index
+            for index, command in enumerate(commands)
+            if "Enable-PnpDevice" in command and second_device.instance_id in command
+        )
+        enable_first_index = next(
+            index
+            for index, command in enumerate(commands)
+            if "Enable-PnpDevice" in command and first_device.instance_id in command
+        )
+
+        self.assertLess(remove_index, mount_index)
+        self.assertLess(mount_index, enable_second_index)
+        self.assertLess(enable_second_index, enable_first_index)
+        wait_enabled.assert_any_call(
+            second_device.instance_id, second_device.location_path
+        )
+        wait_enabled.assert_any_call(
+            first_device.instance_id, first_device.location_path
+        )