This repository was archived by the owner on Jun 5, 2025. It is now read-only.
Add error message for when the virtual machine platform feature isn't enabled for WSL creation #3410
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… on the machine. Fix resource string for exception in creation operation and remove mentions of adaptive cards in error message
bbonaby
requested review from
AmelBawa-msft,
SohamDas2021,
dhoehna,
huzaifa-d,
jsidewhite,
krschau,
safmswork,
sshilov7 and
vineeththomasalex
krschau
reviewed
Jul 11, 2024
dhoehna
reviewed
Jul 11, 2024
| { | ||
| if (!string.IsNullOrEmpty(_wslEnablementError)) | ||
| { | ||
| _log.Error($"Virtual machine platform optional component not enabled. A reboot is needed."); |
Contributor
There was a problem hiding this comment.
This might be getting too specific. Is it worth separating out the logs into these two states.
- Virtual Machine Platform optional component is not enabled
- Virtual Machine component is enabled but the computer need to restart?
dhoehna
reviewed
Jul 11, 2024
| var searcher = new ManagementObjectSearcher($"SELECT InstallState FROM Win32_OptionalFeature WHERE Name = 'VirtualMachinePlatform'"); | ||
| var collection = searcher.Get(); | ||
|
|
||
| foreach (var instance in collection) |
Contributor
There was a problem hiding this comment.
NIT: collection.Where(x => x != null)
dhoehna
reviewed
Jul 11, 2024
dhoehna
approved these changes
Jul 11, 2024
AmelBawa-msft
approved these changes
Jul 12, 2024
| /// </summary> | ||
| public ComputeSystemAdaptiveCardResult CreateAdaptiveCardSessionForDeveloperId(IDeveloperId developerId, ComputeSystemAdaptiveCardKind sessionKind) | ||
| { | ||
| if (!string.IsNullOrEmpty(_wslEnablementError)) |
Contributor
There was a problem hiding this comment.
I am assuming we are checking for the error message here to save on calling an expensive operation (IsVirtualMachinePlatformFeatureEnabled) every time? If yes, consider making it Lazy so it gets called once (thread safe), that way the method is more readable.
| var searcher = new ManagementObjectSearcher($"SELECT InstallState FROM Win32_OptionalFeature WHERE Name = 'VirtualMachinePlatform'"); | ||
| var collection = searcher.Get(); | ||
|
|
||
| foreach (var instance in collection) |
Contributor
There was a problem hiding this comment.
This foreach will always return on first iteration, is this right? Consider a FirstOrDefault() if we are interested in the first item only.
| <data name="WSLInstallationFailedWithException" xml:space="preserve"> | ||
| <value>Unable to install and register {0} due to error: {1}. Try using {wsl.exe} to install it manually. See {aka.ms/wslinstall}</value> | ||
| <value>Unable to install and register {0} due to error: {1}. Try using wsl.exe to install it manually. See aka.ms/wslinstall</value> | ||
| <comment>{Locked="{0}", "{1}", "{wsl.exe}", {aka.ms/wslinstall}} Text message to display when we failed to install a wsl distribution. {0} is the name of the distribution and {1} is the error message.</comment> |
Contributor
There was a problem hiding this comment.
Missing change in the comment Locked list for {wsl.exe}
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the pull request
Virtual Machine PlatformWindows feature is not enabled. This is needed when attempting to install a WSL distribution for the first time. Relates to: Add reboot messaging/notifications to Dev Home for WSL installation #3400 . Whenwsl.exe installis ran for the first time it will install theVirtual Machine Platformfeature, update wsl.exe and install the selected distribution. However, when this happens even though the distribution is installed it isn't registered untilVirtual Machine Platformis enabled and the computer is rebooted. Unfortunately there is no way for us to know when both have occurred as they happen in separate processes outside of Dev Homes control, so Dev Home will just show "waiting for 's installation to complete" even though a reboot is needed.So to fix this, we will show an error and lead the user to enable the
Virtual Machine Platformfeature from the Windows Customization page and reboot, before allowing them to go through the WSL environment creation flow. This should prevent a confusing experience.Video showing what will happen now that we show the error:
wsl.install.UI.error.mp4
Also removed the verbiage surrounding adaptive cards in the error and loading progress bar in the create environment page.
References and relevant issues
I noticed that I had to use WMI to check for feature enablement again here. I'm thinking we can add a common service for WMI usage through Dev Home. See: #3381
Detailed description of the pull request / Additional comments
Validation steps performed
PR checklist