cleanup: VirtioNetworking refactoring to be more portable #13783

benhillis · 2025-11-25T00:29:54Z

This pull request introduces a new GuestDeviceManager class to centralize and synchronize guest device operations, refactoring ViritoNetworking to use this new manager and removing legacy callback-based device management. The changes improve encapsulation and thread safety for device operations, and simplify the interface for adding and modifying guest devices within the VM.

As a follow-up change I am looking for ways to push additional guest device logic (virtiofs, virtio-9p, plan9) into this class as well.

Copilot

Pull request overview

This pull request refactors guest device management by introducing a new GuestDeviceManager class to centralize and synchronize guest device operations. The change removes callback-based device management from VirtioNetworking, replacing it with direct method calls through the new manager. This improves encapsulation, thread safety, and simplifies the interface for adding and modifying guest devices.

Key changes:

Introduces GuestDeviceManager class with centralized lock management and device operations
Refactors VirtioNetworking to use the new manager instead of callbacks
Moves CreateComServerAsUser helper template functions to wsl::windows::common::wslutil namespace for reusability

Reviewed changes

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

Show a summary per file

File	Description
`src/windows/service/exe/GuestDeviceManager.h`	New header defining `GuestDeviceManager` class with lock management and device operations
`src/windows/service/exe/GuestDeviceManager.cpp`	Implementation of `GuestDeviceManager` methods with proper lock handling and device management
`src/windows/service/exe/WslCoreVm.h`	Replaces `m_guestDeviceLock` with `m_guestDeviceManager`, removes old device management methods, updates lock annotations
`src/windows/service/exe/WslCoreVm.cpp`	Removes callback handlers, updates all device operations to use `m_guestDeviceManager`, removes duplicate helper methods
`src/windows/service/exe/VirtioNetworking.h`	Removes callback typedefs, updates constructor to accept `GuestDeviceManager` and user token directly
`src/windows/service/exe/VirtioNetworking.cpp`	Implements `ModifyOpenPorts` method directly, replaces callback usage with manager methods
`src/windows/common/wslutil.h`	Adds `CreateComServerAsUser` template functions for creating COM servers with user impersonation
`src/windows/service/exe/CMakeLists.txt`	Adds new `GuestDeviceManager` source and header files
`src/windows/common/CMakeLists.txt`	Minor reordering of `HandleConsoleProgressBar.h` in headers list

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

src/windows/service/exe/GuestDeviceManager.h

src/windows/service/exe/WslCoreVm.h

Copilot

Pull request overview

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

src/windows/service/exe/WslCoreVm.cpp

src/windows/service/exe/GuestDeviceManager.h

src/windows/common/CMakeLists.txt

src/windows/service/exe/VirtioNetworking.cpp

src/windows/service/exe/GuestDeviceManager.cpp

Copilot

Pull request overview

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

src/windows/common/CMakeLists.txt

src/windows/service/exe/GuestDeviceManager.h

Copilot · 2025-11-25T20:56:03Z

src/windows/service/exe/GuestDeviceManager.h

+    _Requires_lock_not_held_(m_lock)
+    GUID AddGuestDevice(
+        _In_ const GUID& DeviceId,
+        _In_ const GUID& ImplementationClsid,
+        _In_ PCWSTR AccessName,
+        _In_opt_ PCWSTR Options,
+        _In_ PCWSTR Path,
+        _In_ UINT32 Flags,
+        _In_ HANDLE UserToken);


The public method AddGuestDevice lacks documentation explaining its parameters and purpose. Consider adding documentation that describes:

What each parameter represents (especially the distinction between DeviceId and ImplementationClsid)

The purpose of the AccessName, Options, Path, and Flags parameters

What the returned GUID represents

Any exceptions that might be thrown

Thread safety guarantees (the _Requires_lock_not_held_ annotation is present but users need to understand this is thread-safe)

src/windows/service/exe/GuestDeviceManager.h

src/windows/service/exe/VirtioNetworking.cpp

src/windows/service/exe/WslCoreVm.h

src/windows/service/exe/GuestDeviceManager.cpp

* wslsettings: allow OOBE window to close with escape key (#13686) * wslsettings: allow OOBE windows to close with escape key * fix memory leak --------- Co-authored-by: Ben Hillis <[email protected]> * Localization change from build: 133610492 (#13704) Co-authored-by: WSL localization <[email protected]> * wslsettings: fix OOBE text truncation at 200% text scaling (#13693) * wslsettings: fix OOBE text truncation at 200% text scaling Add text scaling factor to window resize calculation and make hero image height responsive to text scaling. Increase minimum window size for better accessibility. Fix MAS 1.4.4 compliance for OOBE dialog. * pr feedback --------- Co-authored-by: Ben Hillis <[email protected]> * wslsettings: add underlines to links in about page (#13703) Co-authored-by: Ben Hillis <[email protected]> * .clang-format: add InsertBraces: true and minor fix to FormatSource.ps1 (#13712) Co-authored-by: Ben Hillis <[email protected]> * Localization change from build: 134015316 (#13731) Co-authored-by: WSL localization <[email protected]> * wslsettings: ensure selected setting is auto-expanded and selected (#13689) * wslsettings: ensure selected setting is auto-selected Implement keyboard focus management for SettingsExpander controls across settings pages. This resolves an accessibility issue reported internally. * add asserts --------- Co-authored-by: Ben Hillis <[email protected]> * build: fix minor compiler errors when building with VS2026 (#13744) * build: fix minor compiler errors when building with VS2026 * s * use VS2022 for clang format and cross compiling --------- Co-authored-by: Ben Hillis <[email protected]> * chore(distributions): Almalinux auto-update - 20251119 12:04:35 (#13743) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Mask systemd-networkd-wait-online.service during boot (#13611) * deps: update a number of NuGet packages to the latest available versions (#13728) Co-authored-by: Ben Hillis <[email protected]> * Notice change from build: 134267142 (#13751) Co-authored-by: WSL notice <[email protected]> * Add *.slnx to .gitignore (#13754) * Fix service crash when collecting a linux crash dump when maxCrashDumpCount is set to 0 (#13755) * Fix service crash when collecting a linux crash dump when maxCrashDumpCount is set to 0 * Move the check inside the function * cleanup: VirtioNetworking refactoring (#13760) * cleanup: update VirtioNetworking class to not rely on the WslCoreConfig struct * cleanup: simplify VirtioNetworking construction * remove old constructor and other cleanup * more minor cleanup * string cleanup in HandleVirtioModifyOpenPorts --------- Co-authored-by: Ben Hillis <[email protected]> * cleanup: switch from Microsoft::WRL::ComPtr to wil::com_ptr (#13767) * cleanup: switch from Microsoft::WRL::ComPtr to wil::com_ptr * reformat --------- Co-authored-by: Ben Hillis <[email protected]> * chore(distributions): Almalinux auto-update - 20251124 17:13:02 (#13780) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Notice change from build: 134527933 (#13782) Co-authored-by: WSL notice <[email protected]> * cleanup: VirtioNetworking refactoring to be more portable (#13783) * cleanup: VirtioNetworking refactoring to be more portable * more refactoring * make m_guestDeviceManager private --------- Co-authored-by: Ben Hillis <[email protected]> --------- Co-authored-by: Ben Hillis <[email protected]> Co-authored-by: Blue <[email protected]> Co-authored-by: WSL localization <[email protected]> Co-authored-by: AlmaLinux Autobot <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

benhillis requested a review from OneBlue November 25, 2025 00:29

benhillis requested a review from a team as a code owner November 25, 2025 00:29

benhillis requested review from Copilot, craigloewen-msft and justus-camp-microsoft November 25, 2025 00:29

Copilot started reviewing on behalf of benhillis November 25, 2025 00:30 View session

Copilot finished reviewing on behalf of benhillis November 25, 2025 00:34

Copilot AI reviewed Nov 25, 2025

View reviewed changes

src/windows/service/exe/GuestDeviceManager.h Outdated Show resolved Hide resolved

src/windows/service/exe/WslCoreVm.h Outdated Show resolved Hide resolved

OneBlue approved these changes Nov 25, 2025

View reviewed changes

cleanup: VirtioNetworking refactoring to be more portable

bfc921c

benhillis force-pushed the user/benhill/guest_device_manager_on_master branch from ac41146 to bfc921c Compare November 25, 2025 19:04

benhillis requested a review from Copilot November 25, 2025 19:04

Copilot started reviewing on behalf of benhillis November 25, 2025 19:05 View session

Copilot finished reviewing on behalf of benhillis November 25, 2025 19:08

Copilot AI reviewed Nov 25, 2025

View reviewed changes

benhillis requested a review from Brian-Perkins November 25, 2025 19:37

more refactoring

e2109c1

benhillis requested a review from Copilot November 25, 2025 20:51

Copilot started reviewing on behalf of benhillis November 25, 2025 20:51 View session

Copilot finished reviewing on behalf of benhillis November 25, 2025 20:55

Copilot AI reviewed Nov 25, 2025

View reviewed changes

Brian-Perkins reviewed Nov 25, 2025

View reviewed changes

src/windows/service/exe/WslCoreVm.h Outdated Show resolved Hide resolved

Brian-Perkins approved these changes Nov 25, 2025

View reviewed changes

make m_guestDeviceManager private

f46be35

Brian-Perkins approved these changes Nov 26, 2025

View reviewed changes

OneBlue reviewed Nov 26, 2025

View reviewed changes

src/windows/service/exe/GuestDeviceManager.cpp Show resolved Hide resolved

OneBlue approved these changes Nov 26, 2025

View reviewed changes

benhillis merged commit 4207cc8 into master Nov 26, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cleanup: VirtioNetworking refactoring to be more portable #13783

cleanup: VirtioNetworking refactoring to be more portable #13783

Uh oh!

benhillis commented Nov 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cleanup: VirtioNetworking refactoring to be more portable #13783

cleanup: VirtioNetworking refactoring to be more portable #13783

Uh oh!

Conversation

benhillis commented Nov 25, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants