-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add defaultVhdType configuration setting to .wslconfig #13553
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
base: master
Are you sure you want to change the base?
Add defaultVhdType configuration setting to .wslconfig #13553
Conversation
This commit implements support for the `defaultVhdType` configuration setting
in .wslconfig and improves the handling of VHD size and type defaults when
creating new WSL 2 distributions.
Changes:
1. Added `defaultVhdType` configuration setting to .wslconfig
- New config option accepts "dynamic" or "fixed" values
- Defaults to "dynamic" for backward compatibility
- Documented in wsl-config.md
2. Improved command-line argument handling for VHD options
- `--fixed-vhd` can now be used without `--vhd-size` when `defaultVhdSize`
is configured in .wslconfig
- When neither `--vhd-size` nor `--fixed-vhd` is specified, the VHD type
is determined by the `defaultVhdType` setting in .wslconfig
- Removed the strict validation that required `--vhd-size` with `--fixed-vhd`
3. Updated VHD creation logic in LxssUserSession::RegisterDistribution
- Now applies `defaultVhdSize` from .wslconfig when VhdSize is 0
- Determines VHD type based on explicit `--fixed-vhd` flag OR the
`defaultVhdType` configuration setting
- Ensures backward compatibility: explicit flags override config defaults
4. Extended configuration interfaces
- Added VhdType enum and configuration structures
- Updated WslCoreConfigInterface.h and WslCoreConfigInterface.cpp
- Added C# bindings for VhdType in LibWsl.cs
- Added telemetry tracking for VhdDefaultType
5. Added comprehensive test coverage
- New test method `DefaultVhdTypeConfiguration` validates:
* Installation with `--fixed-vhd` uses defaultVhdSize from config
* defaultVhdType=fixed creates Fixed VHDs
* defaultVhdType=dynamic creates Dynamic VHDs
* Proper VHD type verification via PowerShell Get-VHD
This implementation addresses issue microsoft#13529 and allows users to configure
their preferred VHD type and size in .wslconfig, simplifying distribution
installation commands:
Before:
wsl --install Ubuntu-24.04 --vhd-size 10GB --fixed-vhd
After (with .wslconfig configured):
wsl --install Ubuntu-24.04 --fixed-vhd
# or even simpler:
wsl --install Ubuntu-24.04
Configuration example in %USERPROFILE%\.wslconfig:
[wsl2]
defaultVhdSize=10GB
defaultVhdType=fixed
Fixes microsoft#13529
Signed-off-by: Giovanni Magliocchetti <[email protected]>
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 implements a new defaultVhdType configuration setting for .wslconfig files and improves the handling of VHD size and type defaults when creating WSL 2 distributions. The change allows users to configure their preferred VHD type (dynamic vs. fixed) in .wslconfig and makes the --vhd-size parameter optional when defaults are configured.
Key changes include:
- Added
VhdTypeenum anddefaultVhdTypeconfiguration support across the codebase - Relaxed command-line validation to allow
--fixed-vhdwithout explicit--vhd-sizewhen defaults exist - Enhanced VHD creation logic to apply both size and type defaults from configuration
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/windows/UnitTests.cpp | Added comprehensive test for VHD type configuration validation |
| src/windows/wslsettings/LibWsl.cs | Updated C# bindings with renumbered enums and new VhdType enum |
| src/windows/service/exe/LxssUserSession.cpp | Modified VHD creation to apply config defaults and determine type |
| src/windows/libwsl/WslCoreConfigInterface.cpp | Implemented Get/Set handlers for new VhdType configuration |
| src/windows/inc/WslCoreConfigInterface.h | Added VhdTypeConfiguration enum and struct member |
| src/windows/common/WslCoreConfig.h | Added VhdType enum, string mappings, and config struct field |
| src/windows/common/WslCoreConfig.cpp | Added config file parsing support for defaultVhdType |
| src/windows/common/WslClient.cpp | Removed strict validation requiring --vhd-size with --fixed-vhd |
Comments suppressed due to low confidence (1)
src/windows/libwsl/WslCoreConfigInterface.cpp:1
- Missing
break;statement after the VhdSizeBytes case. This will cause fall-through to the VhdType case, leading to incorrect execution.
/*++
Signed-off-by: Giovanni Magliocchetti <[email protected]>
test/windows/UnitTests.cpp
Outdated
| // Install a distribution using --fixed-vhd without --vhd-size | ||
| // This should use the defaultVhdSize from config | ||
| auto [out, err] = LxsstuLaunchWslAndCaptureOutput( | ||
| std::format(L"--install Ubuntu-24.04 --name {} --no-launch --fixed-vhd", testDistroName)); |
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.
We should avoid to depend on networking in the tests, since that can create unstable test results.
Instead of downloading ubuntu, let' use the test distro tar. Something like this should do:
std::format(L"--install --from-file \"{}\" --no-launch --name test_vhd_flag --fixed-vhd", g_testDistroPath));
OneBlue
left a comment
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.
Thank you for doing this ! Added a couple comments
| WslConfigChange config(configContent); | ||
|
|
||
| // Clean up any existing test distro | ||
| LxsstuLaunchWsl(std::format(L"--unregister {}", testDistroName)); |
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.
Let's use a different name for the distribution we use for this test so we don't have to unregister the main one
test/windows/UnitTests.cpp
Outdated
| VERIFY_ARE_NOT_EQUAL(out.find(L"completed successfully"), std::wstring::npos); | ||
|
|
||
| // Get the VHD path for the test distribution | ||
| auto distroList = LxsstuEnumerateDistributions(); |
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.
nit: it might be worth extracting this logic into a lambda. Something like:
auto validateVhdType = [](const char* Expected){...};
So the test can just do
validateVhdType("Fixed");
| NoEntry, | ||
| ProcessorCount, | ||
| MemorySizeBytes, | ||
| SwapSizeBytes, |
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.
wslsettings.exe doesn't actually display this setting, so let's remove the changes in libwsl and wslsettings
Implements support for a new defaultVhdType setting in .wslconfig that allows users to configure whether new WSL 2 distributions use dynamic or fixed VHDs by default. Additionally, improves the command-line experience by allowing the --fixed-vhd flag to work without requiring an explicit --vhd-size parameter when defaultVhdSize is configured in .wslconfig. Changes: - Added VhdType enum and VhdDefaultType field to WslCoreConfig - Added parsing for defaultVhdType setting in .wslconfig - Updated RegisterDistribution to respect defaultVhdType configuration - Removed strict validation requiring --vhd-size with --fixed-vhd - Added comprehensive unit test for defaultVhdType functionality - Updated documentation in wsl-config.md Fixes microsoft#13529 Signed-off-by: Giovanni Magliocchetti <[email protected]>
Summary
This PR implements a new
defaultVhdTypeconfiguration setting for .wslconfig and improves how VHD size and type defaults are applied when creating new WSL 2 distributions. This addresses the feature request in issue #13529.Problem Statement
Currently, when creating a WSL 2 distribution with a fixed-size VHD, users must always specify both
--vhd-sizeand--fixed-vhd, even if they have already configured adefaultVhdSizein their .wslconfig file:Additionally, there was no way to configure the default VHD type (dynamic vs. fixed) in .wslconfig, requiring users to always specify
--fixed-vhdexplicitly for every distribution they wanted to create with a fixed VHD.Solution
This PR adds:
New
defaultVhdTypeconfiguration setting - Users can now specify their preferred VHD type in .wslconfig:Improved command-line logic - The
--vhd-sizeparameter is now optional whendefaultVhdSizeis configured in .wslconfig, and the VHD type is determined by:--fixed-vhdflag (highest priority)defaultVhdTypeconfiguration setting (if no explicit flag)Simplified installation commands:
Changes Made
Configuration System
VhdTypeenum withDynamicandFixedvalues toWslCoreConfig.hVhdDefaultTypefield to theConfigstruct (defaults toDynamic)wsl2.defaultVhdTypein config file parserVhdDefaultTypeusageCommand-Line Handling
--vhd-sizewith--fixed-vhdVHD Creation Logic
LxssUserSession::RegisterDistributionto applydefaultVhdSizewhen not explicitly providedAPI Interfaces
WslCoreConfigInterface.hwithVhdTypeconfig entry andVhdTypeConfigurationenumWslCoreConfigInterface.cppLibWsl.cswith renumbered enum values and newVhdTypeenumTesting
DefaultVhdTypeConfigurationthat validates:--fixed-vhdusesdefaultVhdSizefrom configdefaultVhdType=fixedcreates Fixed VHDsdefaultVhdType=dynamiccreates Dynamic VHDsGet-VHDDocumentation
wsl-config.mdwith newdefaultVhdTypesetting documentationTesting
The implementation has been tested with:
Backward Compatibility
✅ This PR maintains full backward compatibility:
defaultVhdTypeis not configured (defaults todynamic)--vhd-sizecan still be specified explicitly and takes precedenceRelated Issue
Fixes #13529
Example Usage
Configuration in
%USERPROFILE%\.wslconfig:Installation commands:
Checklist