Rework the process & container API #14117
Conversation
| WSLAContainerFlagsNone = 0, | ||
| WSLAContainerFlagsRm = 1, | ||
| WSLAContainerFlagsGpu = 2, | ||
| WSLAContainerFlagsAttach = 4, |
There was a problem hiding this comment.
Rm controls the AutoDelete behavior, Gpu enables GPU support in the container, and attach creates an initial attached relay (needed for non-detached scenarios, otherwise output can be lost between the start() and attach() calls)
There was a problem hiding this comment.
Shouldn't the Attach option be something specified on Start? Seems odd to put it on Create.
There was a problem hiding this comment.
It could also be on start() yeah, having it here has the benefit of having all flags in one place, but I'm OK with either
There was a problem hiding this comment.
IMO Start is better, if there's no technical reason we can't
There was a problem hiding this comment.
Ok. I'll remove this flag from this enum for now. I'll do the Start() changes in a future change
| pollfd pollDescriptors[3]; | ||
|
|
||
| pollDescriptors[0].fd = Message.TtyInput; | ||
| pollDescriptors[0].fd = Message.Socket; |
There was a problem hiding this comment.
Switching to using the same socket for tty input & input to match the container & exec processes. It also simplifies things a bit
| Info.cbSize = sizeof(Info); | ||
| THROW_IF_WIN32_BOOL_FALSE(::GetConsoleScreenBufferInfoEx(Stdout, &Info)); | ||
|
|
||
| if (containerImage.empty()) |
There was a problem hiding this comment.
Removing support for this since wsladiag can do run now
…oneblue/api-refactor-3
There was a problem hiding this comment.
Pull request overview
This pull request refactors the process and container creation APIs to simplify their usage and add support for several new features. The main changes include:
Changes:
- Replaced the complex file descriptor API (WSLA_PROCESS_FD) with simpler flag-based configuration (WSLAProcessFlags)
- Added support for custom stop signals, working directories, hostnames, domain names, and user specifications
- Expanded the signal enumeration to include all standard Linux signals
- Introduced WSLAStringArray for cleaner array parameter handling
- Updated the IWSLAContainer::Stop API to accept signal and timeout parameters
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/windows/wslaservice/inc/wslaservice.idl | Removed old FD types, added WSLAProcessFlags, WSLAStringArray, expanded WSLASignal enum, updated container and process options structures |
| test/windows/WSLATests.cpp | Updated all tests to use new API, removed OpenFiles test, added tests for new features (stop signals, working directory, hostname, username) |
| src/windows/wslaservice/exe/WSLAVirtualMachine.* | Refactored process creation to use new flag-based FD system, simplified TTY handling |
| src/windows/wslaservice/exe/WSLAContainer.* | Updated container creation/execution to use new API structures, added support for hostname, domainname, user, stop signal |
| src/windows/wslaservice/exe/WSLASession.cpp | Updated CreateRootNamespaceProcess signature to accept Executable parameter |
| src/windows/wslaservice/exe/WSLAProcessIO.* | Simplified OpenFd to remove unused flags parameter |
| src/windows/wslaservice/exe/DockerHTTPClient.* | Updated StopContainer to handle optional signal and timeout parameters |
| src/windows/wslc/main.cpp | Updated to use WSLAProcessFlags instead of manual FD configuration |
| src/windows/wsladiag/wsladiag.cpp | Adapted to new process launcher API |
| src/windows/common/WSLAProcessLauncher.* | Major refactor to use flags instead of FD lists, added SetWorkingDirectory and SetUser methods |
| src/windows/common/WSLAContainerLauncher.* | Added setter methods for entrypoint, stop signal, container flags, hostname, domainname |
| src/windows/common/WslClient.cpp | Simplified process launching code, removed container-related debug code |
| src/windows/inc/docker_schema.h | Added User, Hostname, Domainname, StopSignal, WorkingDir fields to container schema |
| src/shared/inc/lxinitshared.h | Simplified WSLA_TTY_RELAY structure to use single Socket field |
| src/linux/init/WSLAInit.cpp | Updated TTY relay to use unified socket field |
Summary of the Pull Request
The objective of this change is to simplify the APIs for process & container creation.
As part of that, this change adds support and test coverage for:
This change also removes support for custom FDs (including opening files as fds) since it's unused
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed