Skip to content

Add VHD mounts as named volumes#14362

Merged
kvega005 merged 62 commits intomicrosoft:feature/wsl-for-appsfrom
kvega005:feature/wsl-for-apps-named-volumes-merge-20260305-115914
Mar 23, 2026
Merged

Add VHD mounts as named volumes#14362
kvega005 merged 62 commits intomicrosoft:feature/wsl-for-appsfrom
kvega005:feature/wsl-for-apps-named-volumes-merge-20260305-115914

Conversation

@kvega005
Copy link
Copy Markdown
Contributor

@kvega005 kvega005 commented Mar 5, 2026

Summary of the Pull Request

This PR adds named volume support to WSLA containers and wires it through session, container create/open, metadata persistence, and tests.

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

  • Added session-level named volume support:
    • CreateVolume and DeleteVolume in session API/implementation.
    • Volumes are tracked by name in session state and mounted into the guest at /mnt/wsla-volumes/{name}.
    • DeleteVolume blocks when any container still references the named volume.
  • Added container create support for named volumes:
    • New container option field for named volume mounts (Name, ContainerPath, ReadOnly).
    • Named volume mounts resolve via session volume map and are injected as bind mounts using the guest mount path.
  • Added duplicate mount-target validation:
    • Duplicate container target paths are rejected with ERROR_ALREADY_EXISTS.
    • Validation applies across both regular bind volumes and named volumes.
    • Reusing the same named volume multiple times is allowed when container target paths are different.
  • Added persistence/recovery metadata for named volumes:
    • Named volume mount metadata is serialized with container metadata and reconstructed on Open.

Validation Steps Performed

  • Added NamedVolumesTest:
  • Covers duplicate named volume creation failure
  • Named volumes mount in guest
  • Same named volume mounted multiple times to different container paths is allowed
  • Duplicate container target path is rejected (named+named and bind+named)
  • In-use delete rejection and successful delete after container removal

@kvega005 kvega005 requested a review from a team as a code owner March 5, 2026 20:39
Copilot AI review requested due to automatic review settings March 5, 2026 20:39
@kvega005 kvega005 marked this pull request as draft March 5, 2026 20:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds session-scoped named volume support backed by VHDs, integrates named volume mounts into container create/open flows, persists mount metadata, and introduces Windows tests verifying the new behavior.

Changes:

  • Introduces CreateVolume / DeleteVolume on IWSLASession and session state tracking for named volumes
  • Implements VHD-backed volume creation (VHD create → attach → ext4 format → mount in guest) and container mount injection via bind mounts
  • Extends container metadata serialization to persist named volume mounts and adds an end-to-end NamedVolumesTest

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test/windows/WSLATests.cpp Adds a new integration test covering named volume creation, mount behavior, validation, and delete semantics
src/windows/wslasession/WSLAVirtualMachine.h Exposes Ext4Format on the VM abstraction
src/windows/wslasession/WSLAVirtualMachine.cpp Implements Ext4Format using mkfs.ext4 inside the guest
src/windows/wslasession/WSLAVhdVolume.h Adds internal VHD volume implementation class definition
src/windows/wslasession/WSLAVhdVolume.cpp Implements VHD-backed named volume create/mount/delete logic
src/windows/wslasession/WSLASession.h Wires volume management into the session interface and adds a m_volumes map
src/windows/wslasession/WSLASession.cpp Implements CreateVolume / DeleteVolume and passes session volumes into container creation
src/windows/wslasession/WSLAContainerMetadata.h Adds named volume mounts to persisted container metadata
src/windows/wslasession/WSLAContainer.h Tracks referenced named volumes per container and extends Create signature
src/windows/wslasession/WSLAContainer.cpp Validates duplicate mount targets across bind/named volumes and injects named mounts into Docker create request
src/windows/wslasession/CMakeLists.txt Adds new VHD volume sources/headers to the build
src/windows/wslaservice/inc/wslaservice.idl Adds IDL structs for volume options and named volume mounts; extends session interface; adds new HRESULTs
src/windows/common/WSLAContainerLauncher.h Adds named volume mount support to the test/container launcher
src/windows/common/WSLAContainerLauncher.cpp Implements named volume mount wiring into WSLA_CONTAINER_OPTIONS

Comment thread src/windows/wslasession/WSLAVhdVolume.cpp
Comment thread src/windows/wslasession/WSLAVhdVolume.cpp Outdated
Comment thread src/windows/wslasession/WSLAVhdVolume.cpp
Comment thread src/windows/wslasession/WSLAVhdVolume.cpp Outdated
Comment thread src/windows/wslasession/WSLAVhdVolume.cpp Outdated
Comment thread src/windows/wslaservice/inc/wslaservice.idl Outdated
Comment thread src/windows/wslasession/WSLAContainerMetadata.h Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 18 out of 18 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/windows/wslasession/WSLAContainer.cpp:1084

  • The null-check for volume.ContainerPath is duplicated: it’s checked with THROW_HR_IF_MSG(... volume.ContainerPath == nullptr ...) and then immediately again with THROW_HR_IF_NULL_MSG(... volume.ContainerPath ...). Dropping one of these would reduce redundancy without changing behavior.
        THROW_HR_IF_MSG(E_INVALIDARG, volume.ContainerPath == nullptr, "Volume at index %lu has null ContainerPath", i);
        THROW_HR_IF_MSG(
            HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS),
            !containerMountTargets.insert(volume.ContainerPath).second,
            "Duplicate container mount path '%hs'",
            volume.ContainerPath);

        THROW_HR_IF_NULL_MSG(E_INVALIDARG, volume.HostPath, "Volumes[%lu].HostPath is null", i);
        THROW_HR_IF_NULL_MSG(E_INVALIDARG, volume.ContainerPath, "Volumes[%lu].ContainerPath is null", i);

Comment thread src/windows/wslasession/WSLAContainer.cpp
Comment thread src/windows/wslasession/WSLAVhdVolume.cpp
Comment thread src/windows/inc/docker_schema.h
Comment thread src/windows/wslasession/WSLASession.cpp
Comment thread src/windows/wslasession/WSLASession.cpp Outdated
Comment thread src/windows/wslasession/WSLAContainer.cpp
Comment thread src/windows/wslasession/WSLASession.cpp Outdated
Copilot AI review requested due to automatic review settings March 13, 2026 15:27
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 18 out of 18 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/windows/wslasession/WSLASession.cpp:1

  • The new user-facing error text is now the generic “Invalid name”، which is ambiguous in APIs that operate on different entities (container name vs volume name). If this is intended to cover both, consider including the entity type in the message (e.g., “Invalid container name” / “Invalid volume name”) or adding separate localized strings to keep errors actionable.
/*++

Comment thread src/windows/wslasession/WSLAContainer.cpp Outdated
Comment thread src/windows/wslasession/WSLAVhdVolume.cpp
Comment thread localization/strings/en-US/Resources.resw Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 13, 2026 15:36
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 18 out of 18 changed files in this pull request and generated 3 comments.

Comment thread src/windows/inc/docker_schema.h
Comment thread src/windows/inc/docker_schema.h
Comment thread src/windows/wslasession/WSLASession.cpp
Copilot AI review requested due to automatic review settings March 13, 2026 15:49
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 18 out of 18 changed files in this pull request and generated 1 comment.

Comment thread src/windows/wslasession/WSLASession.cpp
kvega005 and others added 2 commits March 13, 2026 11:11
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 13, 2026 18:11
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 18 out of 18 changed files in this pull request and generated 2 comments.

Comment thread src/windows/wslasession/WSLAContainer.cpp Outdated
Comment thread src/windows/wslasession/WSLASession.cpp
Copy link
Copy Markdown
Collaborator

@OneBlue OneBlue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor comments

Comment thread src/windows/service/inc/wslaservice.idl
Comment thread src/windows/wslasession/WSLAContainer.cpp Outdated
Comment thread src/windows/wslasession/WSLASession.cpp Outdated
const auto it = options.find("SizeBytes");
THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslaInvalidVolumeOptions(Options.Options), it == options.end());

auto sizeBytes = wsl::shared::FromJson<ULONGLONG>(it->second.c_str());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I didn't know we could even do that !

Copilot AI review requested due to automatic review settings March 19, 2026 17:36
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 18 out of 18 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/windows/wslasession/WSLASession.cpp:1

  • ValidateName is now used for both container names and volume names, but it still enforces WSLA_MAX_CONTAINER_NAME_LENGTH. If the intent is a shared constraint, consider renaming the constant (or introducing a separate WSLA_MAX_NAME_LENGTH) to avoid container-specific semantics leaking into a generic validator.
/*++

Comment thread src/windows/wslasession/WSLASession.cpp
Comment thread src/windows/wslasession/WSLASession.cpp
Comment thread src/windows/wslasession/WSLAVolumeMetadata.h
Comment thread src/windows/wslasession/WSLASession.cpp Outdated
@kvega005 kvega005 merged commit 237b92c into microsoft:feature/wsl-for-apps Mar 23, 2026
6 checks passed
@kvega005 kvega005 deleted the feature/wsl-for-apps-named-volumes-merge-20260305-115914 branch March 23, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants