Skip to content

Handle Linux failures from GNS callbacks - #41396

Open
Shawn Yuan (shuaiyuanxx) wants to merge 4 commits into
masterfrom
user/shawn/fix-gns-linux-result
Open

Handle Linux failures from GNS callbacks#41396
Shawn Yuan (shuaiyuanxx) wants to merge 4 commits into
masterfrom
user/shawn/fix-gns-linux-result

Conversation

@shuaiyuanxx

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Propagates nonzero Linux GNS results as callback failures instead of treating a successful transport as a successful network configuration operation.

The change preserves transport failures and the special ConnectTest result semantics, and adds regression tests for all result combinations.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

GNS callbacks provide two independent results:

  • a transport HRESULT, indicating whether communication with Linux succeeded;
  • a Linux result code, indicating whether Linux successfully applied the requested network configuration.

Previously, the callback returned only the transport HRESULT. If communication succeeded but Linux failed to apply an address,
route, interface, or DNS update, the operation was reported as successful. This could cause Windows to mark the configuration as
synchronized and stop retrying it.

This change combines the two results for ordinary GNS requests:

  • transport failures retain their original HRESULT;
  • a successful transport with a zero Linux result remains successful;
  • a successful transport with a nonzero Linux result returns a failure;
  • ConnectTest remains exempt because its nonzero result is connectivity status data, not an operation failure.

The result handling is implemented in a shared helper so production code and regression tests use the same logic.

Validation Steps Performed

@shuaiyuanxx
Shawn Yuan (shuaiyuanxx) requested a review from a team as a code owner August 20, 2026 09:20
Copilot AI lite review requested due to automatic review settings August 20, 2026 09:20

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 updates mirrored networking’s GNS callback handling so that a successful transport HRESULT is no longer treated as a successful operation when Linux reports a nonzero result code, aligning Windows-side success/failure with actual guest configuration outcomes.

Changes:

  • Add a shared helper (GetGnsCallbackResult) to combine the transport HRESULT and Linux result code with a ConnectTest exception.
  • Update the mirrored networking GNS callback to use the helper’s combined result.
  • Add unit/regression tests covering key transport/Linux result combinations, including ConnectTest.

Reviewed changes

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

File Description
src/windows/common/WslCoreNetworkingSupport.h Introduces the shared helper for determining the effective callback HRESULT from transport + Linux result.
src/windows/service/exe/MirroredNetworking.cpp Uses the shared helper to return failure when Linux reports nonzero for non-ConnectTest messages.
test/windows/NetworkTests.cpp Adds tests validating the helper’s semantics across transport/Linux result combinations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/windows/service/exe/MirroredNetworking.cpp Outdated
Comment thread src/windows/common/WslCoreNetworkingSupport.h Outdated
Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 20, 2026 09:46

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 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/windows/service/exe/MirroredNetworking.cpp Outdated
Comment thread test/windows/NetworkTests.cpp
Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 20, 2026 10:32

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 3 out of 3 changed files in this pull request and generated no new comments.

@OneBlue Blue (OneBlue) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This a change behavior from the previous implementation, which doesn't retry when an failed HRESULT is returned.

I'm not sure that it makes sense to retry in that case. If the socket is closed for instance, retrying for 3 seconds would only delay a potential shutdown.

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.

3 participants