Refactor the session creation logic to match the new API #13814

OneBlue · 2025-12-02T02:37:17Z

Summary of the Pull Request

** This change depends on: #13791 **

This change refactors the session creation logic to:

Make the storage disk creation easier
Get rid of the old VM settings
Run the init script from within the service
Simplify testing

After this change, running wsl --wsla will open a shell in a VM already configured to run nerdctl. --storage can be passed to enable on disk storage, otherwise a tmpfs will be used to allow container creation.

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

Validation Steps Performed

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…oft/WSL into user/ptrivedi/create-cont

…ptrivedi/create-cont

…crosoft/WSL into user/oneblue/create-container

Copilot

Pull request overview

This pull request refactors the WSLA (Windows Subsystem for Linux API) session creation logic to simplify storage disk management, remove deprecated VM settings, integrate init script execution into the service, and introduce container support via nerdctl.

Key Changes:

Consolidated session settings by removing VIRTUAL_MACHINE_SETTINGS structure and moving relevant fields into WSLA_SESSION_SETTINGS
Added container creation capabilities through new WSLAContainer and WSLAContainerLauncher classes
Fixed critical inverted state validation logic in VM operations (AttachDisk, Fork, Shutdown, Signal)
Introduced storage VHD management with automatic creation and formatting
Switched test environment from full Linux distributions to busybox-based minimal rootfs

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
src/windows/wslaservice/inc/wslaservice.idl	Removed VIRTUAL_MACHINE_SETTINGS; consolidated into WSLA_SESSION_SETTINGS; added WSLAFeatureFlags enum; changed WSLA_PROCESS_OPTIONS.Executable to [unique]; added OpenSessionByName method
test/windows/WSLATests.cpp	Updated all tests to use new session settings API; changed shell from /bin/bash to /bin/sh for busybox compatibility; added CreateContainer test; created GetDefaultSessionSettings() helper
src/windows/wslaservice/exe/WSLAVirtualMachine.h	Introduced Settings struct to replace VIRTUAL_MACHINE_SETTINGS; removed GetDebugShellPipe method; added Mount public method
src/windows/wslaservice/exe/WSLAVirtualMachine.cpp	Fixed inverted m_running validation bugs; removed debug shell pipe support; moved DmesgHandle from ULONG to wil::unique_handle; added FeatureEnabled helper
src/windows/wslaservice/exe/WSLAUserSession.h/cpp	Updated CreateSession to accept only WSLA_SESSION_SETTINGS; added OpenSessionByName implementation
src/windows/wslaservice/exe/WSLASession.h/cpp	Refactored to create VM settings from session settings; added ConfigureStorage for automatic VHD creation/formatting; integrated init script execution
src/windows/wslaservice/exe/WSLAProcess.cpp	Added m_exitEvent.SetEvent() call in OnVmTerminated for proper cleanup
src/windows/wslaservice/exe/WSLAContainer.h/cpp	Implemented container creation via nerdctl with support for basic options (image, name, entrypoint, environment, stdin/tty handling)
src/windows/common/WSLAContainerLauncher.h/cpp	New client-side wrapper for launching containers through the WSLA API
src/windows/common/WSLAProcessLauncher.h/cpp	Added DEFAULT_MOVABLE macro usage for move semantics support
src/windows/common/WslCoreFilesystem.cpp	Changed VHD extension validation from WI_ASSERT to THROW_HR_IF for production safety
src/windows/common/WslClient.cpp	Updated WslaShell debug tool to use new session settings; added container launch support
src/shared/inc/defs.h	Added DEFAULT_MOVABLE macro for default move constructor/assignment
CMakeLists.txt	Fixed CMake conditional from DEFINED check to boolean check
msipackage/package.wix.in	Added wslarootfs.vhd to installer for x64 builds

Copilot · 2025-12-02T02:44:28Z

src/windows/wslaservice/exe/WSLASession.cpp

+        THROW_HR_IF(E_INVALIDARG, Settings.RootVhdTypeOverride == nullptr);
+
+        vmSettings.RootVhd = Settings.RootVhdOverride;
+        vmSettings.RootVhdType = Settings.RootVhdTypeOverride;


[nitpick] The error condition checks if RootVhdTypeOverride is null when RootVhdOverride is not null. However, there's an asymmetry: when RootVhdOverride is null (line 81-83), a default VHD type of "squashfs" is used. Consider also providing a sensible default for RootVhdTypeOverride (e.g., "ext4") instead of requiring it to be specified, or document why it must be explicitly provided.

Suggested change

THROW_HR_IF(E_INVALIDARG, Settings.RootVhdTypeOverride == nullptr);

vmSettings.RootVhd = Settings.RootVhdOverride;

vmSettings.RootVhdType = Settings.RootVhdTypeOverride;

vmSettings.RootVhd = Settings.RootVhdOverride;

vmSettings.RootVhdType = Settings.RootVhdTypeOverride != nullptr ? Settings.RootVhdTypeOverride : "ext4";

Copilot · 2025-12-02T02:44:29Z

test/windows/WSLATests.cpp


        // Note: fd/0 is opened by readlink to read the actual content of /proc/self/fd.
-        if (!PathMatchSpecA(result.Output[1].c_str(), "/proc/self/fd/0 /proc/self/fd/1 /proc/self/fd/2\nsocket:[*]\nsocket:[*]\n"))
+        if (!PathMatchSpecA(result.Output[1].c_str(), "/proc/self/fd/0 /proc/self/fd/1 /proc/self/fd/2\n"))


The test expectation changed to no longer expect socket file descriptors in the output (removed socket:[*]\nsocket:[*]\n). This suggests a behavioral change where processes no longer have extra socket file descriptors open. Verify this is intentional and doesn't indicate a regression in socket handling or file descriptor management.

Copilot · 2025-12-02T02:44:29Z

CMakeLists.txt

 endif()

-if (NOT DEFINED OFFICIAL_BUILD AND ${TARGET_PLATFORM} STREQUAL "x64")
+if (NOT OFFICIAL_BUILD AND ${TARGET_PLATFORM} STREQUAL "x64")


The condition should use && (logical AND) instead of AND (which is not standard C++). While some compilers may define AND as a macro or alternative token, it's better to use the standard && operator for portability and clarity.

Copilot · 2025-12-02T02:44:29Z

src/windows/wslaservice/inc/wslaservice.idl

 };

 struct WSLA_PROCESS_OPTIONS
 {


The [unique] attribute on Executable was added, but this could be a breaking API change. If existing callers pass a non-null Executable, this change should be safe. However, if the intention is to allow null executables (as suggested by the container launcher code), ensure all consumers handle this correctly. The container code at line 142-146 in WSLAContainer.cpp explicitly handles null executables, which is good.

Suggested change

{

{

// May be NULL. Consumers must handle the case where Executable is not provided.

Copilot · 2025-12-02T02:44:29Z

src/windows/wslaservice/inc/wslaservice.idl

    LPCSTR Image;
    LPCSTR Name;
-    struct WSLA_PROCESS_OPTIONS* InitProcessOptions;
+    struct WSLA_PROCESS_OPTIONS InitProcessOptions;


The field changed from a pointer (struct WSLA_PROCESS_OPTIONS*) to a value type (struct WSLA_PROCESS_OPTIONS). This is a breaking API change. Ensure all callers have been updated to pass the structure by value instead of by pointer.

Copilot · 2025-12-02T02:44:32Z

src/windows/wslaservice/exe/WSLAContainer.cpp

+        THROW_HR_IF_MSG(
+            E_INVALIDARG,
+            options.InitProcessOptions.Environment[i][0] == L'-',
+            "Invlaid environment string: %hs",


Typo in error message: "Invlaid" should be "Invalid".

Suggested change

"Invlaid environment string: %hs",

"Invalid environment string: %hs",

Copilot · 2025-12-02T02:44:32Z

src/windows/wslaservice/exe/WSLAContainer.cpp

+        THROW_HR_IF_MSG(
+            E_INVALIDARG,
+            options.InitProcessOptions.Environment[i][0] == L'-',
+            "Invlaid environment string: %hs",


The environment validation checks if the string starts with - but uses the wrong error message format specifier. The parameter options.InitProcessOptions.Environment[i] is declared as LPCSTR* (array of char pointers), so individual elements are LPCSTR (char*). However, the error message uses %hs which expects a char*, but this should work. The real issue is the error message says "Invlaid" (typo already noted) and "environment string" when it's checking for a dash prefix, which typically indicates an argument flag confusion. Consider clarifying the error message to explain why a dash prefix is invalid (e.g., "Environment variable must not start with '-'").

Suggested change

"Invlaid environment string: %hs",

"Invalid environment variable: %hs. Environment variable must not start with '-'",

Copilot · 2025-12-02T02:44:32Z

src/windows/common/WSLAContainerLauncher.h

+    This file contains the definition for WSLAContainerLauncher.
+
+--*/
+#include "WSLAprocessLauncher.h"


Inconsistent casing in include: should be "WSLAProcessLauncher.h" (capital P) to match the actual filename based on other includes in the codebase.

Suggested change

#include "WSLAprocessLauncher.h"

#include "WSLAProcessLauncher.h"

Copilot · 2025-12-02T02:44:33Z

test/windows/WSLATests.cpp

+        if (result.Code != expectedResult)
+        {
+            LogError(
+                "Comman didn't return expected code (%i). ExitCode: %i, Stdout: '%hs', Stderr: '%hs'",


Typo in error message: "Comman" should be "Command".

Suggested change

"Comman didn't return expected code (%i). ExitCode: %i, Stdout: '%hs', Stderr: '%hs'",

"Command didn't return expected code (%i). ExitCode: %i, Stdout: '%hs', Stderr: '%hs'",

Copilot · 2025-12-02T02:44:33Z

src/windows/wslaservice/exe/WSLASession.cpp

+                "Failed to attach vhd: %ls",
+                m_storageVhdPath.c_str());
+
+            // If the VHD wasn'found, create it.


Typo in comment: "wasn'found" should be "wasn't found".

Suggested change

// If the VHD wasn'found, create it.

// If the VHD wasn't found, create it.

benhillis · 2025-12-03T04:08:44Z

src/windows/wslaservice/exe/WSLASession.cpp

+        {
+            THROW_HR_IF_MSG(
+                result,
+                result != HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND) && result != HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),


why are we ignoring these failures?

I see, still seems odd to try to attach a disk that doesn't exist.

I thought about that vs explicitly checking if the file exists, my thinking was that they're equivalent, but in the case where the VHD does exists, this approach allows us to avoid a file access to validate that the VHD exists

src/windows/wslaservice/exe/WSLASession.cpp

benhillis · 2025-12-03T04:12:00Z

src/windows/wslaservice/exe/WSLAVirtualMachine.cpp

+    if (m_settings.DmesgHandle)
    {
-        dmesgOutput.reset(wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(ULongToHandle(m_settings.DmesgOutput)));
+        dmesgOutput = std::move(m_settings.DmesgHandle);


can this whole condition be removed and just always move the dmesg handle? won't harm anythigng if it's empty.

Good point, it can

benhillis · 2025-12-03T04:12:32Z

src/windows/wslaservice/exe/WSLAVirtualMachine.cpp

    WSL_LOG("CreateWSLAVirtualMachine", TraceLoggingValue(json.c_str(), "json"));

    m_computeSystem = hcs::CreateComputeSystem(m_vmIdString.c_str(), json.c_str());
+    m_running = true;


Running is a poor name since the VM has not started yet.

Does it belong after StartComputeSystem?

(200+ line function, phew :) )

Indeed. Moved after StartComputeSystem

src/windows/common/WslClient.cpp

src/windows/wslaservice/exe/WSLASession.cpp

src/windows/wslaservice/exe/WSLAVirtualMachine.cpp

ptrivedi · 2025-12-03T15:52:30Z

src/windows/wslaservice/exe/WSLAVirtualMachine.cpp

    WSL_LOG("CreateWSLAVirtualMachine", TraceLoggingValue(json.c_str(), "json"));

    m_computeSystem = hcs::CreateComputeSystem(m_vmIdString.c_str(), json.c_str());
+    m_running = true;


Does it belong after StartComputeSystem?

(200+ line function, phew :) )

…pdate

Pooja Trivedi and others added 28 commits November 21, 2025 14:31

Implement CreateContainer method

1220fbe

Build fixes

0920c52

Implement CreateContainer method

3d5fa8b

Update src/windows/wslaservice/exe/WSLAContainer.cpp

3a8a669

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix nerdctl host networking parameter

b8c5b8c

Prototype CreateContainer()

cff1cd2

Format

29971ac

Merge branch 'user/ptrivedi/create-cont' of https://github.com/micros…

9c18826

…oft/WSL into user/ptrivedi/create-cont

Format

cdf7c46

Start writing tests

3b5c028

Fix various issues

2a4f8d0

Add new files

9dd34f2

Test cleanup

292707e

Test cleanup

0eda07f

Merge remote-tracking branch 'origin/feature/wsl-for-apps' into user/…

e4822ba

…ptrivedi/create-cont

Prepare for PR

987b1b7

Fix ARM build

a4ed4b2

Add copyright header

ee948d5

Update cmakelists.txt

08d5929

Use ifdefs

2c28e53

Format

52aa7f8

ifdef ARM64

9c5ed51

Install the test .vhd in the MSI

8d9e9cb

Merge branch 'feature/wsl-for-apps' into user/oneblue/create-container

e786943

Update the stdin test

0760be6

Format

70ac182

Merge branch 'user/oneblue/create-container' of https://github.com/mi…

dcc0f5f

…crosoft/WSL into user/oneblue/create-container

Refactor the session creation logic to match the new WSLA API

b7ff00b

OneBlue requested a review from a team as a code owner December 2, 2025 02:37

Copilot AI review requested due to automatic review settings December 2, 2025 02:37

OneBlue requested review from Brian-Perkins, benhillis, craigloewen-msft and justus-camp-microsoft December 2, 2025 02:37

OneBlue changed the base branch from master to feature/wsl-for-apps December 2, 2025 02:37

OneBlue requested a review from a team as a code owner December 2, 2025 02:37

Copilot started reviewing on behalf of OneBlue December 2, 2025 02:37 View session

Copilot finished reviewing on behalf of OneBlue December 2, 2025 02:42

Copilot AI reviewed Dec 2, 2025

View reviewed changes

OneBlue changed the title ~~User/oneblue/container api update~~ Refactor the session creation logic to match the new API Dec 2, 2025

OneBlue added 2 commits December 2, 2025 15:04

Merge

251a5bb

Prepare for review

7284d8e

benhillis reviewed Dec 3, 2025

View reviewed changes

src/windows/wslaservice/exe/WSLASession.cpp Outdated Show resolved Hide resolved

benhillis reviewed Dec 3, 2025

View reviewed changes

ptrivedi reviewed Dec 3, 2025

View reviewed changes

OneBlue added 2 commits December 3, 2025 12:05

Apply PR feedback

f8e2119

Use early return

c69c83b

ptrivedi approved these changes Dec 3, 2025

View reviewed changes

Merge branch 'feature/wsl-for-apps' into user/oneblue/container-api-u…

4d34427

…pdate

OneBlue enabled auto-merge (squash) December 4, 2025 01:38

OneBlue merged commit 844acd0 into feature/wsl-for-apps Dec 4, 2025
6 checks passed

	{
	{
	// May be NULL. Consumers must handle the case where Executable is not provided.

	"Invlaid environment string: %hs",
	"Invalid environment string: %hs",

	"Invlaid environment string: %hs",
	"Invalid environment variable: %hs. Environment variable must not start with '-'",

	#include "WSLAprocessLauncher.h"
	#include "WSLAProcessLauncher.h"

	"Comman didn't return expected code (%i). ExitCode: %i, Stdout: '%hs', Stderr: '%hs'",
	"Command didn't return expected code (%i). ExitCode: %i, Stdout: '%hs', Stderr: '%hs'",

	// If the VHD wasn'found, create it.
	// If the VHD wasn't found, create it.

Refactor the session creation logic to match the new API #13814

Refactor the session creation logic to match the new API #13814

Uh oh!

Conversation

OneBlue commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

OneBlue commented Dec 2, 2025 •

edited

Loading