Repository navigation
[AI Generated] BugFix: recover Cloud Hypervisor passthrough boot stalls - #4729
Conversation
Use fresh environments without redundant in-guest reboots for passthrough performance cases, and retry a timed-out Cloud Hypervisor passthrough domain once. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run passthrough domain destruction under a host-side timeout, capture process and kernel diagnostics when it stalls, and use an exact-process fallback without blocking LISA cleanup or restart paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Identify the exact Cloud Hypervisor process from its libvirt event-monitor argument on the host before issuing a bounded destroy, so recovery does not make an unbounded libvirt RPC first. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep bounded restart recovery active for up to four repeated pre-Linux boot stalls, while preserving the final timeout as a failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.
There was a problem hiding this comment.
Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.
There was a problem hiding this comment.
Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.
There was a problem hiding this comment.
Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.
|
LiliDeng please review this PR |
There was a problem hiding this comment.
Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.
There was a problem hiding this comment.
Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.
Vivek Yadav (vyadavmsft)
left a comment
There was a problem hiding this comment.
approving this but we should make new enviroment runbook driven.
Fix cleanup ownership and recovery gaps after connectivity loss during libvirt passthrough operations. This is follow-up hardening to #4689 (cached IOMMU allocation mappings and management-route protection) and #4729 (bounded Cloud Hypervisor passthrough stopping), not a replacement for those fixes. Previously, libvirt destroy/undefine errors could be logged and swallowed, allowing cleanup to return devices to the available pool even though the domain might still own them. Simply rethrowing is insufficient: the platform marks an environment Deleted before teardown, and the runner can then discard it or reset its nodes without scheduling another cleanup. Preserve failed teardown ownership inside the libvirt platform: - Propagate destroy and undefine failures before releasing devices. - Retain snapshots of the original nodes, disk directory, and forwarding rules independently of runner environment resets. - Retry deletion up to three times, retry pending cleanup before preparing or deploying environments, and retry again during platform shutdown. - Report unresolved final cleanup explicitly while preserving allocations and the device pool's management-route protection. - Track disk directories per environment so delayed cleanup cannot remove a different environment's current directory. Make teardown recoverable after a lost undefine response or stale handle. Reconcile domain absence through a fresh lookup and require a healthy libvirt connection before treating a missing domain as successful cleanup. Retain allocations when lookup fails or the domain state remains uncertain. Cloud Hypervisor passthrough stop failures use CloudHypervisorDomainStopError, not libvirtError, so base-class reconciliation alone missed the actual subclass path. Handle that error in cleanup too: a confirmed absent domain can release its devices, and an existing but confirmed stopped domain can continue through undefine. Active domains remain allocated. Keep the previously-failed-stop safety latch unchanged in the restart/reuse path; cleanup recovery must not make an unsafe guest reusable. No runner, shared-platform, documentation, or repository test-file changes. Validation: - 72 targeted checks passed: 49 existing tests plus 23 out-of-tree cleanup regression checks, including the actual Cloud Hypervisor subclass. - Verified missing domains, previously failed stops, inactive and active domains, unhealthy connections, permission/state-query failures, retained cleanup after runner reset, and offline cached device release. - Black, flake8, mypy, and git diff --check passed. - The original bare-metal connectivity-failure pipeline was not rerun; these results are local mocked regression coverage, not hardware proof. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix cleanup ownership and recovery gaps after connectivity loss during libvirt passthrough operations. This is follow-up hardening to #4689 (cached IOMMU allocation mappings and management-route protection) and #4729 (bounded Cloud Hypervisor passthrough stopping), not a replacement for those fixes. Previously, libvirt destroy/undefine errors could be logged and swallowed, allowing cleanup to return devices to the available pool even though the domain might still own them. Simply rethrowing is insufficient: the platform marks an environment Deleted before teardown, and the runner can then discard it or reset its nodes without scheduling another cleanup. Preserve failed teardown ownership inside the libvirt platform: - Propagate destroy and undefine failures before releasing devices. - Retain snapshots of the original nodes, disk directory, and forwarding rules independently of runner environment resets. - Retry deletion up to three times, retry pending cleanup before preparing or deploying environments, and retry again during platform shutdown. - Report unresolved final cleanup explicitly while preserving allocations and the device pool's management-route protection. - Track disk directories per environment so delayed cleanup cannot remove a different environment's current directory. Make teardown recoverable after a lost undefine response or stale handle. Reconcile domain absence through a fresh lookup and require a healthy libvirt connection before treating a missing domain as successful cleanup. Retain allocations when lookup fails or the domain state remains uncertain. Cloud Hypervisor passthrough stop failures use CloudHypervisorDomainStopError, not libvirtError, so base-class reconciliation alone missed the actual subclass path. Handle that error in cleanup too: a confirmed absent domain can release its devices, and an existing but confirmed stopped domain can continue through undefine. Active domains remain allocated. Keep the previously-failed-stop safety latch unchanged in the restart/reuse path; cleanup recovery must not make an unsafe guest reusable. Make libvirt port-forward cleanup retry-safe after partial deletion. If a FORWARD rule is deleted but NAT deletion fails, retry previously failed on the already-absent FORWARD rule and stranded the pending environment. Delete each rule independently, accept only the explicit missing-rule diagnostic with exit code 1, and propagate other command/transport errors. Remove completed forwarding entries immediately so later failures cannot force retries of earlier completed entries. Keep this handling local to libvirt; the shared firewall tool is unchanged. No runner, shared-platform, firewall-tool, documentation, or repository test-file changes. Validation: - 82 targeted checks passed: 50 existing tests plus 32 out-of-tree cleanup regression checks, including the actual Cloud Hypervisor subclass. - Verified missing domains, previously failed stops, inactive and active domains, unhealthy connections, permission/state-query failures, retained cleanup after runner reset, and offline cached device release. - Verified missing forwarding rules, partial NAT deletion failure, removal of completed entries, pending-owner recovery, forced privileged execution, and propagation of permission, lock, command, and transport failures. - Black, flake8, mypy, and git diff --check passed. - A focused physical run before this forwarding change passed both host-guest NIC passthrough performance cases and normal domain teardown. It used different devices for successive environments and did not encounter or inject the original cleanup connectivity failure. - The forwarding retry change has local fault-regression coverage only; no physical rerun or hardware proof of failure recovery is claimed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix cleanup ownership and recovery gaps after connectivity loss during libvirt passthrough operations. This is follow-up hardening to #4689 (cached IOMMU allocation mappings and management-route protection) and #4729 (bounded Cloud Hypervisor passthrough stopping), not a replacement for those fixes. Previously, libvirt destroy/undefine errors could be logged and swallowed, allowing cleanup to return devices to the available pool even though the domain might still own them. Simply rethrowing is insufficient: the platform marks an environment Deleted before teardown, and the runner can then discard it or reset its nodes without scheduling another cleanup. Preserve failed teardown ownership inside the libvirt platform: - Propagate destroy and undefine failures before releasing devices. - Retain snapshots of the original nodes, disk directory, and forwarding rules independently of runner environment resets. - Retry deletion up to three times, retry pending cleanup before preparing or deploying environments, and retry again during platform shutdown. - Report unresolved final cleanup explicitly while preserving allocations and the device pool's management-route protection. - Track disk directories per environment so delayed cleanup cannot remove a different environment's current directory. Make teardown recoverable after a lost undefine response or stale handle. Reconcile domain absence through a fresh lookup and require a healthy libvirt connection before treating a missing domain as successful cleanup. Retain allocations when lookup fails or the domain state remains uncertain. Cloud Hypervisor passthrough stop failures use CloudHypervisorDomainStopError, not libvirtError, so base-class reconciliation alone missed the actual subclass path. Handle that error in cleanup too: a confirmed absent domain can release its devices, and an existing but confirmed stopped domain can continue through undefine. Active domains remain allocated. Keep the previously-failed-stop safety latch unchanged in the restart/reuse path; cleanup recovery must not make an unsafe guest reusable. Make libvirt port-forward cleanup retry-safe after partial deletion. If a FORWARD rule is deleted but NAT deletion fails, retry previously failed on the already-absent FORWARD rule and stranded the pending environment. Delete each rule independently, accept only the explicit missing-rule diagnostic with exit code 1, and propagate other command/transport errors. Remove completed forwarding entries immediately so later failures cannot force retries of earlier completed entries. Keep this handling local to libvirt; the shared firewall tool is unchanged. No runner, shared-platform, firewall-tool, documentation, or repository test-file changes. Validation: - 82 targeted checks passed: 50 existing tests plus 32 out-of-tree cleanup regression checks, including the actual Cloud Hypervisor subclass. - Verified missing domains, previously failed stops, inactive and active domains, unhealthy connections, permission/state-query failures, retained cleanup after runner reset, and offline cached device release. - Verified missing forwarding rules, partial NAT deletion failure, removal of completed entries, pending-owner recovery, forced privileged execution, and propagation of permission, lock, command, and transport failures. - Black, flake8, mypy, and git diff --check passed. - A focused physical run before this forwarding change passed both host-guest NIC passthrough performance cases and normal domain teardown. It used different devices for successive environments and did not encounter or inject the original cleanup connectivity failure. - The forwarding retry change has local fault-regression coverage only; no physical rerun or hardware proof of failure recovery is claimed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Recover Cloud Hypervisor passthrough guests when firmware initialization stalls before the guest acquires an IP address.
This change:
Related Issue
Type of Change
Checklist
Test Validation
Key Test Cases:
verify_device_passthrough_on_guest|perf_tcp_iperf_passthrough_host_guest|perf_udp_iperf_passthrough_host_guest|perf_tcp_ntttcp_passthrough_two_guest
Impacted LISA Features:
StartStop
Tested Azure Marketplace Images:
Test Results
Additional validation:
git diff --check: PASSED