Skip to content

Harden libvirt connection keepalive handling - #4539

Merged
vyadavmsft merged 6 commits into
mainfrom
vyadav_hune_fix5
Jun 24, 2026
Merged

Harden libvirt connection keepalive handling#4539
vyadavmsft merged 6 commits into
mainfrom
vyadav_hune_fix5

Conversation

@vyadavmsft

Copy link
Copy Markdown
Collaborator

The Cloud Hypervisor passthrough run failed during deployment after libvirtd closed the client connection with a keepalive timeout. Once that happened, the shared virConnect stayed stale and later environments failed immediately with 'client socket is closed'.

Configure libvirtd to stop sending daemon-side keepalive probes while LISA owns the host, disable client-side keepalive probes on opened connections, and reconnect/retry focused domain operations when a stale client socket is detected.

Description

Related Issue

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Documentation update

Checklist

  • Description is filled in above
  • No credentials, secrets, or internal details are included
  • Peer review requested (if not, add required peer reviewers after raising PR)
  • Tests executed and results posted below

Test Validation

Key Test Cases:

Impacted LISA Features:

Tested Azure Marketplace Images:

Test Results

Image VM Size Result
PASSED / FAILED / SKIPPED

@vyadavmsft
vyadavmsft requested a review from anirudhrb as a code owner June 16, 2026 01:26
Copilot AI review requested due to automatic review settings June 16, 2026 01:26
@vyadavmsft
vyadavmsft requested a review from pupacha as a code owner June 16, 2026 01:26
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the libvirt SUT orchestrator’s connection handling to avoid stale shared virConnect objects after libvirtd closes the client socket (e.g., keepalive timeouts), and adds targeted reconnect/retry logic around key domain operations.

Changes:

  • Disables daemon-side keepalive while LISA owns the host (via libvirtd.conf) and disables client-side keepalive on opened connections.
  • Replaces direct defineXML / lookupByName usage with helpers that reopen the libvirt connection and retry when a closed-socket error is detected.
  • Adds reconnect-and-retry handling for domain destroy/undefine when a stale socket is encountered.

Comment thread lisa/sut_orchestrator/libvirt/platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
The Cloud Hypervisor passthrough run failed during deployment after libvirtd closed the client connection with a keepalive timeout. Once that happened, the shared virConnect stayed stale and later environments failed immediately with 'client socket is closed'.

Configure libvirtd to stop sending daemon-side keepalive probes while LISA owns the host, disable client-side keepalive probes on opened connections, and reconnect/retry focused domain operations when a stale client socket is detected.
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

@vyadavmsft

Copy link
Copy Markdown
Collaborator Author

@copilot fix check errors

Copilot AI review requested due to automatic review settings June 16, 2026 17:41
@vyadavmsft
vyadavmsft removed the request for review from Copilot June 16, 2026 17:41

Copilot AI commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@copilot fix check errors

Fixed in commit d6a7427. The flake8 CI was failing with C901: '_delete_node' is too complex (16). Refactored by extracting _destroy_domain and _undefine_domain helper methods, which also restores log.warning for non-keepalive errors as noted in the review comments.

@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI review requested due to automatic review settings June 16, 2026 18:26
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
Release devices back to the libvirt passthrough pool without duplicating entries, then clear the node context so later deployments do not emit stale hostdev XML.

This prevents follow-on device-passthrough tests from failing with 'Hostdev already exists in the domain configuration' after a previous deployment has released its devices.
Copilot AI review requested due to automatic review settings June 20, 2026 23:56
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Extract the closed-connection retry flow into a shared helper and use it for domain define, lookup, destroy, and undefine operations.

Add explicit domain assertions and keep cleanup retry exceptions logged through the standard exception variable.
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread lisa/sut_orchestrator/libvirt/platform.py Outdated
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI review requested due to automatic review settings June 21, 2026 00:18
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread lisa/sut_orchestrator/libvirt/platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py
Route domain.interfaceAddresses through the shared libvirt reconnect helper so IP discovery can recover if the client socket closes after domain lookup.
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Cloud Hypervisor passthrough deployments can lose the libvirt SSH transport while attaching the serial console. In the failing run, domain.openConsole() returned a virt-ssh-helper EOF/connection-reset error, which left the shared libvirt connection stale. Later deployments then failed during defineXML() with 'client socket is closed'.

Treat the virt-ssh-helper EOF/reset messages as reconnectable libvirt connection errors. Wrap CH domain start plus console attach in the existing reconnect helper so the retry reopens libvirt, looks up the already-defined domain, starts it only if needed, and attaches the console again.

Also clean up partially-created console streams and log files when openConsole() fails, so retries do not inherit a half-initialized console logger.
Copilot AI review requested due to automatic review settings June 22, 2026 17:52
@github-actions

Copy link
Copy Markdown

🤖 AI Test Selection

No test cases were selected for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread lisa/sut_orchestrator/libvirt/ch_platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py
Comment thread lisa/sut_orchestrator/libvirt/platform.py
@vyadavmsft
vyadavmsft merged commit 57353fb into main Jun 24, 2026
62 checks passed
@vyadavmsft
vyadavmsft deleted the vyadav_hune_fix5 branch June 24, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants