-
Notifications
You must be signed in to change notification settings - Fork 1.6k
cleanup: VirtioNetworking refactoring #13760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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 refactors the VirtioNetworking initialization in WslCoreVm by replacing the previous chained setter pattern with a constructor that takes explicit callback parameters. The refactoring extracts inline lambda implementations into dedicated handler methods (HandleVirtioAddGuestDevice and HandleVirtioModifyOpenPorts) within WslCoreVm, improving code organization and readability.
Key changes:
- Removed chained setter methods (
OnAddGuestDevice,OnModifyOpenPorts,OnGuestInterfaceStateChanged) fromVirtioNetworking - Added a new constructor that accepts callbacks directly as parameters
- Extracted callback logic into
HandleVirtioAddGuestDeviceandHandleVirtioModifyOpenPortsmethods inWslCoreVm
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/windows/service/exe/WslCoreVm.h | Added declarations for two new handler methods that encapsulate virtio networking callback logic |
| src/windows/service/exe/WslCoreVm.cpp | Refactored VirtioNetworking initialization to use constructor with callbacks, extracted handler methods, and relocated IsVhdAttached method |
| src/windows/service/exe/VirtioNetworking.h | Replaced chained setter methods with constructor parameters for callbacks, removed m_config reference in favor of m_enableLocalhostRelay |
| src/windows/service/exe/VirtioNetworking.cpp | Removed setter methods, added constructor with callback parameters, and removed callback validation check |
There was a problem hiding this comment.
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 no new comments.
* 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: 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]>
* 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>
This pull request refactors how
VirtioNetworkingis constructed and integrated withWslCoreVm, simplifying its initialization and improving code clarity. The main changes involve replacing the previous configuration object with explicit parameters and callbacks, removing chained setter methods, and moving callback logic into dedicated handler methods withinWslCoreVm.