From cc18d03616d9a07e111159cb5f772943dd45cca6 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 23 Apr 2026 15:56:19 -0700 Subject: [PATCH 01/11] Rename ContainerEventTracker to DockerEventTracker --- .../{ContainerEventTracker.cpp => DockerEventTracker.cpp} | 0 .../wslcsession/{ContainerEventTracker.h => DockerEventTracker.h} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/windows/wslcsession/{ContainerEventTracker.cpp => DockerEventTracker.cpp} (100%) rename src/windows/wslcsession/{ContainerEventTracker.h => DockerEventTracker.h} (100%) diff --git a/src/windows/wslcsession/ContainerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp similarity index 100% rename from src/windows/wslcsession/ContainerEventTracker.cpp rename to src/windows/wslcsession/DockerEventTracker.cpp diff --git a/src/windows/wslcsession/ContainerEventTracker.h b/src/windows/wslcsession/DockerEventTracker.h similarity index 100% rename from src/windows/wslcsession/ContainerEventTracker.h rename to src/windows/wslcsession/DockerEventTracker.h From efa122b5cc0801fe95a7a849c263e0aa14e729a3 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 23 Apr 2026 15:57:27 -0700 Subject: [PATCH 02/11] Add guest volume driver, Docker event tracker with volume sync, and WSLCVolumes manager --- localization/strings/en-US/Resources.resw | 4 + src/windows/inc/docker_schema.h | 3 +- src/windows/wslcsession/CMakeLists.txt | 9 +- .../wslcsession/DockerEventTracker.cpp | 170 ++++++--- src/windows/wslcsession/DockerEventTracker.h | 67 ++-- src/windows/wslcsession/DockerHTTPClient.cpp | 5 + src/windows/wslcsession/DockerHTTPClient.h | 1 + src/windows/wslcsession/IWSLCVolume.h | 50 +++ src/windows/wslcsession/WSLCContainer.cpp | 86 +++-- src/windows/wslcsession/WSLCContainer.h | 20 +- src/windows/wslcsession/WSLCGuestVolume.cpp | 141 ++++++++ src/windows/wslcsession/WSLCGuestVolume.h | 79 ++++ .../wslcsession/WSLCProcessControl.cpp | 4 +- src/windows/wslcsession/WSLCProcessControl.h | 10 +- src/windows/wslcsession/WSLCSession.cpp | 130 ++----- src/windows/wslcsession/WSLCSession.h | 23 +- src/windows/wslcsession/WSLCVhdVolume.h | 18 +- src/windows/wslcsession/WSLCVolumeMetadata.h | 3 + src/windows/wslcsession/WSLCVolumes.cpp | 187 ++++++++++ src/windows/wslcsession/WSLCVolumes.h | 57 +++ test/windows/WSLCTests.cpp | 341 +++++++++++++----- 21 files changed, 1100 insertions(+), 308 deletions(-) create mode 100644 src/windows/wslcsession/IWSLCVolume.h create mode 100644 src/windows/wslcsession/WSLCGuestVolume.cpp create mode 100644 src/windows/wslcsession/WSLCGuestVolume.h create mode 100644 src/windows/wslcsession/WSLCVolumes.cpp create mode 100644 src/windows/wslcsession/WSLCVolumes.h diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index f0b0e1059..1b7619037 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2132,6 +2132,10 @@ For privacy information about this product please visit https://aka.ms/privacy.< Unsupported volume type: '{}' {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + unsupported volume driver options: {} + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + Volume '{}' is in use. {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/inc/docker_schema.h b/src/windows/inc/docker_schema.h index 7cb7cd318..78fdd7de8 100644 --- a/src/windows/inc/docker_schema.h +++ b/src/windows/inc/docker_schema.h @@ -273,8 +273,9 @@ struct InspectContainer ContainerInspectState State; ContainerConfig Config; HostConfig HostConfig; + std::vector Mounts; - NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(InspectContainer, Id, Name, Created, Image, State, Config, HostConfig); + NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(InspectContainer, Id, Name, Created, Image, State, Config, HostConfig, Mounts); }; struct InspectExec diff --git a/src/windows/wslcsession/CMakeLists.txt b/src/windows/wslcsession/CMakeLists.txt index 15a034bad..f277e3052 100644 --- a/src/windows/wslcsession/CMakeLists.txt +++ b/src/windows/wslcsession/CMakeLists.txt @@ -19,16 +19,18 @@ set(SOURCES # Volume management WSLCVhdVolume.cpp + WSLCGuestVolume.cpp + WSLCVolumes.cpp # Supporting classes - ContainerEventTracker.cpp + DockerEventTracker.cpp DockerHTTPClient.cpp IORelay.cpp ServiceProcessLauncher.cpp ) set(HEADERS - ContainerEventTracker.h + DockerEventTracker.h DockerHTTPClient.h IORelay.h ServiceProcessLauncher.h @@ -42,6 +44,9 @@ set(HEADERS WSLCSessionReference.h WSLCVirtualMachine.h WSLCVhdVolume.h + WSLCGuestVolume.h + WSLCVolumes.h + IWSLCVolume.h WSLCVolumeMetadata.h) add_executable(wslcsession WIN32 ${SOURCES} ${HEADERS}) diff --git a/src/windows/wslcsession/DockerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp index d84ba7b6f..02df0b27b 100644 --- a/src/windows/wslcsession/DockerEventTracker.cpp +++ b/src/windows/wslcsession/DockerEventTracker.cpp @@ -4,30 +4,30 @@ Copyright (c) Microsoft. All rights reserved. Module Name: - ContainerEventTracker.cpp + DockerEventTracker.cpp Abstract: - Contains the implementation of ContainerEventTracker. + Contains the implementation of DockerEventTracker. --*/ #include "precomp.h" -#include "ContainerEventTracker.h" +#include "DockerEventTracker.h" #include "WSLCVirtualMachine.h" #include using wsl::windows::common::relay::MultiHandleWait; -using wsl::windows::service::wslc::ContainerEventTracker; +using wsl::windows::service::wslc::DockerEventTracker; using wsl::windows::service::wslc::DockerHTTPClient; using wsl::windows::service::wslc::WSLCVirtualMachine; -ContainerEventTracker::ContainerTrackingReference::ContainerTrackingReference(ContainerEventTracker* tracker, size_t id) noexcept : +DockerEventTracker::EventTrackingReference::EventTrackingReference(DockerEventTracker* tracker, size_t id) noexcept : m_tracker(tracker), m_id(id) { } -ContainerEventTracker::ContainerTrackingReference& ContainerEventTracker::ContainerTrackingReference::operator=( - ContainerEventTracker::ContainerTrackingReference&& other) noexcept +DockerEventTracker::EventTrackingReference& DockerEventTracker::EventTrackingReference::operator=( + DockerEventTracker::EventTrackingReference&& other) noexcept { Reset(); m_id = other.m_id; @@ -39,29 +39,29 @@ ContainerEventTracker::ContainerTrackingReference& ContainerEventTracker::Contai return *this; } -void ContainerEventTracker::ContainerTrackingReference::Reset() noexcept +void DockerEventTracker::EventTrackingReference::Reset() noexcept { if (m_tracker != nullptr) { - m_tracker->UnregisterContainerStateUpdates(m_id); + m_tracker->UnregisterCallback(m_id); m_tracker = nullptr; m_id = {}; } } -ContainerEventTracker::ContainerTrackingReference::ContainerTrackingReference(ContainerTrackingReference&& other) noexcept : +DockerEventTracker::EventTrackingReference::EventTrackingReference(EventTrackingReference&& other) noexcept : m_id(other.m_id), m_tracker(other.m_tracker) { other.m_tracker = nullptr; other.m_id = {}; } -ContainerEventTracker::ContainerTrackingReference::~ContainerTrackingReference() noexcept +DockerEventTracker::EventTrackingReference::~EventTrackingReference() noexcept { Reset(); } -ContainerEventTracker::ContainerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay) : +DockerEventTracker::DockerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay) : m_sessionId(sessionId) { auto onChunk = [this](const gsl::span& buffer) { @@ -87,13 +87,14 @@ ContainerEventTracker::ContainerEventTracker(DockerHTTPClient& dockerClient, ULO relay.AddHandle(std::make_unique(std::move(socket), std::move(onChunk))); } -ContainerEventTracker::~ContainerEventTracker() +DockerEventTracker::~DockerEventTracker() { // N.B. No callback should be left when the tracker is destroyed. - WI_ASSERT(m_callbacks.empty()); + WI_ASSERT(m_containerCallbacks.empty()); + WI_ASSERT(m_volumeCallbacks.empty()); } -void ContainerEventTracker::OnEvent(const std::string_view& event) +void DockerEventTracker::OnEvent(const std::string_view& event) { WSL_LOG( "DockerEvent", @@ -101,32 +102,65 @@ void ContainerEventTracker::OnEvent(const std::string_view& event) event.data(), static_cast(std::min(event.size(), static_cast(USHRT_MAX))), "Data"), TraceLoggingValue(m_sessionId, "SessionId")); - static std::map events{ - {"start", ContainerEvent::Start}, {"die", ContainerEvent::Stop}, {"exec_die", ContainerEvent::ExecDied}}; - auto parsed = nlohmann::json::parse(event); auto action = parsed.find("Action"); - auto actor = parsed.find("Actor"); - THROW_HR_IF_MSG( E_INVALIDARG, - action == parsed.end() || actor == parsed.end(), + action == parsed.end(), "Failed to parse json: %.*hs", static_cast(event.size()), event.data()); - auto it = events.find(action->get()); - if (it == events.end()) + auto timeEntry = parsed.find("time"); + THROW_HR_IF_MSG( + E_INVALIDARG, timeEntry == parsed.end(), "Failed to parse time from event: %.*hs", static_cast(event.size()), event.data()); + std::uint64_t eventTime = timeEntry->get(); + + auto actionStr = action->get(); + + // Route events by Type field. Docker uses "container", "volume", "network", etc. + auto type = parsed.find("Type"); + std::string typeStr = (type != parsed.end()) ? type->get() : "container"; + + if (typeStr == "container") { - return; // Event is not tracked, dropped. + OnContainerEvent(parsed, actionStr, eventTime); } + else if (typeStr == "volume") + { + OnVolumeEvent(parsed, actionStr, eventTime); + } +} + +void DockerEventTracker::OnContainerEvent(const nlohmann::json& parsed, const std::string& action, std::uint64_t eventTime) +{ + static std::map events{ + {"start", ContainerEvent::Start}, {"die", ContainerEvent::Stop}, {"exec_die", ContainerEvent::ExecDied}}; + + auto actor = parsed.find("Actor"); + THROW_HR_IF_MSG(E_INVALIDARG, actor == parsed.end(), "Missing Actor in container event"); auto id = actor->find("ID"); - THROW_HR_IF_MSG(E_INVALIDARG, id == actor->end(), "Failed to parse json: %.*hs", static_cast(event.size()), event.data()); + THROW_HR_IF_MSG(E_INVALIDARG, id == actor->end(), "Missing Actor.ID in container event"); auto containerId = id->get(); + // Track container create events for WaitForObjectCreated(). + if (action == "create") + { + std::lock_guard lock{m_lock}; + m_createdObjects.insert(containerId); + m_createdObjectCV.notify_all(); + return; + } + + auto it = events.find(action); + if (it == events.end()) + { + return; // Event is not tracked, dropped. + } + std::optional exitCode; std::optional execId; auto attributes = actor->find("Attributes"); @@ -145,14 +179,9 @@ void ContainerEventTracker::OnEvent(const std::string_view& event) } } - auto timeEntry = parsed.find("time"); - THROW_HR_IF_MSG( - E_INVALIDARG, timeEntry == parsed.end(), "Failed to parse time from event: %.*hs", static_cast(event.size()), event.data()); - std::uint64_t eventTime = timeEntry->get(); - std::lock_guard lock{m_lock}; - for (const auto& e : m_callbacks) + for (const auto& e : m_containerCallbacks) { if (e.ContainerId == containerId && (!e.ExecId.has_value() || e.ExecId == execId)) { @@ -161,34 +190,93 @@ void ContainerEventTracker::OnEvent(const std::string_view& event) } } -ContainerEventTracker::ContainerTrackingReference ContainerEventTracker::RegisterContainerStateUpdates( +void DockerEventTracker::OnVolumeEvent(const nlohmann::json& parsed, const std::string& action, std::uint64_t eventTime) +{ + static std::map events{{"create", VolumeEvent::Create}, {"destroy", VolumeEvent::Destroy}}; + + auto it = events.find(action); + if (it == events.end()) + { + return; // Event is not tracked, dropped. + } + + auto actor = parsed.find("Actor"); + THROW_HR_IF_MSG(E_INVALIDARG, actor == parsed.end(), "Missing Actor in volume event"); + + auto id = actor->find("ID"); + THROW_HR_IF_MSG(E_INVALIDARG, id == actor->end(), "Missing Actor.ID in volume event"); + + auto volumeName = id->get(); + + std::lock_guard lock{m_lock}; + + for (const auto& e : m_volumeCallbacks) + { + e.Callback(volumeName, it->second, eventTime); + } +} + +void DockerEventTracker::WaitForObjectCreated(const std::string& ObjectId) +{ + constexpr auto c_timeout = std::chrono::seconds{60}; + + std::unique_lock lock{m_lock}; + THROW_HR_IF_MSG( + HRESULT_FROM_WIN32(ERROR_TIMEOUT), + !m_createdObjectCV.wait_for(lock, c_timeout, [&]() { return m_createdObjects.contains(ObjectId); }), + "Timed out waiting for Docker create event for object '%hs'", + ObjectId.c_str()); + + m_createdObjects.erase(ObjectId); +} + +DockerEventTracker::EventTrackingReference DockerEventTracker::RegisterContainerStateUpdates( const std::string& ContainerId, ContainerStateChangeCallback&& Callback) noexcept { std::lock_guard lock{m_lock}; auto id = m_callbackId++; - m_callbacks.emplace_back(id, ContainerId, std::optional{}, std::move(Callback)); + m_containerCallbacks.emplace_back(id, ContainerId, std::optional{}, std::move(Callback)); - return ContainerTrackingReference{this, id}; + return EventTrackingReference{this, id}; } -ContainerEventTracker::ContainerTrackingReference ContainerEventTracker::RegisterExecStateUpdates( +DockerEventTracker::EventTrackingReference DockerEventTracker::RegisterExecStateUpdates( const std::string& ContainerId, const std::string& ExecId, ContainerStateChangeCallback&& Callback) noexcept { std::lock_guard lock{m_lock}; auto id = m_callbackId++; - m_callbacks.emplace_back(id, ContainerId, ExecId, std::move(Callback)); + m_containerCallbacks.emplace_back(id, ContainerId, ExecId, std::move(Callback)); - return ContainerTrackingReference{this, id}; + return EventTrackingReference{this, id}; } -void ContainerEventTracker::UnregisterContainerStateUpdates(size_t Id) noexcept +DockerEventTracker::EventTrackingReference DockerEventTracker::RegisterVolumeUpdates(VolumeEventCallback&& Callback) noexcept { std::lock_guard lock{m_lock}; - auto remove = std::ranges::remove_if(m_callbacks, [Id](auto& entry) { return entry.CallbackId == Id; }); - WI_ASSERT(remove.size() == 1); + auto id = m_callbackId++; + m_volumeCallbacks.emplace_back(id, std::move(Callback)); + + return EventTrackingReference{this, id}; +} + +void DockerEventTracker::UnregisterCallback(size_t Id) noexcept +{ + std::lock_guard lock{m_lock}; + + // Try container callbacks first. + auto containerRemove = std::ranges::remove_if(m_containerCallbacks, [Id](auto& entry) { return entry.CallbackId == Id; }); + if (!containerRemove.empty()) + { + WI_ASSERT(containerRemove.size() == 1); + m_containerCallbacks.erase(containerRemove.begin(), containerRemove.end()); + return; + } - m_callbacks.erase(remove.begin(), remove.end()); + // Then volume callbacks. + auto volumeRemove = std::ranges::remove_if(m_volumeCallbacks, [Id](auto& entry) { return entry.CallbackId == Id; }); + WI_ASSERT(volumeRemove.size() == 1); + m_volumeCallbacks.erase(volumeRemove.begin(), volumeRemove.end()); } \ No newline at end of file diff --git a/src/windows/wslcsession/DockerEventTracker.h b/src/windows/wslcsession/DockerEventTracker.h index b2e05308f..b970ed773 100644 --- a/src/windows/wslcsession/DockerEventTracker.h +++ b/src/windows/wslcsession/DockerEventTracker.h @@ -4,11 +4,11 @@ Copyright (c) Microsoft. All rights reserved. Module Name: - ContainerEventTracker.h + DockerEventTracker.h Abstract: - Contains the definition for ContainerEventTracker. + Contains the definition for DockerEventTracker. --*/ @@ -31,45 +31,59 @@ enum class ContainerEvent ExecDied }; -class ContainerEventTracker +enum class VolumeEvent +{ + Create, + Destroy +}; + +class DockerEventTracker { public: - NON_COPYABLE(ContainerEventTracker); - NON_MOVABLE(ContainerEventTracker); + NON_COPYABLE(DockerEventTracker); + NON_MOVABLE(DockerEventTracker); - struct ContainerTrackingReference + struct EventTrackingReference { - NON_COPYABLE(ContainerTrackingReference); + NON_COPYABLE(EventTrackingReference); - ContainerTrackingReference() = default; - ContainerTrackingReference(ContainerEventTracker* tracker, size_t id) noexcept; - ContainerTrackingReference(ContainerTrackingReference&& other) noexcept; - ~ContainerTrackingReference() noexcept; + EventTrackingReference() = default; + EventTrackingReference(DockerEventTracker* tracker, size_t id) noexcept; + EventTrackingReference(EventTrackingReference&& other) noexcept; + ~EventTrackingReference() noexcept; - ContainerTrackingReference& operator=(ContainerTrackingReference&&) noexcept; + EventTrackingReference& operator=(EventTrackingReference&&) noexcept; void Reset() noexcept; size_t m_id; - ContainerEventTracker* m_tracker = nullptr; + DockerEventTracker* m_tracker = nullptr; }; using ContainerStateChangeCallback = std::function, std::uint64_t)>; + using VolumeEventCallback = std::function; - ContainerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay); - ~ContainerEventTracker(); + DockerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay); + ~DockerEventTracker(); void Stop(); - ContainerTrackingReference RegisterContainerStateUpdates(const std::string& ContainerId, ContainerStateChangeCallback&& Callback) noexcept; - ContainerTrackingReference RegisterExecStateUpdates(const std::string& ContainerId, const std::string& ExecId, ContainerStateChangeCallback&& Callback) noexcept; - void UnregisterContainerStateUpdates(size_t Id) noexcept; + EventTrackingReference RegisterContainerStateUpdates(const std::string& ContainerId, ContainerStateChangeCallback&& Callback) noexcept; + EventTrackingReference RegisterExecStateUpdates(const std::string& ContainerId, const std::string& ExecId, ContainerStateChangeCallback&& Callback) noexcept; + EventTrackingReference RegisterVolumeUpdates(VolumeEventCallback&& Callback) noexcept; + void UnregisterCallback(size_t Id) noexcept; + + // Blocks until the event stream has delivered a create event for the given object ID. + // Because Docker emits events in order, waiting for a container create event guarantees + // that all preceding volume create events have already been processed. + void WaitForObjectCreated(const std::string& ObjectId); private: void OnEvent(const std::string_view& event); - void Run(wil::unique_socket&& Socket); + void OnContainerEvent(const nlohmann::json& parsed, const std::string& action, std::uint64_t eventTime); + void OnVolumeEvent(const nlohmann::json& parsed, const std::string& action, std::uint64_t eventTime); - struct Callback + struct ContainerCallback { size_t CallbackId; std::string ContainerId; @@ -77,7 +91,18 @@ class ContainerEventTracker ContainerStateChangeCallback Callback; }; - std::vector m_callbacks; + struct VolumeCallback + { + size_t CallbackId; + VolumeEventCallback Callback; + }; + + std::vector m_containerCallbacks; + std::vector m_volumeCallbacks; + + // Tracks object IDs that have been seen via create events, used by WaitForObjectCreated(). + std::unordered_set m_createdObjects; + std::condition_variable_any m_createdObjectCV; ULONG m_sessionId{}; std::recursive_mutex m_lock; diff --git a/src/windows/wslcsession/DockerHTTPClient.cpp b/src/windows/wslcsession/DockerHTTPClient.cpp index 35789c59e..5deda7577 100644 --- a/src/windows/wslcsession/DockerHTTPClient.cpp +++ b/src/windows/wslcsession/DockerHTTPClient.cpp @@ -427,6 +427,11 @@ docker_schema::Volume DockerHTTPClient::CreateVolume(const docker_schema::Create return Transaction(verb::post, URL::Create("/volumes/create"), Request); } +docker_schema::Volume DockerHTTPClient::InspectVolume(const std::string& Name) +{ + return Transaction(verb::get, URL::Create("/volumes/{}", Name)); +} + void DockerHTTPClient::RemoveVolume(const std::string& Name) { Transaction(verb::delete_, URL::Create("/volumes/{}", Name)); diff --git a/src/windows/wslcsession/DockerHTTPClient.h b/src/windows/wslcsession/DockerHTTPClient.h index 72d63d715..e8484fc40 100644 --- a/src/windows/wslcsession/DockerHTTPClient.h +++ b/src/windows/wslcsession/DockerHTTPClient.h @@ -145,6 +145,7 @@ class DockerHTTPClient // Volume management. common::docker_schema::Volume CreateVolume(const common::docker_schema::CreateVolume& Request); + common::docker_schema::Volume InspectVolume(const std::string& Name); void RemoveVolume(const std::string& Name); std::vector ListVolumes(); diff --git a/src/windows/wslcsession/IWSLCVolume.h b/src/windows/wslcsession/IWSLCVolume.h new file mode 100644 index 000000000..b49b24ec5 --- /dev/null +++ b/src/windows/wslcsession/IWSLCVolume.h @@ -0,0 +1,50 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + IWSLCVolume.h + +Abstract: + + Abstract interface implemented by all WSLC-managed volume drivers + (currently WSLCVhdVolumeImpl and WSLCGuestVolumeImpl). WSLCSession + stores volumes through this interface so it can hold a single map + regardless of the concrete driver type. + +--*/ + +#pragma once + +#include "wslc.h" +#include + +namespace wsl::windows::service::wslc { + +class IWSLCVolume +{ +public: + virtual ~IWSLCVolume() = default; + + // The docker volume name. + virtual const std::string& Name() const noexcept = 0; + + // The WSLC volume driver, e.g. "vhd" or "guest". This is the driver + // stored in the WSLC volume metadata label, not the underlying docker + // driver (which may be "local" for guest volumes). + virtual const char* Driver() const noexcept = 0; + + // Remove the volume from docker and release any host-side resources + // (e.g. detach/delete the VHD for VHD volumes). Throws on failure. + virtual void Delete() = 0; + + // Returns a JSON string for the COM-facing InspectVolume result. + virtual std::string Inspect() const = 0; + + // Returns the WSLCVolumeInformation struct for the COM-facing + // ListVolumes / CreateVolume results. + virtual WSLCVolumeInformation GetVolumeInformation() const = 0; +}; + +} // namespace wsl::windows::service::wslc diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 1775425b5..3ce3f977c 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -21,6 +21,7 @@ Module Name: #include "WSLCContainer.h" #include "WSLCProcess.h" #include "WSLCProcessIO.h" +#include "WSLCVolumes.h" using wsl::windows::common::COMServiceExecutionContext; using wsl::windows::common::docker_schema::ErrorResponse; @@ -31,6 +32,7 @@ using wsl::windows::common::relay::OverlappedIOHandle; using wsl::windows::common::relay::ReadHandle; using wsl::windows::common::relay::RelayHandle; using wsl::windows::service::wslc::ContainerPortMapping; +using wsl::windows::service::wslc::IWSLCVolume; using wsl::windows::service::wslc::RelayedProcessIO; using wsl::windows::service::wslc::TypedHandle; using wsl::windows::service::wslc::VMPortMapping; @@ -40,9 +42,9 @@ using wsl::windows::service::wslc::WSLCContainerMetadata; using wsl::windows::service::wslc::WSLCContainerMetadataV1; using wsl::windows::service::wslc::WSLCPortMapping; using wsl::windows::service::wslc::WSLCSession; -using wsl::windows::service::wslc::WSLCVhdVolumeImpl; using wsl::windows::service::wslc::WSLCVirtualMachine; using wsl::windows::service::wslc::WSLCVolumeMount; +using wsl::windows::service::wslc::WSLCVolumes; using namespace wsl::windows::common::relay; using namespace wsl::windows::common::docker_schema; @@ -258,10 +260,7 @@ std::string SerializeContainerMetadata(const WSLCContainerMetadataV1& metadata) return wsl::shared::ToJson(wrapper); } -void ProcessNamedVolumes( - const WSLCContainerOptions& containerOptions, - const std::unordered_map>& sessionVolumes, - wsl::windows::common::docker_schema::CreateContainer& request) +void ProcessNamedVolumes(const WSLCContainerOptions& containerOptions, WSLCVolumes& volumes, wsl::windows::common::docker_schema::CreateContainer& request) { THROW_HR_IF(E_INVALIDARG, containerOptions.NamedVolumesCount > 0 && containerOptions.NamedVolumes == nullptr); @@ -274,7 +273,7 @@ void ProcessNamedVolumes( std::string volumeName = nv.Name; THROW_HR_WITH_USER_ERROR_IF( - WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(nv.Name), !sessionVolumes.contains(volumeName)); + WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(nv.Name), !volumes.ContainsVolume(volumeName)); wsl::windows::common::docker_schema::Mount mount{}; mount.Source = std::move(volumeName); @@ -286,21 +285,29 @@ void ProcessNamedVolumes( } } -void ValidateNamedVolumes( - const std::vector& mounts, - const std::unordered_map>& sessionVolumes, - const std::unordered_set& anonymousVolumes) +std::unordered_set GetAnonymousVolumeNames( + const wsl::windows::common::docker_schema::InspectContainer& inspectData) { - for (const auto& mount : mounts) + // Build a set of all volumes mounted on the container. + std::unordered_set volumeNames; + for (const auto& mount : inspectData.Mounts) { if (mount.Type == "volume" && !mount.Name.empty()) { - THROW_HR_WITH_USER_ERROR_IF( - WSLC_E_VOLUME_NOT_FOUND, - Localization::MessageWslcVolumeNotFound(mount.Name), - !sessionVolumes.contains(mount.Name) && !anonymousVolumes.contains(mount.Name)); + volumeNames.insert(mount.Name); } } + + // Remove any volumes that were user specified in the HostConfig.Mounts. + for (const auto& mount : inspectData.HostConfig.Mounts) + { + if (!mount.Source.empty() && mount.Type == "volume") + { + volumeNames.erase(mount.Source); + } + } + + return volumeNames; } } // namespace @@ -359,8 +366,9 @@ WSLCContainerImpl::WSLCContainerImpl( std::vector&& volumes, std::vector&& ports, std::map&& labels, + std::unordered_set&& anonymousVolumes, std::function&& onDeleted, - ContainerEventTracker& EventTracker, + DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, IORelay& Relay, WSLCContainerState InitialState, @@ -374,6 +382,7 @@ WSLCContainerImpl::WSLCContainerImpl( m_networkingMode(NetworkMode), m_id(std::move(Id)), m_mountedVolumes(std::move(volumes)), + m_dockerVolumeNames(std::move(anonymousVolumes)), m_mappedPorts(std::move(ports)), m_labels(std::move(labels)), m_comWrapper(wil::MakeOrThrow(this, std::move(onDeleted))), @@ -784,6 +793,11 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::DeleteExclusiveLo } CATCH_AND_THROW_DOCKER_USER_ERROR("Failed to delete container '%hs'", m_id.c_str()); + if (WI_IsFlagSet(Flags, WSLCDeleteFlagsDeleteVolumes)) + { + m_wslcSession.DeleteContainerVolumes(m_dockerVolumeNames); + } + Transition(WslcContainerStateDeleted); ReleaseResources(); } @@ -1038,9 +1052,9 @@ std::unique_ptr WSLCContainerImpl::Create( const WSLCContainerOptions& containerOptions, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - const std::unordered_map>& sessionVolumes, + WSLCVolumes& volumes, std::function&& OnDeleted, - ContainerEventTracker& EventTracker, + DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, IORelay& IoRelay) { @@ -1137,9 +1151,9 @@ std::unique_ptr WSLCContainerImpl::Create( THROW_HR_IF_NULL_MSG(E_INVALIDARG, containerOptions.Volumes, "Volumes is null with VolumesCount=%lu", containerOptions.VolumesCount); } - // Build volume list from container options. - std::vector volumes; - volumes.reserve(containerOptions.VolumesCount); + // Build bind mount list from container options. + std::vector volumeMounts; + volumeMounts.reserve(containerOptions.VolumesCount); std::vector binds; binds.reserve(containerOptions.VolumesCount); @@ -1188,7 +1202,7 @@ std::unique_ptr WSLCContainerImpl::Create( } } - volumes.push_back(WSLCVolumeMount{hostPath, parentVMPath, volume.ContainerPath, static_cast(volume.ReadOnly), sourceFilename}); + volumeMounts.push_back(WSLCVolumeMount{hostPath, parentVMPath, volume.ContainerPath, static_cast(volume.ReadOnly), sourceFilename}); auto options = volume.ReadOnly ? "ro" : "rw"; auto bindSource = sourceFilename.empty() ? parentVMPath : std::format("{}/{}", parentVMPath, sourceFilename); @@ -1214,7 +1228,7 @@ std::unique_ptr WSLCContainerImpl::Create( } } - ProcessNamedVolumes(containerOptions, sessionVolumes, request); + ProcessNamedVolumes(containerOptions, volumes, request); // Process port mappings from container options. auto [ports, networkMode] = ProcessPortMappings(containerOptions, virtualMachine); @@ -1241,7 +1255,7 @@ std::unique_ptr WSLCContainerImpl::Create( WSLCContainerMetadataV1 metadata; metadata.Flags = containerOptions.Flags; metadata.InitProcessFlags = containerOptions.InitProcessOptions.Flags; - metadata.Volumes = volumes; + metadata.Volumes = volumeMounts; for (const auto& e : ports) { @@ -1264,6 +1278,15 @@ std::unique_ptr WSLCContainerImpl::Create( // Inspect the container to fetch its generated name (if needed) and Docker's authoritative Created timestamp. auto inspectData = DockerClient.InspectContainer(result.Id); + // Wait for the container create event to be delivered on the Docker event stream. + // Docker emits volume create events before the container create event, so once this returns + // we are guaranteed that all anonymous volumes created by Docker have been processed by the + // event tracker's volume callback and are already tracked in WSLCVolumes. + EventTracker.WaitForObjectCreated(result.Id); + + // Identify the anonymous volumes so we know which ones to delete with the container. + auto anonymousVolumes = GetAnonymousVolumeNames(inspectData); + auto container = std::make_unique( wslcSession, virtualMachine, @@ -1271,9 +1294,10 @@ std::unique_ptr WSLCContainerImpl::Create( CleanContainerName(inspectData.Name), std::string(containerOptions.Image), containerOptions.ContainerNetwork.ContainerNetworkType, - std::move(volumes), + std::move(volumeMounts), std::move(ports), std::move(labels), + std::move(anonymousVolumes), std::move(OnDeleted), EventTracker, DockerClient, @@ -1291,18 +1315,15 @@ std::unique_ptr WSLCContainerImpl::Open( const common::docker_schema::ContainerInfo& dockerContainer, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - const std::unordered_map>& sessionVolumes, - const std::unordered_set& anonymousVolumes, + WSLCVolumes& volumes, std::function&& OnDeleted, - ContainerEventTracker& EventTracker, + DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, IORelay& ioRelay) { // Extract container name from Docker's names list. std::string name = ExtractContainerName(dockerContainer.Names, dockerContainer.Id); - ValidateNamedVolumes(dockerContainer.Mounts, sessionVolumes, anonymousVolumes); - auto labels(dockerContainer.Labels); auto metadataIt = labels.find(WSLCContainerMetadataLabel); @@ -1349,6 +1370,7 @@ std::unique_ptr WSLCContainerImpl::Open( std::move(metadata.Volumes), std::move(ports), std::move(labels), + std::unordered_set{}, std::move(OnDeleted), EventTracker, DockerClient, @@ -1358,7 +1380,7 @@ std::unique_ptr WSLCContainerImpl::Open( metadata.InitProcessFlags, metadata.Flags); - // Restore the state change timestamp from Docker inspect data. + // Restore the state change timestamp and identify implicit volumes from Docker inspect data. try { auto inspectData = DockerClient.InspectContainer(dockerContainer.Id); @@ -1369,6 +1391,8 @@ std::unique_ptr WSLCContainerImpl::Open( { container->m_stateChangedAt = ParseDockerTimestamp(timestamp); } + + container->m_dockerVolumeNames = GetAnonymousVolumeNames(inspectData); } CATCH_LOG(); diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index 6ee4ff5ff..0ae3182af 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -16,7 +16,7 @@ Module Name: #include "ServiceProcessLauncher.h" #include "WSLCSession.h" -#include "ContainerEventTracker.h" +#include "DockerEventTracker.h" #include "DockerHTTPClient.h" #include "WSLCProcessControl.h" #include "IORelay.h" @@ -30,6 +30,7 @@ namespace wsl::windows::service::wslc { class WSLCContainer; class WSLCSession; +class WSLCVolumes; struct ContainerPortMapping { @@ -63,8 +64,9 @@ class WSLCContainerImpl std::vector&& volumes, std::vector&& ports, std::map&& labels, + std::unordered_set&& anonymousVolumes, std::function&& OnDeleted, - ContainerEventTracker& EventTracker, + DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, IORelay& Relay, WSLCContainerState InitialState, @@ -112,9 +114,9 @@ class WSLCContainerImpl const WSLCContainerOptions& Options, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - const std::unordered_map>& SessionVolumes, + WSLCVolumes& volumes, std::function&& OnDeleted, - ContainerEventTracker& EventTracker, + DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, IORelay& Relay); @@ -122,10 +124,9 @@ class WSLCContainerImpl const common::docker_schema::ContainerInfo& DockerContainer, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - const std::unordered_map>& sessionVolumes, - const std::unordered_set& anonymousVolumes, + WSLCVolumes& volumes, std::function&& OnDeleted, - ContainerEventTracker& EventTracker, + DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, IORelay& Relay); @@ -168,10 +169,11 @@ class WSLCContainerImpl WSLCVirtualMachine& m_virtualMachine; std::vector m_mappedPorts; std::vector m_mountedVolumes; + std::unordered_set m_dockerVolumeNames; std::map m_labels; Microsoft::WRL::ComPtr m_comWrapper; - ContainerEventTracker& m_eventTracker; - ContainerEventTracker::ContainerTrackingReference m_containerEvents; + DockerEventTracker& m_eventTracker; + DockerEventTracker::EventTrackingReference m_containerEvents; IORelay& m_ioRelay; WSLCContainerNetworkType m_networkingMode{}; }; diff --git a/src/windows/wslcsession/WSLCGuestVolume.cpp b/src/windows/wslcsession/WSLCGuestVolume.cpp new file mode 100644 index 000000000..3ea59acb0 --- /dev/null +++ b/src/windows/wslcsession/WSLCGuestVolume.cpp @@ -0,0 +1,141 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + WSLCGuestVolume.cpp + +Abstract: + + Implementation of WSLCGuestVolumeImpl - a WSLC volume whose storage is + owned entirely by docker's built-in "local" driver inside the guest VM. + +--*/ + +#include "precomp.h" +#include "DockerHTTPClient.h" +#include "WSLCGuestVolume.h" +#include "WSLCVolumeMetadata.h" +#include "wslc_schema.h" + +using namespace wsl::windows::common; +using wsl::shared::Localization; + +namespace wsl::windows::service::wslc { + +namespace { + + // The Docker local driver passes type/device/o directly to mount(8). + // Validate that the driver opts are either empty or only contain "type=tmpfs". + void ValidateDriverOpts(const std::map& DriverOpts) + { + if (DriverOpts.empty()) + { + return; + } + + // Determine the mount type. Only tmpfs is supported; everything else + // (none, nfs, cifs, ext4, ...) requires a device path or network access. + auto typeIt = DriverOpts.find("type"); + std::string type = (typeIt != DriverOpts.end()) ? typeIt->second : ""; + + THROW_HR_WITH_USER_ERROR_IF( + E_INVALIDARG, Localization::MessageWslcUnsupportedVolumeDriverOpts("type=" + type), !type.empty() && type != "tmpfs"); + } + +} // namespace + +WSLCGuestVolumeImpl::WSLCGuestVolumeImpl( + std::string&& Name, + std::string&& CreatedAt, + std::map&& DriverOpts, + std::map&& Labels, + DockerHTTPClient& DockerClient) : + m_name(std::move(Name)), m_createdAt(std::move(CreatedAt)), m_driverOpts(std::move(DriverOpts)), m_labels(std::move(Labels)), m_dockerClient(DockerClient) +{ +} + +std::unique_ptr WSLCGuestVolumeImpl::Create( + LPCSTR Name, std::map&& DriverOpts, std::map&& Labels, DockerHTTPClient& DockerClient) +{ + ValidateDriverOpts(DriverOpts); + + docker_schema::CreateVolume request{}; + if (Name != nullptr && Name[0] != '\0') + { + request.Name = Name; + } + request.Driver = "local"; + request.DriverOpts = DriverOpts; + + // Merge user labels into the Docker volume labels. + for (const auto& [key, value] : Labels) + { + request.Labels[key] = value; + } + + try + { + auto createdVolume = DockerClient.CreateVolume(request); + + return std::make_unique( + std::move(createdVolume.Name), std::move(createdVolume.CreatedAt), std::move(DriverOpts), std::move(Labels), DockerClient); + } + CATCH_AND_THROW_DOCKER_USER_ERROR("Failed to create volume '%hs'", Name != nullptr ? Name : ""); +} + +std::unique_ptr WSLCGuestVolumeImpl::Open(const wsl::windows::common::docker_schema::Volume& Volume, DockerHTTPClient& DockerClient) +{ + THROW_HR_IF(E_INVALIDARG, Volume.Driver != "local"); + + if (Volume.Options.has_value()) + { + ValidateDriverOpts(Volume.Options.value()); + } + + std::map driverOpts = Volume.Options.value_or(std::map{}); + std::map labels = Volume.Labels.value_or(std::map{}); + + return std::make_unique( + std::string{Volume.Name}, std::string{Volume.CreatedAt}, std::move(driverOpts), std::move(labels), DockerClient); +} + +void WSLCGuestVolumeImpl::Delete() +{ + try + { + m_dockerClient.RemoveVolume(m_name); + } + catch (const DockerHTTPException& e) + { + THROW_HR_WITH_USER_ERROR_IF( + HRESULT_FROM_WIN32(ERROR_SHARING_VIOLATION), Localization::MessageWslcVolumeInUse(m_name.c_str()), e.StatusCode() == 409); + THROW_HR_WITH_USER_ERROR_IF(WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(m_name.c_str()), e.StatusCode() == 404); + THROW_DOCKER_USER_ERROR_MSG(e, "Failed to delete volume '%hs'", m_name.c_str()); + } +} + +std::string WSLCGuestVolumeImpl::Inspect() const +{ + wslc_schema::InspectVolume inspect{}; + inspect.Name = m_name; + inspect.Driver = WSLCGuestVolumeDriver; + inspect.CreatedAt = m_createdAt; + inspect.DriverOpts = m_driverOpts; + inspect.Labels = m_labels; + + return wsl::shared::ToJson(inspect); +} + +WSLCVolumeInformation WSLCGuestVolumeImpl::GetVolumeInformation() const +{ + WSLCVolumeInformation Info{}; + + THROW_HR_IF(E_UNEXPECTED, strcpy_s(Info.Name, m_name.c_str()) != 0); + THROW_HR_IF(E_UNEXPECTED, strcpy_s(Info.Driver, WSLCGuestVolumeDriver) != 0); + + return Info; +} + +} // namespace wsl::windows::service::wslc diff --git a/src/windows/wslcsession/WSLCGuestVolume.h b/src/windows/wslcsession/WSLCGuestVolume.h new file mode 100644 index 000000000..3ebc701f7 --- /dev/null +++ b/src/windows/wslcsession/WSLCGuestVolume.h @@ -0,0 +1,79 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + WSLCGuestVolume.h + +Abstract: + + Volume implementation that delegates storage to docker's built-in "local" + volume driver. The volume lives on the session storage VHD at + /var/lib/docker/volumes//_data inside the guest VM. No host-side + artifacts (no extra VHD file, no disk attach). + +--*/ + +#pragma once + +#include "IWSLCVolume.h" +#include "WSLCVolumeMetadata.h" +#include "wslc.h" +#include +#include +#include + +namespace wsl::windows::common::docker_schema { +struct Volume; +} + +namespace wsl::windows::service::wslc { + +class DockerHTTPClient; + +class WSLCGuestVolumeImpl : public IWSLCVolume +{ +public: + NON_COPYABLE(WSLCGuestVolumeImpl); + NON_MOVABLE(WSLCGuestVolumeImpl); + + WSLCGuestVolumeImpl( + std::string&& Name, + std::string&& CreatedAt, + std::map&& DriverOpts, + std::map&& Labels, + DockerHTTPClient& DockerClient); + + ~WSLCGuestVolumeImpl() = default; + + static std::unique_ptr Create( + _In_opt_ LPCSTR Name, + _In_ std::map&& DriverOpts, + _In_ std::map&& Labels, + _In_ DockerHTTPClient& DockerClient); + + static std::unique_ptr Open(_In_ const wsl::windows::common::docker_schema::Volume& Volume, _In_ DockerHTTPClient& DockerClient); + + // IWSLCVolume + const std::string& Name() const noexcept override + { + return m_name; + } + const char* Driver() const noexcept override + { + return WSLCGuestVolumeDriver; + } + void Delete() override; + std::string Inspect() const override; + WSLCVolumeInformation GetVolumeInformation() const override; + +private: + std::string m_name; + std::string m_createdAt; + std::map m_driverOpts; + std::map m_labels; + DockerHTTPClient& m_dockerClient; +}; + +} // namespace wsl::windows::service::wslc diff --git a/src/windows/wslcsession/WSLCProcessControl.cpp b/src/windows/wslcsession/WSLCProcessControl.cpp index 43eddbda8..158e47516 100644 --- a/src/windows/wslcsession/WSLCProcessControl.cpp +++ b/src/windows/wslcsession/WSLCProcessControl.cpp @@ -41,7 +41,7 @@ const wil::unique_event& WSLCProcessControl::GetExitEvent() const return m_exitEvent; } -DockerContainerProcessControl::DockerContainerProcessControl(WSLCContainerImpl& Container, DockerHTTPClient& DockerClient, ContainerEventTracker& EventTracker) : +DockerContainerProcessControl::DockerContainerProcessControl(WSLCContainerImpl& Container, DockerHTTPClient& DockerClient, DockerEventTracker& EventTracker) : m_container(&Container), m_client(DockerClient), m_trackingReference(EventTracker.RegisterContainerStateUpdates( @@ -114,7 +114,7 @@ void DockerContainerProcessControl::OnContainerReleased() noexcept } DockerExecProcessControl::DockerExecProcessControl( - WSLCContainerImpl& Container, const std::string& Id, DockerHTTPClient& DockerClient, ContainerEventTracker& EventTracker) : + WSLCContainerImpl& Container, const std::string& Id, DockerHTTPClient& DockerClient, DockerEventTracker& EventTracker) : m_container(&Container), m_id(Id), m_client(DockerClient), diff --git a/src/windows/wslcsession/WSLCProcessControl.h b/src/windows/wslcsession/WSLCProcessControl.h index f33e0703e..8644a2d06 100644 --- a/src/windows/wslcsession/WSLCProcessControl.h +++ b/src/windows/wslcsession/WSLCProcessControl.h @@ -15,7 +15,7 @@ Module Name: #pragma once #include "wslc.h" #include "DockerHTTPClient.h" -#include "ContainerEventTracker.h" +#include "DockerEventTracker.h" namespace wsl::windows::service::wslc { @@ -42,7 +42,7 @@ class WSLCProcessControl class DockerContainerProcessControl : public WSLCProcessControl { public: - DockerContainerProcessControl(WSLCContainerImpl& Container, DockerHTTPClient& DockerClient, ContainerEventTracker& EventTracker); + DockerContainerProcessControl(WSLCContainerImpl& Container, DockerHTTPClient& DockerClient, DockerEventTracker& EventTracker); ~DockerContainerProcessControl(); void Signal(int Signal) override; void ResizeTty(ULONG Rows, ULONG Columns) override; @@ -55,13 +55,13 @@ class DockerContainerProcessControl : public WSLCProcessControl std::mutex m_lock; DockerHTTPClient& m_client; WSLCContainerImpl* m_container{}; - ContainerEventTracker::ContainerTrackingReference m_trackingReference; + DockerEventTracker::EventTrackingReference m_trackingReference; }; class DockerExecProcessControl : public WSLCProcessControl { public: - DockerExecProcessControl(WSLCContainerImpl& Container, const std::string& Id, DockerHTTPClient& DockerClient, ContainerEventTracker& EventTracker); + DockerExecProcessControl(WSLCContainerImpl& Container, const std::string& Id, DockerHTTPClient& DockerClient, DockerEventTracker& EventTracker); ~DockerExecProcessControl(); void Signal(int Signal) override; void ResizeTty(ULONG Rows, ULONG Columns) override; @@ -79,7 +79,7 @@ class DockerExecProcessControl : public WSLCProcessControl std::optional m_pid{}; DockerHTTPClient& m_client; WSLCContainerImpl* m_container{}; - ContainerEventTracker::ContainerTrackingReference m_trackingReference; + DockerEventTracker::EventTrackingReference m_trackingReference; }; class VMProcessControl : public WSLCProcessControl diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index f38050727..75ea74e56 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -260,9 +260,10 @@ try // Start the event tracker. m_eventTracker.emplace(m_dockerClient.value(), m_id, m_ioRelay); - // Recover any existing containers from storage. + m_volumes.emplace(m_dockerClient.value(), m_virtualMachine.value(), m_eventTracker.value()); + + // Recover any existing resources from storage. RecoverExistingNetworks(); - RecoverExistingVolumes(); RecoverExistingContainers(); errorCleanup.release(); @@ -1586,13 +1587,13 @@ try try { - std::scoped_lock lock(m_containersLock, m_volumesLock); + std::lock_guard lock(m_containersLock); auto& it = m_containers.emplace_back(WSLCContainerImpl::Create( *containerOptions, *this, m_virtualMachine.value(), - m_volumes, + *m_volumes, std::bind(&WSLCSession::OnContainerDeleted, this, std::placeholders::_1), m_eventTracker.value(), m_dockerClient.value(), @@ -1880,44 +1881,20 @@ try RETURN_HR_IF_NULL(E_POINTER, VolumeInfo); ZeroMemory(VolumeInfo, sizeof(*VolumeInfo)); - // Default driver to "vhd" if not specified. Currently only "vhd" is supported. - // TODO: Add support for docker's builtin volume drivers and change the default to "local" - // to match docker's behaviour. - std::string driver = (Options->Driver != nullptr) ? Options->Driver : WSLCVhdVolumeDriver; - THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcInvalidVolumeType(driver), driver != WSLCVhdVolumeDriver); + std::string driver = (Options->Driver != nullptr && *Options->Driver != '\0') ? Options->Driver : WSLCVhdVolumeDriver; auto driverOpts = wslutil::ParseKeyValuePairs(Options->DriverOpts, Options->DriverOptsCount); auto labels = wslutil::ParseKeyValuePairs(Options->Labels, Options->LabelsCount, WSLCVolumeMetadataLabel); auto lock = m_lock.lock_shared(); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_virtualMachine); - - std::lock_guard volumesLock(m_volumesLock); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_volumes); if (Options->Name != nullptr && Options->Name[0] != '\0') { ValidateName(Options->Name, WSLC_MAX_VOLUME_NAME_LENGTH); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), m_volumes.contains(Options->Name)); } - auto volume = WSLCVhdVolumeImpl::Create( - Options->Name, - std::move(driverOpts), - std::move(labels), - m_storageVhdPath.parent_path(), - m_virtualMachine.value(), - m_dockerClient.value()); - - const auto& name = volume->Name(); - auto info = volume->GetVolumeInformation(); - - auto [it, inserted] = m_volumes.insert({name, std::move(volume)}); - WI_VERIFY(inserted); - - WSL_LOG("VolumeCreated", TraceLoggingValue(name.c_str(), "VolumeName")); - - *VolumeInfo = info; + *VolumeInfo = m_volumes->CreateVolume(Options->Name, driver, std::move(driverOpts), std::move(labels), m_storageVhdPath.parent_path()); return S_OK; } CATCH_RETURN(); @@ -1932,18 +1909,9 @@ try ValidateName(name.c_str(), WSLC_MAX_VOLUME_NAME_LENGTH); auto lock = m_lock.lock_shared(); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_virtualMachine); - - std::lock_guard volumesLock(m_volumesLock); - - auto it = m_volumes.find(name); - THROW_HR_WITH_USER_ERROR_IF(WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(name), it == m_volumes.end()); - - it->second->Delete(); - m_volumes.erase(it); - WSL_LOG("VolumeDeleted", TraceLoggingValue(name.c_str(), "VolumeName")); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_volumes); + m_volumes->DeleteVolume(name); return S_OK; } CATCH_RETURN(); @@ -1960,25 +1928,23 @@ try *Count = 0; auto lock = m_lock.lock_shared(); - std::lock_guard volumesLock(m_volumesLock); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_volumes); + + auto volumeList = m_volumes->ListVolumes(); - if (m_volumes.empty()) + if (volumeList.empty()) { return S_OK; } - auto output = wil::make_unique_cotaskmem(m_volumes.size()); - - ULONG index = 0; - for (const auto& [name, vol] : m_volumes) + auto output = wil::make_unique_cotaskmem(volumeList.size()); + for (size_t i = 0; i < volumeList.size(); i++) { - output[index] = vol->GetVolumeInformation(); - index++; + output[i] = volumeList[i]; } + *Count = static_cast(volumeList.size()); *Volumes = output.release(); - *Count = index; - return S_OK; } CATCH_RETURN(); @@ -1997,14 +1963,9 @@ try ValidateName(name.c_str(), WSLC_MAX_VOLUME_NAME_LENGTH); auto lock = m_lock.lock_shared(); - std::lock_guard volumesLock(m_volumesLock); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_volumes); - auto it = m_volumes.find(name); - THROW_HR_WITH_USER_ERROR_IF(WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(name), it == m_volumes.end()); - - const auto& volume = it->second; - - std::string json = volume->Inspect(); + std::string json = m_volumes->InspectVolume(name); *Output = wil::make_unique_ansistring(json.c_str()).release(); return S_OK; @@ -2292,11 +2253,10 @@ try // Acquire an exclusive lock to ensure that no operation is running. auto lock = m_lock.lock_exclusive(); std::lock_guard containersLock(m_containersLock); - std::lock_guard volumesLock(m_volumesLock); std::lock_guard networksLock(m_networksLock); m_containers.clear(); - m_volumes.clear(); + m_volumes.reset(); m_networks.clear(); // Stop the IO relay. @@ -2560,6 +2520,17 @@ void WSLCSession::OnContainerDeleted(const WSLCContainerImpl* Container) WI_VERIFY(std::erase_if(m_containers, [Container](const auto& e) { return e.get() == Container; }) == 1); } +void WSLCSession::DeleteContainerVolumes(const std::unordered_set& VolumeNames) +{ + if (m_volumes) + { + for (const auto& name : VolumeNames) + { + m_volumes->OnVolumeDeleted(name); + } + } +} + HRESULT WSLCSession::GetState(_Out_ WSLCSessionState* State) { *State = m_terminated ? WSLCSessionStateTerminated : WSLCSessionStateRunning; @@ -2582,8 +2553,7 @@ void WSLCSession::RecoverExistingContainers() dockerContainer, *this, m_virtualMachine.value(), - m_volumes, - m_anonymousVolumes, + *m_volumes, std::bind(&WSLCSession::OnContainerDeleted, this, std::placeholders::_1), m_eventTracker.value(), m_dockerClient.value(), @@ -2652,38 +2622,4 @@ void WSLCSession::RecoverExistingNetworks() TraceLoggingValue(m_networks.size(), "NetworkCount")); } -void WSLCSession::RecoverExistingVolumes() -{ - WI_ASSERT(m_dockerClient.has_value()); - WI_ASSERT(m_virtualMachine.has_value()); - - auto volumes = m_dockerClient->ListVolumes(); - - std::lock_guard volumesLock(m_volumesLock); - - for (const auto& volume : volumes) - { - if (!volume.Labels.has_value() || !volume.Labels->contains(WSLCVolumeMetadataLabel)) - { - m_anonymousVolumes.insert(volume.Name); - continue; - } - - try - { - WI_ASSERT(!m_volumes.contains(volume.Name)); - - auto vhdVolume = WSLCVhdVolumeImpl::Open(volume, m_virtualMachine.value(), m_dockerClient.value()); - auto [_, inserted] = m_volumes.insert({volume.Name, std::move(vhdVolume)}); - WI_VERIFY(inserted); - } - CATCH_LOG_MSG("Failed to recover volume: %hs", volume.Name.c_str()); - } - - WSL_LOG( - "VolumesRecovered", - TraceLoggingValue(m_displayName.c_str(), "SessionName"), - TraceLoggingValue(m_volumes.size(), "VolumeCount")); -} - } // namespace wsl::windows::service::wslc diff --git a/src/windows/wslcsession/WSLCSession.h b/src/windows/wslcsession/WSLCSession.h index edc6ed85d..7c919fbf4 100644 --- a/src/windows/wslcsession/WSLCSession.h +++ b/src/windows/wslcsession/WSLCSession.h @@ -17,10 +17,9 @@ Module Name: #include "wslc.h" #include "WSLCVirtualMachine.h" #include "WSLCContainer.h" -#include "WSLCVhdVolume.h" -#include "WSLCVolumeMetadata.h" +#include "WSLCVolumes.h" #include "WSLCNetworkMetadata.h" -#include "ContainerEventTracker.h" +#include "DockerEventTracker.h" #include "DockerHTTPClient.h" #include "IORelay.h" #include @@ -151,9 +150,13 @@ class DECLSPEC_UUID("4877FEFC-4977-4929-A958-9F36AA1892A4") WSLCSession UserHandle OpenUserHandle(WSLCHandle Handle); void ReleaseUserHandle(HANDLE Handle); + + UserCOMCallback RegisterUserCOMCallback(); void UnregisterUserCOMCallback(DWORD ThreadId); + void DeleteContainerVolumes(_In_ const std::unordered_set& VolumeNames); + private: ULONG m_id = 0; @@ -172,7 +175,6 @@ class DECLSPEC_UUID("4877FEFC-4977-4929-A958-9F36AA1892A4") WSLCSession int StopProcess(ServiceRunningProcess& Process, DWORD TerminateTimeoutMs, DWORD KillTimeoutMs); void ImportImageImpl(DockerHTTPClient::HTTPRequestContext& Request, const WSLCHandle ImageHandle); void RecoverExistingContainers(); - void RecoverExistingVolumes(); void RecoverExistingNetworks(); void SaveImageImpl(std::pair& RequestCodePair, WSLCHandle OutputHandle, HANDLE CancelEvent); @@ -180,19 +182,18 @@ class DECLSPEC_UUID("4877FEFC-4977-4929-A958-9F36AA1892A4") WSLCSession std::optional m_dockerClient; std::optional m_virtualMachine; - std::optional m_eventTracker; + std::optional m_eventTracker; wil::unique_event m_dockerdReadyEvent{wil::EventOptions::ManualReset}; std::wstring m_displayName; std::filesystem::path m_storageVhdPath; - // N.B. m_lock must be acquired before acquiring m_volumesLock, m_containersLock, or m_networksLock. - // These locks protect m_volumes / m_containers without requiring an exclusive m_lock. - // This allows independent operations to proceed while volume/container bookkeeping remains synchronized. + // N.B. m_lock must be acquired before acquiring m_containersLock or m_networksLock. + // These locks protect m_containers without requiring an exclusive m_lock. + // This allows independent operations to proceed while container bookkeeping remains synchronized. + // WSLCVolumes has its own internal srwlock and does not require m_lock. std::mutex m_containersLock; - std::mutex m_volumesLock; std::vector> m_containers; - std::unordered_map> m_volumes; - std::unordered_set m_anonymousVolumes; // TODO: Implement proper anonymous volume support. + std::optional m_volumes; std::mutex m_networksLock; std::unordered_map m_networks; wil::unique_event m_sessionTerminatingEvent{wil::EventOptions::ManualReset}; diff --git a/src/windows/wslcsession/WSLCVhdVolume.h b/src/windows/wslcsession/WSLCVhdVolume.h index 6082c36f8..e0945224a 100644 --- a/src/windows/wslcsession/WSLCVhdVolume.h +++ b/src/windows/wslcsession/WSLCVhdVolume.h @@ -14,6 +14,7 @@ Module Name: #pragma once +#include "IWSLCVolume.h" #include "WSLCVolumeMetadata.h" #include "wslc.h" #include @@ -29,7 +30,7 @@ namespace wsl::windows::service::wslc { class WSLCVirtualMachine; class DockerHTTPClient; -class WSLCVhdVolumeImpl +class WSLCVhdVolumeImpl : public IWSLCVolume { public: NON_COPYABLE(WSLCVhdVolumeImpl); @@ -59,14 +60,19 @@ class WSLCVhdVolumeImpl static std::unique_ptr Open( _In_ const wsl::windows::common::docker_schema::Volume& Volume, _In_ WSLCVirtualMachine& VirtualMachine, _In_ DockerHTTPClient& DockerClient); - void Delete(); - std::string Inspect() const; - WSLCVolumeInformation GetVolumeInformation() const; - - const std::string& Name() const noexcept + // IWSLCVolume + const std::string& Name() const noexcept override { return m_name; } + const char* Driver() const noexcept override + { + return WSLCVhdVolumeDriver; + } + void Delete() override; + std::string Inspect() const override; + WSLCVolumeInformation GetVolumeInformation() const override; + const std::string& VirtualMachinePath() const noexcept { return m_virtualMachinePath; diff --git a/src/windows/wslcsession/WSLCVolumeMetadata.h b/src/windows/wslcsession/WSLCVolumeMetadata.h index 0b98c0adc..6861745ea 100644 --- a/src/windows/wslcsession/WSLCVolumeMetadata.h +++ b/src/windows/wslcsession/WSLCVolumeMetadata.h @@ -24,6 +24,9 @@ constexpr auto WSLCVolumeMetadataLabel = "com.microsoft.wsl.volume.metadata"; // Volume driver name for VHD-backed volumes. constexpr auto WSLCVhdVolumeDriver = "vhd"; +// Volume driver name for guest-backed volumes (passthrough to docker's built-in "local" driver). +constexpr auto WSLCGuestVolumeDriver = "guest"; + struct WSLCVolumeMetadata { std::string Driver; diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp new file mode 100644 index 000000000..5fe287843 --- /dev/null +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -0,0 +1,187 @@ +// Copyright (C) Microsoft Corporation. All rights reserved. + +#include "precomp.h" +#include "WSLCVolumes.h" +#include "WSLCVhdVolume.h" +#include "WSLCGuestVolume.h" +#include "WSLCVirtualMachine.h" +#include "docker_schema.h" + +using wsl::shared::Localization; +using wsl::windows::service::wslc::DockerHTTPClient; +using wsl::windows::service::wslc::WSLCVolumes; + +namespace wsl::windows::service::wslc { + +WSLCVolumes::WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker) : + m_dockerClient(dockerClient), + m_virtualMachine(virtualMachine) +{ + // Recover existing volumes from Docker. + for (const auto& volume : dockerClient.ListVolumes()) + { + try + { + m_volumes.insert({volume.Name, OpenDockerVolume(volume)}); + } + CATCH_LOG_MSG("Failed to recover volume: %hs", volume.Name.c_str()); + } + + // Register for volume events after recovery is complete. + m_volumeEventTracking = eventTracker.RegisterVolumeUpdates( + std::bind(&WSLCVolumes::OnVolumeEvent, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); +} + +std::unique_ptr WSLCVolumes::OpenDockerVolume(const wsl::windows::common::docker_schema::Volume& vol) +{ + std::unique_ptr opened; + + if (vol.Labels.has_value() && vol.Labels->contains(WSLCVolumeMetadataLabel)) + { + auto metadata = wsl::shared::FromJson(vol.Labels->at(WSLCVolumeMetadataLabel).c_str()); + + if (metadata.Driver == WSLCVhdVolumeDriver) + { + opened = WSLCVhdVolumeImpl::Open(vol, m_virtualMachine, m_dockerClient); + } + } + else if (vol.Driver == "local") + { + opened = WSLCGuestVolumeImpl::Open(vol, m_dockerClient); + } + else + { + THROW_HR_MSG(E_UNEXPECTED, "Unrecognized volume driver: %hs", vol.Driver.c_str()); + } + + WSL_LOG("VolumeOpened", TraceLoggingValue(vol.Name.c_str(), "VolumeName"), TraceLoggingValue(vol.Driver.c_str(), "Driver")); + + return opened; +} + +void WSLCVolumes::OnVolumeEvent(const std::string& volumeName, VolumeEvent event, std::uint64_t) +{ + if (event == VolumeEvent::Create) + { + OpenVolume(volumeName); + } + else if (event == VolumeEvent::Destroy) + { + OnVolumeDeleted(volumeName); + } +} + +WSLCVolumeInformation WSLCVolumes::CreateVolume( + LPCSTR Name, + const std::string& Driver, + std::map&& DriverOpts, + std::map&& Labels, + const std::filesystem::path& StoragePath) +{ + auto lock = m_lock.lock_exclusive(); + + if (Name != nullptr && Name[0] != '\0') + { + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), m_volumes.contains(Name)); + } + + std::unique_ptr volume; + + if (Driver == WSLCVhdVolumeDriver) + { + volume = WSLCVhdVolumeImpl::Create(Name, std::move(DriverOpts), std::move(Labels), StoragePath, m_virtualMachine, m_dockerClient); + } + else if (Driver == WSLCGuestVolumeDriver) + { + volume = WSLCGuestVolumeImpl::Create(Name, std::move(DriverOpts), std::move(Labels), m_dockerClient); + } + else + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::MessageWslcInvalidVolumeType(Driver)); + } + + const auto& name = volume->Name(); + auto info = volume->GetVolumeInformation(); + + auto [it, inserted] = m_volumes.insert({name, std::move(volume)}); + WI_VERIFY(inserted); + + WSL_LOG("VolumeCreated", TraceLoggingValue(name.c_str(), "VolumeName"), TraceLoggingValue(Driver.c_str(), "Driver")); + + return info; +} + +void WSLCVolumes::DeleteVolume(const std::string& Name) +{ + auto lock = m_lock.lock_exclusive(); + + auto it = m_volumes.find(Name); + THROW_HR_WITH_USER_ERROR_IF(WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(Name), it == m_volumes.end()); + + it->second->Delete(); + m_volumes.erase(it); + + WSL_LOG("VolumeDeleted", TraceLoggingValue(Name.c_str(), "VolumeName")); +} + +std::vector WSLCVolumes::ListVolumes() const +{ + auto lock = m_lock.lock_shared(); + + std::vector result; + result.reserve(m_volumes.size()); + + for (const auto& [name, vol] : m_volumes) + { + result.push_back(vol->GetVolumeInformation()); + } + + return result; +} + +std::string WSLCVolumes::InspectVolume(const std::string& Name) const +{ + auto lock = m_lock.lock_shared(); + + auto it = m_volumes.find(Name); + THROW_HR_WITH_USER_ERROR_IF(WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(Name), it == m_volumes.end()); + + return it->second->Inspect(); +} + +bool WSLCVolumes::ContainsVolume(const std::string& Name) const +{ + auto lock = m_lock.lock_shared(); + return m_volumes.contains(Name); +} + +void WSLCVolumes::OpenVolume(const std::string& VolumeName) +{ + auto lock = m_lock.lock_exclusive(); + + if (VolumeName.empty() || m_volumes.contains(VolumeName)) + { + return; + } + + try + { + auto vol = m_dockerClient.InspectVolume(VolumeName); + m_volumes.insert({VolumeName, OpenDockerVolume(vol)}); + } + CATCH_LOG_MSG("Failed to open volume: %hs", VolumeName.c_str()); +} + +void WSLCVolumes::OnVolumeDeleted(const std::string& VolumeName) +{ + auto lock = m_lock.lock_exclusive(); + + auto it = m_volumes.find(VolumeName); + if (it != m_volumes.end()) + { + WSL_LOG("VolumeRemovedWithContainer", TraceLoggingValue(VolumeName.c_str(), "VolumeName")); + m_volumes.erase(it); + } +} + +} // namespace wsl::windows::service::wslc diff --git a/src/windows/wslcsession/WSLCVolumes.h b/src/windows/wslcsession/WSLCVolumes.h new file mode 100644 index 000000000..a750852e5 --- /dev/null +++ b/src/windows/wslcsession/WSLCVolumes.h @@ -0,0 +1,57 @@ +// Copyright (C) Microsoft Corporation. All rights reserved. + +#pragma once + +#include "IWSLCVolume.h" +#include "WSLCVolumeMetadata.h" +#include "DockerHTTPClient.h" +#include "DockerEventTracker.h" +#include +#include + +namespace wsl::windows::service::wslc { + +class WSLCVirtualMachine; + +class WSLCVolumes +{ +public: + NON_COPYABLE(WSLCVolumes); + NON_MOVABLE(WSLCVolumes); + + WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker); + ~WSLCVolumes() = default; + + WSLCVolumeInformation CreateVolume( + _In_opt_ LPCSTR Name, + _In_ const std::string& Driver, + _In_ std::map&& DriverOpts, + _In_ std::map&& Labels, + _In_ const std::filesystem::path& StoragePath); + + void DeleteVolume(_In_ const std::string& Name); + + std::vector ListVolumes() const; + std::string InspectVolume(_In_ const std::string& Name) const; + bool ContainsVolume(_In_ const std::string& Name) const; + + // Opens a volume by name. Used to track volumes Docker implicitly created for VOLUME directives. + void OpenVolume(_In_ const std::string& VolumeName); + + // Removes a volume from tracking. Called when Docker deletes a volume (e.g. container delete with -v). + void OnVolumeDeleted(_In_ const std::string& VolumeName); + +private: + std::unique_ptr OpenDockerVolume(const wsl::windows::common::docker_schema::Volume& vol); + + void OnVolumeEvent(const std::string& volumeName, VolumeEvent event, std::uint64_t eventTime); + + mutable wil::srwlock m_lock; + _Guarded_by_(m_lock) std::unordered_map> m_volumes; + + DockerHTTPClient& m_dockerClient; + WSLCVirtualMachine& m_virtualMachine; + DockerEventTracker::EventTrackingReference m_volumeEventTracking; +}; + +} // namespace wsl::windows::service::wslc diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index b375ccbbf..387cf65e1 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -1925,21 +1925,21 @@ class WSLCTests VERIFY_SUCCEEDED(BuildImageFromContext(contextDir, "wslc-test-build:latest")); ExpectImagePresent(*m_defaultSession, "wslc-test-build:latest"); - // Lists anonymous docker volume names via the VM's docker CLI. - // TODO: Add proper support so we can list via session's API instead. auto listAnonymousVolumes = [&]() { - auto result = ExpectCommandResult( - m_defaultSession.get(), {"/usr/bin/docker", "volume", "ls", "-q", "-f", "label=com.docker.volume.anonymous"}, 0); + wil::unique_cotaskmem_array_ptr volumes; + VERIFY_SUCCEEDED(m_defaultSession->ListVolumes(volumes.addressof(), volumes.size_address())); + std::vector names; - std::stringstream ss(result.Output[1]); - std::string line; - while (std::getline(ss, line)) + + // TODO: Replace with filter for anonymous volumes in ListVolumes API. + for (const auto& vol : volumes) { - if (!line.empty()) + if (std::string(vol.Driver) == "guest") { - names.push_back(line); + names.push_back(vol.Name); } } + return names; }; @@ -1983,16 +1983,21 @@ class WSLCTests container.GetInitProcess().Wait(); container.SetDeleteOnClose(false); + // Clean up any leaked anonymous volumes when this block exits. + auto volumeCleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + auto volumes = listAnonymousVolumes(); + for (const auto& name : volumes) + { + LOG_IF_FAILED(m_defaultSession->DeleteVolume(name.c_str())); + } + }); + VERIFY_ARE_EQUAL(listAnonymousVolumes().size(), 1u); VERIFY_SUCCEEDED(container.Get().Delete(WSLCDeleteFlagsNone)); // Anonymous volume was NOT deleted by Docker. - auto leaked = listAnonymousVolumes(); - VERIFY_ARE_EQUAL(leaked.size(), 1u); - - RunCommand(m_defaultSession.get(), {"/usr/bin/docker", "volume", "prune", "-f"}); - VERIFY_ARE_EQUAL(listAnonymousVolumes().size(), 0u); + VERIFY_ARE_EQUAL(listAnonymousVolumes().size(), 1u); } // Delete container with WSLCDeleteFlagsDeleteVolumes -> anonymous volume is cleaned up. @@ -3437,42 +3442,38 @@ class WSLCTests VERIFY_ARE_EQUAL(m_defaultSession->FormatVirtualDisk(L"C:\\DoesNotExist.vhdx"), HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)); } - WSLC_TEST_METHOD(NamedVolumesTest) + // Exercises behavior that all volume drivers must implement identically: + // create, duplicate-name rejection, multi-mount, cross-container read/write, + // in-use deletion rejection, and clean deletion after the referencing container is removed. + void ValidateNamedVolumeContract(std::string_view driver, const WSLCDriverOption* driverOpts, ULONG driverOptsCount) { - const std::string volumeName = "wslc-test-named-volume"; - const std::filesystem::path volumeVhdPath = m_storagePath / "volumes" / (volumeName + ".vhdx"); + const std::string driverStr(driver); + const std::string volumeName = std::format("wslc-test-named-volume-{}", driver); // Best-effort cleanup in case of leftovers from a previous failed run. LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); - auto cleanup = wil::scope_exit([&]() { - LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); - std::error_code ec; - std::filesystem::remove(volumeVhdPath, ec); - }); + auto cleanup = wil::scope_exit([&]() { LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); }); WSLCVolumeOptions volumeOptions{}; volumeOptions.Name = volumeName.c_str(); - - WSLCDriverOption driverOpts[] = {{"SizeBytes", "1073741824"}}; + volumeOptions.Driver = driverStr.c_str(); volumeOptions.DriverOpts = driverOpts; - volumeOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + volumeOptions.DriverOptsCount = driverOptsCount; // Create volume and validate duplicate volume name handling. WSLCVolumeInformation volInfo{}; VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); VERIFY_ARE_EQUAL(std::string(volInfo.Name), volumeName); - VERIFY_ARE_EQUAL(std::string(volInfo.Driver), std::string("vhd")); + VERIFY_ARE_EQUAL(std::string(volInfo.Driver), driverStr); VERIFY_ARE_EQUAL(m_defaultSession->CreateVolume(&volumeOptions, &volInfo), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS)); - // Verify volume VHD exists and mount point is present in the VM. - VERIFY_IS_TRUE(std::filesystem::exists(volumeVhdPath)); - ExpectMount(m_defaultSession.get(), std::format("/mnt/wslc-volumes/{}", volumeName), std::optional{"*ext4*"}); - // Verify the same named volume can be mounted more than once with different container paths. { WSLCContainerLauncher duplicateNamedVolumes( - "debian:latest", "named-volume-dup", {"/bin/sh", "-c", "echo duplicated >/data-a/dup.txt ; cat /data-b/dup.txt"}); + "debian:latest", + std::format("named-volume-dup-{}", driver), + {"/bin/sh", "-c", "echo duplicated >/data-a/dup.txt ; cat /data-b/dup.txt"}); duplicateNamedVolumes.AddNamedVolume(volumeName, "/data-a", false); duplicateNamedVolumes.AddNamedVolume(volumeName, "/data-b", true); @@ -3484,14 +3485,17 @@ class WSLCTests // Verify CreateContainer with named volume mounts the volume into the container. { WSLCContainerLauncher writer( - "debian:latest", "named-volume-writer", {"/bin/sh", "-c", "echo wslc-named-volume >/data/marker.txt"}); + "debian:latest", + std::format("named-volume-writer-{}", driver), + {"/bin/sh", "-c", "echo wslc-named-volume >/data/marker.txt"}); writer.AddNamedVolume(volumeName, "/data", false); auto writerContainer = writer.Launch(*m_defaultSession); auto writerProcess = writerContainer.GetInitProcess(); ValidateProcessOutput(writerProcess, {}); - WSLCContainerLauncher reader("debian:latest", "named-volume-reader", {"/bin/sh", "-c", "cat /data/marker.txt"}); + WSLCContainerLauncher reader( + "debian:latest", std::format("named-volume-reader-{}", driver), {"/bin/sh", "-c", "cat /data/marker.txt"}); reader.AddNamedVolume(volumeName, "/data", true); auto readerContainer = reader.Launch(*m_defaultSession); @@ -3500,7 +3504,7 @@ class WSLCTests } // Verify we cannot delete a named volume while a container references it. - WSLCContainerLauncher holder("debian:latest", "named-volume-holder", {"sleep", "99999"}); + WSLCContainerLauncher holder("debian:latest", std::format("named-volume-holder-{}", driver), {"sleep", "99999"}); holder.AddNamedVolume(volumeName, "/data", false); auto [holderCreateResult, holderContainerResult] = holder.CreateNoThrow(*m_defaultSession); @@ -3516,44 +3520,69 @@ class WSLCTests VERIFY_SUCCEEDED(holderContainer.Get().Delete(WSLCDeleteFlagsNone)); VERIFY_SUCCEEDED(m_defaultSession->DeleteVolume(volumeName.c_str())); + cleanup.release(); + } + + WSLC_TEST_METHOD(NamedVolumesVhd) + { + WSLCDriverOption driverOpts[] = {{"SizeBytes", "1073741824"}}; + ValidateNamedVolumeContract("vhd", driverOpts, ARRAYSIZE(driverOpts)); + + // VHD-driver-specific: validate the host-side .vhdx artifact and the + // /mnt/wslc-volumes ext4 mount inside the VM appear and disappear with + // the volume. + const std::string volumeName = "wslc-test-named-volume-vhd-host"; + const std::filesystem::path volumeVhdPath = m_storagePath / "volumes" / (volumeName + ".vhdx"); + + WSLCVolumeOptions volumeOptions{}; + volumeOptions.Name = volumeName.c_str(); + volumeOptions.Driver = "vhd"; + volumeOptions.DriverOpts = driverOpts; + volumeOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + + WSLCVolumeInformation volInfo{}; + VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); + auto cleanup = wil::scope_exit([&]() { LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); }); + + VERIFY_IS_TRUE(std::filesystem::exists(volumeVhdPath)); + ExpectMount(m_defaultSession.get(), std::format("/mnt/wslc-volumes/{}", volumeName), std::optional{"*ext4*"}); + + VERIFY_SUCCEEDED(m_defaultSession->DeleteVolume(volumeName.c_str())); + cleanup.release(); + ExpectMount(m_defaultSession.get(), std::format("/mnt/wslc-volumes/{}", volumeName), std::nullopt); VERIFY_IS_FALSE(std::filesystem::exists(volumeVhdPath)); + } - cleanup.release(); + WSLC_TEST_METHOD(NamedVolumesGuest) + { + ValidateNamedVolumeContract("guest", nullptr, 0); } - WSLC_TEST_METHOD(NamedVolumesSessionRecovery) + // Verifies that a container using a named volume survives a session restart and the volume's data is preserved. + void ValidateNamedVolumeRecoveryContract(std::string_view driver, const WSLCDriverOption* driverOpts, ULONG driverOptsCount) { - const std::string volumeName = "wslc-test-named-volume"; - const std::string containerName = "wslc-test-container"; - const std::filesystem::path volumeVhdPath = m_storagePath / "volumes" / (volumeName + ".vhdx"); + const std::string driverStr(driver); + const std::string volumeName = std::format("wslc-test-named-volume-{}", driver); + const std::string containerName = std::format("wslc-test-container-{}", driver); // Best-effort cleanup in case prior failed runs left artifacts behind. RunCommand(m_defaultSession.get(), {"/usr/bin/docker", "rm", "-f", containerName}); LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); - { - std::error_code ec; - std::filesystem::remove(volumeVhdPath, ec); - } auto cleanup = wil::scope_exit([&]() { RunCommand(m_defaultSession.get(), {"/usr/bin/docker", "rm", "-f", containerName}); LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); - - std::error_code ec; - std::filesystem::remove(volumeVhdPath, ec); }); WSLCVolumeOptions volumeOptions{}; volumeOptions.Name = volumeName.c_str(); - - WSLCDriverOption driverOpts[] = {{"SizeBytes", "1073741824"}}; + volumeOptions.Driver = driverStr.c_str(); volumeOptions.DriverOpts = driverOpts; - volumeOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + volumeOptions.DriverOptsCount = driverOptsCount; WSLCVolumeInformation volInfo{}; VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); - VERIFY_IS_TRUE(std::filesystem::exists(volumeVhdPath)); // Create a container that uses the named volume and writes a marker. { @@ -3576,15 +3605,55 @@ class WSLCTests // Verify the named volume still contains the marker after restart. { - WSLCContainerLauncher reader("debian:latest", "wslc-test-container-reader", {"/bin/sh", "-c", "cat /data/marker.txt"}); + WSLCContainerLauncher reader( + "debian:latest", std::format("{}-reader", containerName), {"/bin/sh", "-c", "cat /data/marker.txt"}); reader.AddNamedVolume(volumeName, "/data", true); auto readerContainer = reader.Launch(*m_defaultSession); auto readerProcess = readerContainer.GetInitProcess(); ValidateProcessOutput(readerProcess, {{1, "named-volume-recovery\n"}}); } + } + + WSLC_TEST_METHOD(NamedVolumeRecovery) + { + ValidateNamedVolumeRecoveryContract("guest", nullptr, 0); + } + + WSLC_TEST_METHOD(NamedVolumesVhdSessionRecovery) + { + WSLCDriverOption driverOpts[] = {{"SizeBytes", "1073741824"}}; + ValidateNamedVolumeRecoveryContract("vhd", driverOpts, ARRAYSIZE(driverOpts)); + + // Re-create the volume (the recovery helper cleans up on exit) so we + // can test the "delete VHD while session is down" scenario. + const std::string volumeName = "wslc-test-named-volume-vhd"; + const std::string containerName = "wslc-test-container-vhd"; + + WSLCVolumeOptions volumeOptions{}; + volumeOptions.Name = volumeName.c_str(); + volumeOptions.Driver = "vhd"; + volumeOptions.DriverOpts = driverOpts; + volumeOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + + WSLCVolumeInformation volInfo{}; + VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); + + // Create a container that depends on the volume so we can verify it + // gets dropped when the backing .vhdx is removed. + { + WSLCContainerLauncher writer("debian:latest", containerName, {"/bin/sh", "-c", "echo vhd-recovery >/data/marker.txt"}); + writer.AddNamedVolume(volumeName, "/data", false); + + auto writerContainer = writer.Launch(*m_defaultSession); + writerContainer.SetDeleteOnClose(false); + + auto writerProcess = writerContainer.GetInitProcess(); + ValidateProcessOutput(writerProcess, {}); + } + + const std::filesystem::path volumeVhdPath = m_storagePath / "volumes" / (volumeName + ".vhdx"); - // Stop the session, delete the backing VHD, and restart. { auto restartSession = ResetTestSession(); @@ -3602,7 +3671,86 @@ class WSLCTests VERIFY_ARE_EQUAL(m_defaultSession->DeleteVolume(volumeName.c_str()), WSLC_E_VOLUME_NOT_FOUND); } - WSLC_TEST_METHOD(NamedVolumeOptionsParseTest) + WSLC_TEST_METHOD(NamedVolumeGuestDriverOptsTest) + { + const std::string volumeName = "wslc-test-vol"; + LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); + auto cleanup = wil::scope_exit([&]() { LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); }); + + auto expectReject = [&](const WSLCDriverOption* opts, ULONG optsCount, const std::wstring& expectedMessage) { + LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); + + WSLCVolumeOptions volumeOptions{}; + volumeOptions.Name = volumeName.c_str(); + volumeOptions.Driver = "guest"; + volumeOptions.DriverOpts = opts; + volumeOptions.DriverOptsCount = optsCount; + + WSLCVolumeInformation volInfo{}; + VERIFY_ARE_EQUAL(m_defaultSession->CreateVolume(&volumeOptions, &volInfo), E_INVALIDARG); + ValidateCOMErrorMessageContains(expectedMessage); + }; + + auto expectAccept = [&](const WSLCDriverOption* opts, ULONG optsCount) { + LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName.c_str())); + + WSLCVolumeOptions volumeOptions{}; + volumeOptions.Name = volumeName.c_str(); + volumeOptions.Driver = "guest"; + volumeOptions.DriverOpts = opts; + volumeOptions.DriverOptsCount = optsCount; + + WSLCVolumeInformation volInfo{}; + VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); + }; + + // Allowed: no options (nullptr). + expectAccept(nullptr, 0); + + // Allowed: type=tmpfs with device=tmpfs. + { + WSLCDriverOption opts[] = {{"type", "tmpfs"}, {"device", "tmpfs"}}; + expectAccept(opts, ARRAYSIZE(opts)); + } + + // Allowed: type=tmpfs with device=tmpfs and o= suboptions. + { + WSLCDriverOption opts[] = {{"type", "tmpfs"}, {"device", "tmpfs"}, {"o", "size=100m,uid=1000"}}; + expectAccept(opts, ARRAYSIZE(opts)); + } + + // Blocked: type=none (bind mount). + { + WSLCDriverOption opts[] = {{"type", "none"}}; + expectReject(opts, ARRAYSIZE(opts), L"unsupported volume driver options: type=none"); + } + + // Blocked: type=nfs. + { + WSLCDriverOption opts[] = {{"type", "nfs"}}; + expectReject(opts, ARRAYSIZE(opts), L"unsupported volume driver options: type=nfs"); + } + + // Blocked by Docker: device without type. + { + WSLCDriverOption opts[] = {{"device", "/some/path"}}; + expectReject(opts, ARRAYSIZE(opts), L"create wslc-test-vol: missing required option: \"type\""); + } + + // Blocked by Docker: device=tmpfs without type. + { + WSLCDriverOption opts[] = {{"device", "tmpfs"}}; + expectReject(opts, ARRAYSIZE(opts), L"create wslc-test-vol: missing required option: \"type\""); + } + + // Blocked by Docker: device and o without type. + { + WSLCDriverOption opts[] = {{"device", "tmpfs"}, {"o", "size=100m"}}; + expectReject(opts, ARRAYSIZE(opts), L"create wslc-test-vol: missing required option: \"type\""); + } + } + + WSLC_TEST_METHOD(NamedVolumeVhdOptionsParseTest) { const std::string volumeName = "wslc-volume-name"; @@ -3616,6 +3764,7 @@ class WSLCTests WSLCVolumeOptions volumeOptions{}; volumeOptions.Name = volumeName.c_str(); + volumeOptions.Driver = "vhd"; volumeOptions.DriverOpts = opts; volumeOptions.DriverOptsCount = optsCount; @@ -3666,12 +3815,12 @@ class WSLCTests WSLC_TEST_METHOD(ListAndInspectNamedVolumesTest) { - const std::string volumeName1 = "wsla-test-vol1"; - const std::string volumeName2 = "wsla-test-vol2"; + const std::string vhdVolumeName = "wsla-test-vol-vhd"; + const std::string guestVolumeName = "wsla-test-vol-guest"; auto cleanup = wil::scope_exit([&]() { - LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName1.c_str())); - LOG_IF_FAILED(m_defaultSession->DeleteVolume(volumeName2.c_str())); + LOG_IF_FAILED(m_defaultSession->DeleteVolume(vhdVolumeName.c_str())); + LOG_IF_FAILED(m_defaultSession->DeleteVolume(guestVolumeName.c_str())); }); // Verify empty list is returned when no volumes exist. @@ -3679,57 +3828,85 @@ class WSLCTests VERIFY_SUCCEEDED(m_defaultSession->ListVolumes(volumes.addressof(), volumes.size_address())); VERIFY_ARE_EQUAL(0u, volumes.size()); - // Create first volume and verify list returns one entry. - WSLCVolumeOptions volumeOptions{}; - volumeOptions.Name = volumeName1.c_str(); - + // Create a VHD volume and verify list returns one entry. WSLCDriverOption driverOpts[] = {{"SizeBytes", "1073741824"}}; - volumeOptions.DriverOpts = driverOpts; - volumeOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + + WSLCVolumeOptions vhdOptions{}; + vhdOptions.Name = vhdVolumeName.c_str(); + vhdOptions.Driver = "vhd"; + vhdOptions.DriverOpts = driverOpts; + vhdOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + WSLCVolumeInformation volInfo{}; - VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); + VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&vhdOptions, &volInfo)); VERIFY_SUCCEEDED(m_defaultSession->ListVolumes(volumes.addressof(), volumes.size_address())); VERIFY_ARE_EQUAL(1u, volumes.size()); - VERIFY_ARE_EQUAL(std::string(volumes[0].Name), volumeName1); + VERIFY_ARE_EQUAL(std::string(volumes[0].Name), vhdVolumeName); VERIFY_ARE_EQUAL(std::string(volumes[0].Driver), std::string("vhd")); - // Create second volume and verify list returns two entries. - volumeOptions.Name = volumeName2.c_str(); - VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&volumeOptions, &volInfo)); + // Verify that a guest volume cannot be created with the same name as an existing vhd volume. + WSLCVolumeOptions duplicateGuestOptions{}; + duplicateGuestOptions.Name = vhdVolumeName.c_str(); + duplicateGuestOptions.Driver = "guest"; + VERIFY_ARE_EQUAL(m_defaultSession->CreateVolume(&duplicateGuestOptions, &volInfo), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS)); + + // Create a guest volume and verify both drivers show up in the list. + WSLCVolumeOptions guestOptions{}; + guestOptions.Name = guestVolumeName.c_str(); + guestOptions.Driver = "guest"; + VERIFY_SUCCEEDED(m_defaultSession->CreateVolume(&guestOptions, &volInfo)); + + // Verify that a vhd volume cannot be created with the same name as an existing guest volume. + WSLCVolumeOptions duplicateVhdOptions{}; + duplicateVhdOptions.Name = guestVolumeName.c_str(); + duplicateVhdOptions.Driver = "vhd"; + duplicateVhdOptions.DriverOpts = driverOpts; + duplicateVhdOptions.DriverOptsCount = ARRAYSIZE(driverOpts); + VERIFY_ARE_EQUAL(m_defaultSession->CreateVolume(&duplicateVhdOptions, &volInfo), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS)); VERIFY_SUCCEEDED(m_defaultSession->ListVolumes(volumes.addressof(), volumes.size_address())); VERIFY_ARE_EQUAL(2u, volumes.size()); - std::set names; + std::map namesToDrivers; for (const auto& v : volumes) { - names.insert(v.Name); - VERIFY_ARE_EQUAL(std::string(v.Driver), std::string("vhd")); + namesToDrivers.emplace(v.Name, v.Driver); } - VERIFY_IS_TRUE(names.contains(volumeName1)); - VERIFY_IS_TRUE(names.contains(volumeName2)); + VERIFY_ARE_EQUAL(namesToDrivers[vhdVolumeName], std::string("vhd")); + VERIFY_ARE_EQUAL(namesToDrivers[guestVolumeName], std::string("guest")); - // Verify InspectVolume returns correct details. + // Verify InspectVolume returns correct details for the VHD volume (driver opts present). wil::unique_cotaskmem_ansistring output; - VERIFY_SUCCEEDED(m_defaultSession->InspectVolume(volumeName1.c_str(), &output)); + VERIFY_SUCCEEDED(m_defaultSession->InspectVolume(vhdVolumeName.c_str(), &output)); + VERIFY_IS_NOT_NULL(output.get()); + + auto vhdInspect = wsl::shared::FromJson(output.get()); + VERIFY_ARE_EQUAL(vhdInspect.Name, vhdVolumeName); + VERIFY_ARE_EQUAL(vhdInspect.Driver, std::string("vhd")); + VERIFY_IS_TRUE(vhdInspect.DriverOpts.contains("SizeBytes")); + + // Verify InspectVolume returns correct details for the guest volume (no driver opts). + output.reset(); + VERIFY_SUCCEEDED(m_defaultSession->InspectVolume(guestVolumeName.c_str(), &output)); VERIFY_IS_NOT_NULL(output.get()); - auto inspect = wsl::shared::FromJson(output.get()); - VERIFY_ARE_EQUAL(inspect.Name, volumeName1); - VERIFY_ARE_EQUAL(inspect.Driver, std::string("vhd")); - VERIFY_IS_TRUE(inspect.DriverOpts.contains("SizeBytes")); + auto guestInspect = wsl::shared::FromJson(output.get()); + VERIFY_ARE_EQUAL(guestInspect.Name, guestVolumeName); + VERIFY_ARE_EQUAL(guestInspect.Driver, std::string("guest")); + VERIFY_IS_TRUE(guestInspect.DriverOpts.empty()); // Verify InspectVolume fails for a non-existent volume. output.reset(); VERIFY_ARE_EQUAL(m_defaultSession->InspectVolume("does-not-exist", &output), WSLC_E_VOLUME_NOT_FOUND); - // Delete first volume and verify list returns one entry. - VERIFY_SUCCEEDED(m_defaultSession->DeleteVolume(volumeName1.c_str())); + // Delete the VHD volume and verify only the guest volume remains. + VERIFY_SUCCEEDED(m_defaultSession->DeleteVolume(vhdVolumeName.c_str())); VERIFY_SUCCEEDED(m_defaultSession->ListVolumes(volumes.addressof(), volumes.size_address())); VERIFY_ARE_EQUAL(1u, volumes.size()); - VERIFY_ARE_EQUAL(std::string(volumes[0].Name), volumeName2); + VERIFY_ARE_EQUAL(std::string(volumes[0].Name), guestVolumeName); + VERIFY_ARE_EQUAL(std::string(volumes[0].Driver), std::string("guest")); } WSLC_TEST_METHOD(NetworkCreateDeleteListTest) From f5cb90e8671b5562f5e81afe9e6acaada44a15be Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 23 Apr 2026 16:48:09 -0700 Subject: [PATCH 03/11] Address feedback and fix formatting Co-authored-by: Copilot --- .../wslcsession/DockerEventTracker.cpp | 37 +++++++++++------ src/windows/wslcsession/DockerEventTracker.h | 7 +--- src/windows/wslcsession/WSLCContainer.cpp | 41 +++---------------- src/windows/wslcsession/WSLCContainer.h | 3 +- src/windows/wslcsession/WSLCSession.cpp | 11 ----- src/windows/wslcsession/WSLCSession.h | 4 -- src/windows/wslcsession/WSLCVolumes.cpp | 13 +++++- test/windows/WSLCTests.cpp | 8 +++- 8 files changed, 51 insertions(+), 73 deletions(-) diff --git a/src/windows/wslcsession/DockerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp index 02df0b27b..649f6319b 100644 --- a/src/windows/wslcsession/DockerEventTracker.cpp +++ b/src/windows/wslcsession/DockerEventTracker.cpp @@ -26,8 +26,7 @@ DockerEventTracker::EventTrackingReference::EventTrackingReference(DockerEventTr { } -DockerEventTracker::EventTrackingReference& DockerEventTracker::EventTrackingReference::operator=( - DockerEventTracker::EventTrackingReference&& other) noexcept +DockerEventTracker::EventTrackingReference& DockerEventTracker::EventTrackingReference::operator=(DockerEventTracker::EventTrackingReference&& other) noexcept { Reset(); m_id = other.m_id; @@ -61,8 +60,7 @@ DockerEventTracker::EventTrackingReference::~EventTrackingReference() noexcept Reset(); } -DockerEventTracker::DockerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay) : - m_sessionId(sessionId) +DockerEventTracker::DockerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay) : m_sessionId(sessionId) { auto onChunk = [this](const gsl::span& buffer) { if (!buffer.empty()) // docker inserts empty lines between events, skip those. @@ -105,12 +103,7 @@ void DockerEventTracker::OnEvent(const std::string_view& event) auto parsed = nlohmann::json::parse(event); auto action = parsed.find("Action"); - THROW_HR_IF_MSG( - E_INVALIDARG, - action == parsed.end(), - "Failed to parse json: %.*hs", - static_cast(event.size()), - event.data()); + THROW_HR_IF_MSG(E_INVALIDARG, action == parsed.end(), "Failed to parse json: %.*hs", static_cast(event.size()), event.data()); auto timeEntry = parsed.find("time"); THROW_HR_IF_MSG( @@ -151,10 +144,18 @@ void DockerEventTracker::OnContainerEvent(const nlohmann::json& parsed, const st { std::lock_guard lock{m_lock}; m_createdObjects.insert(containerId); - m_createdObjectCV.notify_all(); + m_objectStateChanged.notify_all(); return; } + // Track container destroy events for WaitForObjectDestroyed() and clean up create tracking. + if (action == "destroy") + { + std::lock_guard lock{m_lock}; + m_createdObjects.erase(containerId); + m_objectStateChanged.notify_all(); + } + auto it = events.find(action); if (it == events.end()) { @@ -223,13 +224,25 @@ void DockerEventTracker::WaitForObjectCreated(const std::string& ObjectId) std::unique_lock lock{m_lock}; THROW_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_TIMEOUT), - !m_createdObjectCV.wait_for(lock, c_timeout, [&]() { return m_createdObjects.contains(ObjectId); }), + !m_objectStateChanged.wait_for(lock, c_timeout, [&]() { return m_createdObjects.contains(ObjectId); }), "Timed out waiting for Docker create event for object '%hs'", ObjectId.c_str()); m_createdObjects.erase(ObjectId); } +void DockerEventTracker::WaitForObjectDestroyed(const std::string& ObjectId) +{ + constexpr auto c_timeout = std::chrono::seconds{60}; + + std::unique_lock lock{m_lock}; + THROW_HR_IF_MSG( + HRESULT_FROM_WIN32(ERROR_TIMEOUT), + !m_objectStateChanged.wait_for(lock, c_timeout, [&]() { return !m_createdObjects.contains(ObjectId); }), + "Timed out waiting for Docker destroy event for object '%hs'", + ObjectId.c_str()); +} + DockerEventTracker::EventTrackingReference DockerEventTracker::RegisterContainerStateUpdates( const std::string& ContainerId, ContainerStateChangeCallback&& Callback) noexcept { diff --git a/src/windows/wslcsession/DockerEventTracker.h b/src/windows/wslcsession/DockerEventTracker.h index b970ed773..d0f9f9ad8 100644 --- a/src/windows/wslcsession/DockerEventTracker.h +++ b/src/windows/wslcsession/DockerEventTracker.h @@ -73,10 +73,8 @@ class DockerEventTracker EventTrackingReference RegisterVolumeUpdates(VolumeEventCallback&& Callback) noexcept; void UnregisterCallback(size_t Id) noexcept; - // Blocks until the event stream has delivered a create event for the given object ID. - // Because Docker emits events in order, waiting for a container create event guarantees - // that all preceding volume create events have already been processed. void WaitForObjectCreated(const std::string& ObjectId); + void WaitForObjectDestroyed(const std::string& ObjectId); private: void OnEvent(const std::string_view& event); @@ -100,9 +98,8 @@ class DockerEventTracker std::vector m_containerCallbacks; std::vector m_volumeCallbacks; - // Tracks object IDs that have been seen via create events, used by WaitForObjectCreated(). std::unordered_set m_createdObjects; - std::condition_variable_any m_createdObjectCV; + std::condition_variable_any m_objectStateChanged; ULONG m_sessionId{}; std::recursive_mutex m_lock; diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 3ce3f977c..d5fd5e9bb 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -285,31 +285,6 @@ void ProcessNamedVolumes(const WSLCContainerOptions& containerOptions, WSLCVolum } } -std::unordered_set GetAnonymousVolumeNames( - const wsl::windows::common::docker_schema::InspectContainer& inspectData) -{ - // Build a set of all volumes mounted on the container. - std::unordered_set volumeNames; - for (const auto& mount : inspectData.Mounts) - { - if (mount.Type == "volume" && !mount.Name.empty()) - { - volumeNames.insert(mount.Name); - } - } - - // Remove any volumes that were user specified in the HostConfig.Mounts. - for (const auto& mount : inspectData.HostConfig.Mounts) - { - if (!mount.Source.empty() && mount.Type == "volume") - { - volumeNames.erase(mount.Source); - } - } - - return volumeNames; -} - } // namespace ContainerPortMapping::ContainerPortMapping(VMPortMapping&& VmMapping, uint16_t ContainerPort) : @@ -366,7 +341,6 @@ WSLCContainerImpl::WSLCContainerImpl( std::vector&& volumes, std::vector&& ports, std::map&& labels, - std::unordered_set&& anonymousVolumes, std::function&& onDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, @@ -382,7 +356,6 @@ WSLCContainerImpl::WSLCContainerImpl( m_networkingMode(NetworkMode), m_id(std::move(Id)), m_mountedVolumes(std::move(volumes)), - m_dockerVolumeNames(std::move(anonymousVolumes)), m_mappedPorts(std::move(ports)), m_labels(std::move(labels)), m_comWrapper(wil::MakeOrThrow(this, std::move(onDeleted))), @@ -793,9 +766,12 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::DeleteExclusiveLo } CATCH_AND_THROW_DOCKER_USER_ERROR("Failed to delete container '%hs'", m_id.c_str()); + // Wait for the container destroy event on the Docker event stream. + // Docker emits volume destroy events before the container destroy event, so once this returns + // we are guaranteed that all anonymous volumes deleted with the container have been removed from tracking. if (WI_IsFlagSet(Flags, WSLCDeleteFlagsDeleteVolumes)) { - m_wslcSession.DeleteContainerVolumes(m_dockerVolumeNames); + m_eventTracker.WaitForObjectDestroyed(m_id); } Transition(WslcContainerStateDeleted); @@ -1284,9 +1260,6 @@ std::unique_ptr WSLCContainerImpl::Create( // event tracker's volume callback and are already tracked in WSLCVolumes. EventTracker.WaitForObjectCreated(result.Id); - // Identify the anonymous volumes so we know which ones to delete with the container. - auto anonymousVolumes = GetAnonymousVolumeNames(inspectData); - auto container = std::make_unique( wslcSession, virtualMachine, @@ -1297,7 +1270,6 @@ std::unique_ptr WSLCContainerImpl::Create( std::move(volumeMounts), std::move(ports), std::move(labels), - std::move(anonymousVolumes), std::move(OnDeleted), EventTracker, DockerClient, @@ -1370,7 +1342,6 @@ std::unique_ptr WSLCContainerImpl::Open( std::move(metadata.Volumes), std::move(ports), std::move(labels), - std::unordered_set{}, std::move(OnDeleted), EventTracker, DockerClient, @@ -1380,7 +1351,7 @@ std::unique_ptr WSLCContainerImpl::Open( metadata.InitProcessFlags, metadata.Flags); - // Restore the state change timestamp and identify implicit volumes from Docker inspect data. + // Restore the state change timestamp from Docker inspect data. try { auto inspectData = DockerClient.InspectContainer(dockerContainer.Id); @@ -1391,8 +1362,6 @@ std::unique_ptr WSLCContainerImpl::Open( { container->m_stateChangedAt = ParseDockerTimestamp(timestamp); } - - container->m_dockerVolumeNames = GetAnonymousVolumeNames(inspectData); } CATCH_LOG(); diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index 0ae3182af..f432f6421 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -64,10 +64,10 @@ class WSLCContainerImpl std::vector&& volumes, std::vector&& ports, std::map&& labels, - std::unordered_set&& anonymousVolumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, + WSLCVolumes& Volumes, IORelay& Relay, WSLCContainerState InitialState, std::uint64_t CreatedAt, @@ -169,7 +169,6 @@ class WSLCContainerImpl WSLCVirtualMachine& m_virtualMachine; std::vector m_mappedPorts; std::vector m_mountedVolumes; - std::unordered_set m_dockerVolumeNames; std::map m_labels; Microsoft::WRL::ComPtr m_comWrapper; DockerEventTracker& m_eventTracker; diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 75ea74e56..7091e4fa0 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -2520,17 +2520,6 @@ void WSLCSession::OnContainerDeleted(const WSLCContainerImpl* Container) WI_VERIFY(std::erase_if(m_containers, [Container](const auto& e) { return e.get() == Container; }) == 1); } -void WSLCSession::DeleteContainerVolumes(const std::unordered_set& VolumeNames) -{ - if (m_volumes) - { - for (const auto& name : VolumeNames) - { - m_volumes->OnVolumeDeleted(name); - } - } -} - HRESULT WSLCSession::GetState(_Out_ WSLCSessionState* State) { *State = m_terminated ? WSLCSessionStateTerminated : WSLCSessionStateRunning; diff --git a/src/windows/wslcsession/WSLCSession.h b/src/windows/wslcsession/WSLCSession.h index 7c919fbf4..95f8f0b7f 100644 --- a/src/windows/wslcsession/WSLCSession.h +++ b/src/windows/wslcsession/WSLCSession.h @@ -150,13 +150,9 @@ class DECLSPEC_UUID("4877FEFC-4977-4929-A958-9F36AA1892A4") WSLCSession UserHandle OpenUserHandle(WSLCHandle Handle); void ReleaseUserHandle(HANDLE Handle); - - UserCOMCallback RegisterUserCOMCallback(); void UnregisterUserCOMCallback(DWORD ThreadId); - void DeleteContainerVolumes(_In_ const std::unordered_set& VolumeNames); - private: ULONG m_id = 0; diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp index 5fe287843..b983fafe8 100644 --- a/src/windows/wslcsession/WSLCVolumes.cpp +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -14,8 +14,7 @@ using wsl::windows::service::wslc::WSLCVolumes; namespace wsl::windows::service::wslc { WSLCVolumes::WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker) : - m_dockerClient(dockerClient), - m_virtualMachine(virtualMachine) + m_dockerClient(dockerClient), m_virtualMachine(virtualMachine) { // Recover existing volumes from Docker. for (const auto& volume : dockerClient.ListVolumes()) @@ -44,6 +43,14 @@ std::unique_ptr WSLCVolumes::OpenDockerVolume(const wsl::windows::c { opened = WSLCVhdVolumeImpl::Open(vol, m_virtualMachine, m_dockerClient); } + else if (metadata.Driver == WSLCGuestVolumeDriver) + { + opened = WSLCGuestVolumeImpl::Open(vol, m_dockerClient); + } + else + { + THROW_HR_MSG(E_UNEXPECTED, "Unrecognized WSLC volume driver: %hs", metadata.Driver.c_str()); + } } else if (vol.Driver == "local") { @@ -54,6 +61,8 @@ std::unique_ptr WSLCVolumes::OpenDockerVolume(const wsl::windows::c THROW_HR_MSG(E_UNEXPECTED, "Unrecognized volume driver: %hs", vol.Driver.c_str()); } + THROW_HR_IF_NULL_MSG(E_UNEXPECTED, opened, "Volume driver returned null for volume: %hs", vol.Name.c_str()); + WSL_LOG("VolumeOpened", TraceLoggingValue(vol.Name.c_str(), "VolumeName"), TraceLoggingValue(vol.Driver.c_str(), "Driver")); return opened; diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 387cf65e1..b881c7466 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -1985,7 +1985,7 @@ class WSLCTests // Clean up any leaked anonymous volumes when this block exits. auto volumeCleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { - auto volumes = listAnonymousVolumes(); + auto volumes = listAnonymousVolumes(); for (const auto& name : volumes) { LOG_IF_FAILED(m_defaultSession->DeleteVolume(name.c_str())); @@ -2023,6 +2023,12 @@ class WSLCTests VERIFY_ARE_EQUAL(listAnonymousVolumes().size(), 1u); VERIFY_SUCCEEDED(container.Get().Stop(WSLCSignalSIGKILL, 0)); + // Wait for the volume destroy event to be processed by the event tracker. + wsl::shared::retry::RetryWithTimeout( + [&]() { THROW_WIN32_IF(ERROR_RETRY, listAnonymousVolumes().size() != 0u); }, + std::chrono::milliseconds{100}, + std::chrono::seconds{10}); + VERIFY_ARE_EQUAL(listAnonymousVolumes().size(), 0u); } } From 35962d6731a610a98a058dd947a7293dfcf3738e Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 23 Apr 2026 17:29:05 -0700 Subject: [PATCH 04/11] refactor --- src/windows/wslcsession/IWSLCVolume.h | 6 +++ src/windows/wslcsession/WSLCContainer.cpp | 16 ++---- src/windows/wslcsession/WSLCContainer.h | 4 -- src/windows/wslcsession/WSLCSession.cpp | 11 ++--- src/windows/wslcsession/WSLCVhdVolume.h | 2 +- src/windows/wslcsession/WSLCVolumes.cpp | 49 +++++++------------ src/windows/wslcsession/WSLCVolumes.h | 15 ++---- .../wslc/e2e/WSLCE2EContainerRunTests.cpp | 8 ++- 8 files changed, 41 insertions(+), 70 deletions(-) diff --git a/src/windows/wslcsession/IWSLCVolume.h b/src/windows/wslcsession/IWSLCVolume.h index b49b24ec5..4d8e1629a 100644 --- a/src/windows/wslcsession/IWSLCVolume.h +++ b/src/windows/wslcsession/IWSLCVolume.h @@ -39,6 +39,12 @@ class IWSLCVolume // (e.g. detach/delete the VHD for VHD volumes). Throws on failure. virtual void Delete() = 0; + // Called when Docker has already destroyed the volume (e.g. container delete with -v). + // Releases any host-side resources without contacting Docker. Default is a no-op. + virtual void OnDeleted() + { + } + // Returns a JSON string for the COM-facing InspectVolume result. virtual std::string Inspect() const = 0; diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index d5fd5e9bb..c0f78546d 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -21,7 +21,6 @@ Module Name: #include "WSLCContainer.h" #include "WSLCProcess.h" #include "WSLCProcessIO.h" -#include "WSLCVolumes.h" using wsl::windows::common::COMServiceExecutionContext; using wsl::windows::common::docker_schema::ErrorResponse; @@ -32,7 +31,6 @@ using wsl::windows::common::relay::OverlappedIOHandle; using wsl::windows::common::relay::ReadHandle; using wsl::windows::common::relay::RelayHandle; using wsl::windows::service::wslc::ContainerPortMapping; -using wsl::windows::service::wslc::IWSLCVolume; using wsl::windows::service::wslc::RelayedProcessIO; using wsl::windows::service::wslc::TypedHandle; using wsl::windows::service::wslc::VMPortMapping; @@ -44,7 +42,6 @@ using wsl::windows::service::wslc::WSLCPortMapping; using wsl::windows::service::wslc::WSLCSession; using wsl::windows::service::wslc::WSLCVirtualMachine; using wsl::windows::service::wslc::WSLCVolumeMount; -using wsl::windows::service::wslc::WSLCVolumes; using namespace wsl::windows::common::relay; using namespace wsl::windows::common::docker_schema; @@ -260,7 +257,7 @@ std::string SerializeContainerMetadata(const WSLCContainerMetadataV1& metadata) return wsl::shared::ToJson(wrapper); } -void ProcessNamedVolumes(const WSLCContainerOptions& containerOptions, WSLCVolumes& volumes, wsl::windows::common::docker_schema::CreateContainer& request) +void ProcessNamedVolumes(const WSLCContainerOptions& containerOptions, wsl::windows::common::docker_schema::CreateContainer& request) { THROW_HR_IF(E_INVALIDARG, containerOptions.NamedVolumesCount > 0 && containerOptions.NamedVolumes == nullptr); @@ -270,13 +267,8 @@ void ProcessNamedVolumes(const WSLCContainerOptions& containerOptions, WSLCVolum THROW_HR_IF_NULL_MSG(E_INVALIDARG, nv.Name, "NamedVolume at index %lu has null Name", i); THROW_HR_IF_NULL_MSG(E_INVALIDARG, nv.ContainerPath, "NamedVolume at index %lu has null ContainerPath", i); - std::string volumeName = nv.Name; - - THROW_HR_WITH_USER_ERROR_IF( - WSLC_E_VOLUME_NOT_FOUND, Localization::MessageWslcVolumeNotFound(nv.Name), !volumes.ContainsVolume(volumeName)); - wsl::windows::common::docker_schema::Mount mount{}; - mount.Source = std::move(volumeName); + mount.Source = std::string(nv.Name); mount.Target = std::string(nv.ContainerPath); mount.Type = "volume"; mount.ReadOnly = static_cast(nv.ReadOnly); @@ -1028,7 +1020,6 @@ std::unique_ptr WSLCContainerImpl::Create( const WSLCContainerOptions& containerOptions, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - WSLCVolumes& volumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, @@ -1204,7 +1195,7 @@ std::unique_ptr WSLCContainerImpl::Create( } } - ProcessNamedVolumes(containerOptions, volumes, request); + ProcessNamedVolumes(containerOptions, request); // Process port mappings from container options. auto [ports, networkMode] = ProcessPortMappings(containerOptions, virtualMachine); @@ -1287,7 +1278,6 @@ std::unique_ptr WSLCContainerImpl::Open( const common::docker_schema::ContainerInfo& dockerContainer, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - WSLCVolumes& volumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index f432f6421..26209bb08 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -30,7 +30,6 @@ namespace wsl::windows::service::wslc { class WSLCContainer; class WSLCSession; -class WSLCVolumes; struct ContainerPortMapping { @@ -67,7 +66,6 @@ class WSLCContainerImpl std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, - WSLCVolumes& Volumes, IORelay& Relay, WSLCContainerState InitialState, std::uint64_t CreatedAt, @@ -114,7 +112,6 @@ class WSLCContainerImpl const WSLCContainerOptions& Options, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - WSLCVolumes& volumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, @@ -124,7 +121,6 @@ class WSLCContainerImpl const common::docker_schema::ContainerInfo& DockerContainer, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, - WSLCVolumes& volumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 7091e4fa0..89a182650 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -1593,7 +1593,6 @@ try *containerOptions, *this, m_virtualMachine.value(), - *m_volumes, std::bind(&WSLCSession::OnContainerDeleted, this, std::placeholders::_1), m_eventTracker.value(), m_dockerClient.value(), @@ -1881,8 +1880,6 @@ try RETURN_HR_IF_NULL(E_POINTER, VolumeInfo); ZeroMemory(VolumeInfo, sizeof(*VolumeInfo)); - std::string driver = (Options->Driver != nullptr && *Options->Driver != '\0') ? Options->Driver : WSLCVhdVolumeDriver; - auto driverOpts = wslutil::ParseKeyValuePairs(Options->DriverOpts, Options->DriverOptsCount); auto labels = wslutil::ParseKeyValuePairs(Options->Labels, Options->LabelsCount, WSLCVolumeMetadataLabel); @@ -1894,7 +1891,8 @@ try ValidateName(Options->Name, WSLC_MAX_VOLUME_NAME_LENGTH); } - *VolumeInfo = m_volumes->CreateVolume(Options->Name, driver, std::move(driverOpts), std::move(labels), m_storageVhdPath.parent_path()); + *VolumeInfo = + m_volumes->CreateVolume(Options->Name, Options->Driver, std::move(driverOpts), std::move(labels), m_storageVhdPath.parent_path()); return S_OK; } CATCH_RETURN(); @@ -1905,13 +1903,11 @@ try COMServiceExecutionContext context; RETURN_HR_IF_NULL(E_POINTER, Name); - std::string name = Name; - ValidateName(name.c_str(), WSLC_MAX_VOLUME_NAME_LENGTH); auto lock = m_lock.lock_shared(); THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_volumes); - m_volumes->DeleteVolume(name); + m_volumes->DeleteVolume(Name); return S_OK; } CATCH_RETURN(); @@ -2542,7 +2538,6 @@ void WSLCSession::RecoverExistingContainers() dockerContainer, *this, m_virtualMachine.value(), - *m_volumes, std::bind(&WSLCSession::OnContainerDeleted, this, std::placeholders::_1), m_eventTracker.value(), m_dockerClient.value(), diff --git a/src/windows/wslcsession/WSLCVhdVolume.h b/src/windows/wslcsession/WSLCVhdVolume.h index e0945224a..df0bfc1cb 100644 --- a/src/windows/wslcsession/WSLCVhdVolume.h +++ b/src/windows/wslcsession/WSLCVhdVolume.h @@ -78,7 +78,7 @@ class WSLCVhdVolumeImpl : public IWSLCVolume return m_virtualMachinePath; } - void OnDeleted(); + void OnDeleted() override; private: void Detach(); diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp index b983fafe8..60fa341dc 100644 --- a/src/windows/wslcsession/WSLCVolumes.cpp +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -21,7 +21,7 @@ WSLCVolumes::WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& vir { try { - m_volumes.insert({volume.Name, OpenDockerVolume(volume)}); + OpenVolumeExclusiveLockHeld(volume); } CATCH_LOG_MSG("Failed to recover volume: %hs", volume.Name.c_str()); } @@ -31,7 +31,7 @@ WSLCVolumes::WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& vir std::bind(&WSLCVolumes::OnVolumeEvent, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); } -std::unique_ptr WSLCVolumes::OpenDockerVolume(const wsl::windows::common::docker_schema::Volume& vol) +__requires_lock_held(m_lock) void WSLCVolumes::OpenVolumeExclusiveLockHeld(const wsl::windows::common::docker_schema::Volume& vol) { std::unique_ptr opened; @@ -43,14 +43,6 @@ std::unique_ptr WSLCVolumes::OpenDockerVolume(const wsl::windows::c { opened = WSLCVhdVolumeImpl::Open(vol, m_virtualMachine, m_dockerClient); } - else if (metadata.Driver == WSLCGuestVolumeDriver) - { - opened = WSLCGuestVolumeImpl::Open(vol, m_dockerClient); - } - else - { - THROW_HR_MSG(E_UNEXPECTED, "Unrecognized WSLC volume driver: %hs", metadata.Driver.c_str()); - } } else if (vol.Driver == "local") { @@ -61,11 +53,10 @@ std::unique_ptr WSLCVolumes::OpenDockerVolume(const wsl::windows::c THROW_HR_MSG(E_UNEXPECTED, "Unrecognized volume driver: %hs", vol.Driver.c_str()); } - THROW_HR_IF_NULL_MSG(E_UNEXPECTED, opened, "Volume driver returned null for volume: %hs", vol.Name.c_str()); - + WI_ASSERT(opened != nullptr); WSL_LOG("VolumeOpened", TraceLoggingValue(vol.Name.c_str(), "VolumeName"), TraceLoggingValue(vol.Driver.c_str(), "Driver")); - return opened; + m_volumes.insert({vol.Name, std::move(opened)}); } void WSLCVolumes::OnVolumeEvent(const std::string& volumeName, VolumeEvent event, std::uint64_t) @@ -81,11 +72,7 @@ void WSLCVolumes::OnVolumeEvent(const std::string& volumeName, VolumeEvent event } WSLCVolumeInformation WSLCVolumes::CreateVolume( - LPCSTR Name, - const std::string& Driver, - std::map&& DriverOpts, - std::map&& Labels, - const std::filesystem::path& StoragePath) + LPCSTR Name, LPCSTR Driver, std::map&& DriverOpts, std::map&& Labels, const std::filesystem::path& StoragePath) { auto lock = m_lock.lock_exclusive(); @@ -94,19 +81,20 @@ WSLCVolumeInformation WSLCVolumes::CreateVolume( THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), m_volumes.contains(Name)); } + std::string driver = (Driver != nullptr && Driver[0] != '\0') ? Driver : WSLCVhdVolumeDriver; std::unique_ptr volume; - if (Driver == WSLCVhdVolumeDriver) + if (driver == WSLCVhdVolumeDriver) { volume = WSLCVhdVolumeImpl::Create(Name, std::move(DriverOpts), std::move(Labels), StoragePath, m_virtualMachine, m_dockerClient); } - else if (Driver == WSLCGuestVolumeDriver) + else if (driver == WSLCGuestVolumeDriver) { volume = WSLCGuestVolumeImpl::Create(Name, std::move(DriverOpts), std::move(Labels), m_dockerClient); } else { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::MessageWslcInvalidVolumeType(Driver)); + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::MessageWslcInvalidVolumeType(driver)); } const auto& name = volume->Name(); @@ -115,13 +103,15 @@ WSLCVolumeInformation WSLCVolumes::CreateVolume( auto [it, inserted] = m_volumes.insert({name, std::move(volume)}); WI_VERIFY(inserted); - WSL_LOG("VolumeCreated", TraceLoggingValue(name.c_str(), "VolumeName"), TraceLoggingValue(Driver.c_str(), "Driver")); + WSL_LOG("VolumeCreated", TraceLoggingValue(name.c_str(), "VolumeName"), TraceLoggingValue(driver.c_str(), "Driver")); return info; } -void WSLCVolumes::DeleteVolume(const std::string& Name) +void WSLCVolumes::DeleteVolume(LPCSTR Name) { + THROW_HR_IF(E_POINTER, Name == nullptr); + auto lock = m_lock.lock_exclusive(); auto it = m_volumes.find(Name); @@ -130,7 +120,7 @@ void WSLCVolumes::DeleteVolume(const std::string& Name) it->second->Delete(); m_volumes.erase(it); - WSL_LOG("VolumeDeleted", TraceLoggingValue(Name.c_str(), "VolumeName")); + WSL_LOG("VolumeDeleted", TraceLoggingValue(Name, "VolumeName")); } std::vector WSLCVolumes::ListVolumes() const @@ -158,12 +148,6 @@ std::string WSLCVolumes::InspectVolume(const std::string& Name) const return it->second->Inspect(); } -bool WSLCVolumes::ContainsVolume(const std::string& Name) const -{ - auto lock = m_lock.lock_shared(); - return m_volumes.contains(Name); -} - void WSLCVolumes::OpenVolume(const std::string& VolumeName) { auto lock = m_lock.lock_exclusive(); @@ -176,7 +160,7 @@ void WSLCVolumes::OpenVolume(const std::string& VolumeName) try { auto vol = m_dockerClient.InspectVolume(VolumeName); - m_volumes.insert({VolumeName, OpenDockerVolume(vol)}); + OpenVolumeExclusiveLockHeld(vol); } CATCH_LOG_MSG("Failed to open volume: %hs", VolumeName.c_str()); } @@ -188,7 +172,8 @@ void WSLCVolumes::OnVolumeDeleted(const std::string& VolumeName) auto it = m_volumes.find(VolumeName); if (it != m_volumes.end()) { - WSL_LOG("VolumeRemovedWithContainer", TraceLoggingValue(VolumeName.c_str(), "VolumeName")); + it->second->OnDeleted(); + WSL_LOG("VolumeRemoved", TraceLoggingValue(VolumeName.c_str(), "VolumeName")); m_volumes.erase(it); } } diff --git a/src/windows/wslcsession/WSLCVolumes.h b/src/windows/wslcsession/WSLCVolumes.h index a750852e5..6fbd481bd 100644 --- a/src/windows/wslcsession/WSLCVolumes.h +++ b/src/windows/wslcsession/WSLCVolumes.h @@ -24,27 +24,22 @@ class WSLCVolumes WSLCVolumeInformation CreateVolume( _In_opt_ LPCSTR Name, - _In_ const std::string& Driver, + _In_opt_ LPCSTR Driver, _In_ std::map&& DriverOpts, _In_ std::map&& Labels, _In_ const std::filesystem::path& StoragePath); - void DeleteVolume(_In_ const std::string& Name); + void DeleteVolume(_In_ LPCSTR Name); std::vector ListVolumes() const; std::string InspectVolume(_In_ const std::string& Name) const; - bool ContainsVolume(_In_ const std::string& Name) const; - - // Opens a volume by name. Used to track volumes Docker implicitly created for VOLUME directives. - void OpenVolume(_In_ const std::string& VolumeName); - - // Removes a volume from tracking. Called when Docker deletes a volume (e.g. container delete with -v). - void OnVolumeDeleted(_In_ const std::string& VolumeName); private: - std::unique_ptr OpenDockerVolume(const wsl::windows::common::docker_schema::Volume& vol); + void OpenVolume(_In_ const std::string& VolumeName); + __requires_lock_held(m_lock) void OpenVolumeExclusiveLockHeld(const wsl::windows::common::docker_schema::Volume& vol); void OnVolumeEvent(const std::string& volumeName, VolumeEvent event, std::uint64_t eventTime); + void OnVolumeDeleted(_In_ const std::string& VolumeName); mutable wil::srwlock m_lock; _Guarded_by_(m_lock) std::unordered_map> m_volumes; diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index 916427cfe..4f51ffbf0 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -616,13 +616,17 @@ class WSLCE2EContainerRunTests result.Verify({.Stdout = L"WSLC Named Volume Test", .Stderr = L"", .ExitCode = 0}); } - WSLC_TEST_METHOD(WSLCE2E_Container_Run_Volume_NamedVolume_NotFound_Fail) + WSLC_TEST_METHOD(WSLCE2E_Container_Run_Volume_NamedVolume_AutoCreate) { auto result = RunWslc(std::format( L"container run --rm --volume {}:/data {} sh -c \"echo -n 'WSLC Named Volume Test' > /data/test.txt\"", WslcVolumeName, DebianImage.NameAndTag())); - result.Verify({.Stderr = std::format(L"Volume not found: '{}'\r\nError code: WSLC_E_VOLUME_NOT_FOUND\r\n", WslcVolumeName), .ExitCode = 1}); + result.Verify({.Stderr = L"", .ExitCode = 0}); + + // Verify the volume was auto-created by removing it (fails if it doesn't exist). + result = RunWslc(std::format(L"volume rm {}", WslcVolumeName)); + result.Verify({.Stderr = L"", .ExitCode = 0}); } private: From 6bc4123e7122ce9d05cd6cfbc69ba478e5d68ba0 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 23 Apr 2026 17:40:46 -0700 Subject: [PATCH 05/11] address comment --- src/windows/wslcsession/WSLCProcessControl.cpp | 8 ++++---- src/windows/wslcsession/WSLCProcessControl.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/windows/wslcsession/WSLCProcessControl.cpp b/src/windows/wslcsession/WSLCProcessControl.cpp index 158e47516..8b5af5536 100644 --- a/src/windows/wslcsession/WSLCProcessControl.cpp +++ b/src/windows/wslcsession/WSLCProcessControl.cpp @@ -44,7 +44,7 @@ const wil::unique_event& WSLCProcessControl::GetExitEvent() const DockerContainerProcessControl::DockerContainerProcessControl(WSLCContainerImpl& Container, DockerHTTPClient& DockerClient, DockerEventTracker& EventTracker) : m_container(&Container), m_client(DockerClient), - m_trackingReference(EventTracker.RegisterContainerStateUpdates( + m_eventTrackingReference(EventTracker.RegisterContainerStateUpdates( Container.ID(), std::bind(&DockerContainerProcessControl::OnEvent, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3))) { @@ -103,7 +103,7 @@ void DockerContainerProcessControl::OnContainerReleased() noexcept // N.B. The caller might keep a reference to the process even after the container is released. // If that happens, make sure that the state tracking can't outlive the session. // This is safe to call without the lock because removing the tracking reference is protected by the event tracker lock. - m_trackingReference.Reset(); + m_eventTrackingReference.Reset(); // Signal the exit event to prevent callers from being blocked on it. if (!m_exitEvent.is_signaled()) @@ -118,7 +118,7 @@ DockerExecProcessControl::DockerExecProcessControl( m_container(&Container), m_id(Id), m_client(DockerClient), - m_trackingReference(EventTracker.RegisterExecStateUpdates( + m_eventTrackingReference(EventTracker.RegisterExecStateUpdates( Container.ID(), Id, std::bind(&DockerExecProcessControl::OnEvent, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3))) { } @@ -197,7 +197,7 @@ void DockerExecProcessControl::OnContainerReleased() noexcept // If that happens, make sure that the state tracking can't outlive the session. // This is safe to call without the lock because removing the tracking reference is protected by the event tracker lock. - m_trackingReference.Reset(); + m_eventTrackingReference.Reset(); // Signal the exit event to prevent callers being blocked on it. if (!m_exitEvent.is_signaled()) diff --git a/src/windows/wslcsession/WSLCProcessControl.h b/src/windows/wslcsession/WSLCProcessControl.h index 8644a2d06..1d807075e 100644 --- a/src/windows/wslcsession/WSLCProcessControl.h +++ b/src/windows/wslcsession/WSLCProcessControl.h @@ -55,7 +55,7 @@ class DockerContainerProcessControl : public WSLCProcessControl std::mutex m_lock; DockerHTTPClient& m_client; WSLCContainerImpl* m_container{}; - DockerEventTracker::EventTrackingReference m_trackingReference; + DockerEventTracker::EventTrackingReference m_eventTrackingReference; }; class DockerExecProcessControl : public WSLCProcessControl @@ -79,7 +79,7 @@ class DockerExecProcessControl : public WSLCProcessControl std::optional m_pid{}; DockerHTTPClient& m_client; WSLCContainerImpl* m_container{}; - DockerEventTracker::EventTrackingReference m_trackingReference; + DockerEventTracker::EventTrackingReference m_eventTrackingReference; }; class VMProcessControl : public WSLCProcessControl From 081263fa08600ad9e4041ee99a1b310a0694397b Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 23 Apr 2026 18:37:42 -0700 Subject: [PATCH 06/11] Address feedback --- .../wslcsession/DockerEventTracker.cpp | 40 +++++++++------- src/windows/wslcsession/WSLCSession.cpp | 5 +- src/windows/wslcsession/WSLCVolumes.cpp | 48 +++++++++---------- src/windows/wslcsession/WSLCVolumes.h | 21 ++++++-- 4 files changed, 63 insertions(+), 51 deletions(-) diff --git a/src/windows/wslcsession/DockerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp index 649f6319b..4a968cc01 100644 --- a/src/windows/wslcsession/DockerEventTracker.cpp +++ b/src/windows/wslcsession/DockerEventTracker.cpp @@ -112,6 +112,29 @@ void DockerEventTracker::OnEvent(const std::string_view& event) auto actionStr = action->get(); + // Track object lifecycle for WaitForObjectCreated/WaitForObjectDestroyed. + auto actor = parsed.find("Actor"); + if (actor != parsed.end()) + { + auto id = actor->find("ID"); + if (id != actor->end()) + { + auto objectId = id->get(); + if (actionStr == "create") + { + std::lock_guard lock{m_lock}; + m_createdObjects.insert(objectId); + m_objectStateChanged.notify_all(); + } + else if (actionStr == "destroy") + { + std::lock_guard lock{m_lock}; + m_createdObjects.erase(objectId); + m_objectStateChanged.notify_all(); + } + } + } + // Route events by Type field. Docker uses "container", "volume", "network", etc. auto type = parsed.find("Type"); std::string typeStr = (type != parsed.end()) ? type->get() : "container"; @@ -139,23 +162,6 @@ void DockerEventTracker::OnContainerEvent(const nlohmann::json& parsed, const st auto containerId = id->get(); - // Track container create events for WaitForObjectCreated(). - if (action == "create") - { - std::lock_guard lock{m_lock}; - m_createdObjects.insert(containerId); - m_objectStateChanged.notify_all(); - return; - } - - // Track container destroy events for WaitForObjectDestroyed() and clean up create tracking. - if (action == "destroy") - { - std::lock_guard lock{m_lock}; - m_createdObjects.erase(containerId); - m_objectStateChanged.notify_all(); - } - auto it = events.find(action); if (it == events.end()) { diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index 89a182650..59dd6b928 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -260,7 +260,7 @@ try // Start the event tracker. m_eventTracker.emplace(m_dockerClient.value(), m_id, m_ioRelay); - m_volumes.emplace(m_dockerClient.value(), m_virtualMachine.value(), m_eventTracker.value()); + m_volumes.emplace(m_dockerClient.value(), m_virtualMachine.value(), m_eventTracker.value(), m_storageVhdPath.parent_path()); // Recover any existing resources from storage. RecoverExistingNetworks(); @@ -1891,8 +1891,7 @@ try ValidateName(Options->Name, WSLC_MAX_VOLUME_NAME_LENGTH); } - *VolumeInfo = - m_volumes->CreateVolume(Options->Name, Options->Driver, std::move(driverOpts), std::move(labels), m_storageVhdPath.parent_path()); + *VolumeInfo = m_volumes->CreateVolume(Options->Name, Options->Driver, std::move(driverOpts), std::move(labels)); return S_OK; } CATCH_RETURN(); diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp index 60fa341dc..01b9cdce2 100644 --- a/src/windows/wslcsession/WSLCVolumes.cpp +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -1,4 +1,16 @@ -// Copyright (C) Microsoft Corporation. All rights reserved. +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + WSLCVolumes.cpp + +Abstract: + + Contains the implementation of WSLCVolumes. + +--*/ #include "precomp.h" #include "WSLCVolumes.h" @@ -8,13 +20,12 @@ #include "docker_schema.h" using wsl::shared::Localization; -using wsl::windows::service::wslc::DockerHTTPClient; -using wsl::windows::service::wslc::WSLCVolumes; namespace wsl::windows::service::wslc { -WSLCVolumes::WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker) : - m_dockerClient(dockerClient), m_virtualMachine(virtualMachine) +WSLCVolumes::WSLCVolumes( + DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker, const std::filesystem::path& storagePath) : + m_dockerClient(dockerClient), m_virtualMachine(virtualMachine), m_storagePath(storagePath) { // Recover existing volumes from Docker. for (const auto& volume : dockerClient.ListVolumes()) @@ -33,7 +44,7 @@ WSLCVolumes::WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& vir __requires_lock_held(m_lock) void WSLCVolumes::OpenVolumeExclusiveLockHeld(const wsl::windows::common::docker_schema::Volume& vol) { - std::unique_ptr opened; + THROW_HR_IF_MSG(E_UNEXPECTED, vol.Driver != "local", "Unrecognized volume driver: %hs", vol.Driver.c_str()); if (vol.Labels.has_value() && vol.Labels->contains(WSLCVolumeMetadataLabel)) { @@ -41,22 +52,12 @@ __requires_lock_held(m_lock) void WSLCVolumes::OpenVolumeExclusiveLockHeld(const if (metadata.Driver == WSLCVhdVolumeDriver) { - opened = WSLCVhdVolumeImpl::Open(vol, m_virtualMachine, m_dockerClient); + m_volumes.insert({vol.Name, WSLCVhdVolumeImpl::Open(vol, m_virtualMachine, m_dockerClient)}); + return; } } - else if (vol.Driver == "local") - { - opened = WSLCGuestVolumeImpl::Open(vol, m_dockerClient); - } - else - { - THROW_HR_MSG(E_UNEXPECTED, "Unrecognized volume driver: %hs", vol.Driver.c_str()); - } - WI_ASSERT(opened != nullptr); - WSL_LOG("VolumeOpened", TraceLoggingValue(vol.Name.c_str(), "VolumeName"), TraceLoggingValue(vol.Driver.c_str(), "Driver")); - - m_volumes.insert({vol.Name, std::move(opened)}); + m_volumes.insert({vol.Name, WSLCGuestVolumeImpl::Open(vol, m_dockerClient)}); } void WSLCVolumes::OnVolumeEvent(const std::string& volumeName, VolumeEvent event, std::uint64_t) @@ -72,7 +73,7 @@ void WSLCVolumes::OnVolumeEvent(const std::string& volumeName, VolumeEvent event } WSLCVolumeInformation WSLCVolumes::CreateVolume( - LPCSTR Name, LPCSTR Driver, std::map&& DriverOpts, std::map&& Labels, const std::filesystem::path& StoragePath) + LPCSTR Name, LPCSTR Driver, std::map&& DriverOpts, std::map&& Labels) { auto lock = m_lock.lock_exclusive(); @@ -86,7 +87,7 @@ WSLCVolumeInformation WSLCVolumes::CreateVolume( if (driver == WSLCVhdVolumeDriver) { - volume = WSLCVhdVolumeImpl::Create(Name, std::move(DriverOpts), std::move(Labels), StoragePath, m_virtualMachine, m_dockerClient); + volume = WSLCVhdVolumeImpl::Create(Name, std::move(DriverOpts), std::move(Labels), m_storagePath, m_virtualMachine, m_dockerClient); } else if (driver == WSLCGuestVolumeDriver) { @@ -103,8 +104,6 @@ WSLCVolumeInformation WSLCVolumes::CreateVolume( auto [it, inserted] = m_volumes.insert({name, std::move(volume)}); WI_VERIFY(inserted); - WSL_LOG("VolumeCreated", TraceLoggingValue(name.c_str(), "VolumeName"), TraceLoggingValue(driver.c_str(), "Driver")); - return info; } @@ -119,8 +118,6 @@ void WSLCVolumes::DeleteVolume(LPCSTR Name) it->second->Delete(); m_volumes.erase(it); - - WSL_LOG("VolumeDeleted", TraceLoggingValue(Name, "VolumeName")); } std::vector WSLCVolumes::ListVolumes() const @@ -173,7 +170,6 @@ void WSLCVolumes::OnVolumeDeleted(const std::string& VolumeName) if (it != m_volumes.end()) { it->second->OnDeleted(); - WSL_LOG("VolumeRemoved", TraceLoggingValue(VolumeName.c_str(), "VolumeName")); m_volumes.erase(it); } } diff --git a/src/windows/wslcsession/WSLCVolumes.h b/src/windows/wslcsession/WSLCVolumes.h index 6fbd481bd..c3a5ca275 100644 --- a/src/windows/wslcsession/WSLCVolumes.h +++ b/src/windows/wslcsession/WSLCVolumes.h @@ -1,4 +1,16 @@ -// Copyright (C) Microsoft Corporation. All rights reserved. +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + WSLCVolumes.h + +Abstract: + + Contains the definition for WSLCVolumes. + +--*/ #pragma once @@ -7,7 +19,6 @@ #include "DockerHTTPClient.h" #include "DockerEventTracker.h" #include -#include namespace wsl::windows::service::wslc { @@ -19,15 +30,14 @@ class WSLCVolumes NON_COPYABLE(WSLCVolumes); NON_MOVABLE(WSLCVolumes); - WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker); + WSLCVolumes(DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker, const std::filesystem::path& storagePath); ~WSLCVolumes() = default; WSLCVolumeInformation CreateVolume( _In_opt_ LPCSTR Name, _In_opt_ LPCSTR Driver, _In_ std::map&& DriverOpts, - _In_ std::map&& Labels, - _In_ const std::filesystem::path& StoragePath); + _In_ std::map&& Labels); void DeleteVolume(_In_ LPCSTR Name); @@ -46,6 +56,7 @@ class WSLCVolumes DockerHTTPClient& m_dockerClient; WSLCVirtualMachine& m_virtualMachine; + std::filesystem::path m_storagePath; DockerEventTracker::EventTrackingReference m_volumeEventTracking; }; From 94f4bd5e30ec40f5998add57169597f48b16bbb9 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 24 Apr 2026 00:48:05 -0700 Subject: [PATCH 07/11] Address copilot feedback Co-authored-by: Copilot --- src/windows/wslcsession/DockerEventTracker.cpp | 2 -- src/windows/wslcsession/DockerEventTracker.h | 2 -- src/windows/wslcsession/WSLCVolumes.cpp | 1 + 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/windows/wslcsession/DockerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp index 4a968cc01..a56673ddb 100644 --- a/src/windows/wslcsession/DockerEventTracker.cpp +++ b/src/windows/wslcsession/DockerEventTracker.cpp @@ -233,8 +233,6 @@ void DockerEventTracker::WaitForObjectCreated(const std::string& ObjectId) !m_objectStateChanged.wait_for(lock, c_timeout, [&]() { return m_createdObjects.contains(ObjectId); }), "Timed out waiting for Docker create event for object '%hs'", ObjectId.c_str()); - - m_createdObjects.erase(ObjectId); } void DockerEventTracker::WaitForObjectDestroyed(const std::string& ObjectId) diff --git a/src/windows/wslcsession/DockerEventTracker.h b/src/windows/wslcsession/DockerEventTracker.h index d0f9f9ad8..fe0719b8e 100644 --- a/src/windows/wslcsession/DockerEventTracker.h +++ b/src/windows/wslcsession/DockerEventTracker.h @@ -66,8 +66,6 @@ class DockerEventTracker DockerEventTracker(DockerHTTPClient& dockerClient, ULONG sessionId, IORelay& relay); ~DockerEventTracker(); - void Stop(); - EventTrackingReference RegisterContainerStateUpdates(const std::string& ContainerId, ContainerStateChangeCallback&& Callback) noexcept; EventTrackingReference RegisterExecStateUpdates(const std::string& ContainerId, const std::string& ExecId, ContainerStateChangeCallback&& Callback) noexcept; EventTrackingReference RegisterVolumeUpdates(VolumeEventCallback&& Callback) noexcept; diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp index 01b9cdce2..63d2d2a75 100644 --- a/src/windows/wslcsession/WSLCVolumes.cpp +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -28,6 +28,7 @@ WSLCVolumes::WSLCVolumes( m_dockerClient(dockerClient), m_virtualMachine(virtualMachine), m_storagePath(storagePath) { // Recover existing volumes from Docker. + auto lock = m_lock.lock_exclusive(); for (const auto& volume : dockerClient.ListVolumes()) { try From 1e5f70987d2ae16ef4e16d4da081674b6cdeffb6 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 24 Apr 2026 14:16:17 -0700 Subject: [PATCH 08/11] FIX ABBA deadlock Co-authored-by: Copilot --- src/windows/wslcsession/WSLCContainer.cpp | 25 +++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index d197816ac..228fe2c47 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -804,10 +804,21 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) { - // Acquire an exclusive lock since this method modifies m_state. - auto lock = m_lock.lock_exclusive(); + { + // Acquire an exclusive lock since this method modifies m_state. + auto lock = m_lock.lock_exclusive(); + + DeleteExclusiveLockHeld(Flags); + } - DeleteExclusiveLockHeld(Flags); + // Wait for the container destroy event on the Docker event stream after releasing m_lock. + // Docker emits volume destroy events before the container destroy event, so once this returns + // we are guaranteed that all anonymous volumes deleted with the container have been removed from tracking. + // N.B. This must be done outside m_lock to avoid an ABBA deadlock with the relay thread. + if (WI_IsFlagSet(Flags, WSLCDeleteFlagsDeleteVolumes)) + { + m_eventTracker.WaitForObjectDestroyed(m_id); + } } __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::DeleteExclusiveLockHeld(WSLCDeleteFlags Flags) @@ -829,14 +840,6 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::DeleteExclusiveLo } CATCH_AND_THROW_DOCKER_USER_ERROR("Failed to delete container '%hs'", m_id.c_str()); - // Wait for the container destroy event on the Docker event stream. - // Docker emits volume destroy events before the container destroy event, so once this returns - // we are guaranteed that all anonymous volumes deleted with the container have been removed from tracking. - if (WI_IsFlagSet(Flags, WSLCDeleteFlagsDeleteVolumes)) - { - m_eventTracker.WaitForObjectDestroyed(m_id); - } - Transition(WslcContainerStateDeleted); ReleaseResources(); } From cfc4992e1d3185cfd8593f8f1fd9f5c94c8c8a52 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 24 Apr 2026 14:44:27 -0700 Subject: [PATCH 09/11] Address feedback Co-authored-by: Copilot --- src/windows/wslcsession/WSLCVolumes.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp index 63d2d2a75..5dc47ce5f 100644 --- a/src/windows/wslcsession/WSLCVolumes.cpp +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -27,8 +27,14 @@ WSLCVolumes::WSLCVolumes( DockerHTTPClient& dockerClient, WSLCVirtualMachine& virtualMachine, DockerEventTracker& eventTracker, const std::filesystem::path& storagePath) : m_dockerClient(dockerClient), m_virtualMachine(virtualMachine), m_storagePath(storagePath) { - // Recover existing volumes from Docker. + // Hold m_lock exclusively across both callback registration and the recovery loop. + // This ensures any volume events that arrive while recovering are queued behind us in OnVolumeEvent, + // and dedup naturally against entries inserted by recovery (insert is a no-op for existing keys). auto lock = m_lock.lock_exclusive(); + + m_volumeEventTracking = eventTracker.RegisterVolumeUpdates( + std::bind(&WSLCVolumes::OnVolumeEvent, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); + for (const auto& volume : dockerClient.ListVolumes()) { try @@ -37,10 +43,6 @@ WSLCVolumes::WSLCVolumes( } CATCH_LOG_MSG("Failed to recover volume: %hs", volume.Name.c_str()); } - - // Register for volume events after recovery is complete. - m_volumeEventTracking = eventTracker.RegisterVolumeUpdates( - std::bind(&WSLCVolumes::OnVolumeEvent, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); } __requires_lock_held(m_lock) void WSLCVolumes::OpenVolumeExclusiveLockHeld(const wsl::windows::common::docker_schema::Volume& vol) @@ -83,7 +85,7 @@ WSLCVolumeInformation WSLCVolumes::CreateVolume( THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), m_volumes.contains(Name)); } - std::string driver = (Driver != nullptr && Driver[0] != '\0') ? Driver : WSLCVhdVolumeDriver; + std::string driver = (Driver != nullptr && Driver[0] != '\0') ? Driver : WSLCGuestVolumeDriver; std::unique_ptr volume; if (driver == WSLCVhdVolumeDriver) From 71c9dfdb69e23f5eaea83eb5a93eda7375bcd4ef Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 24 Apr 2026 22:47:57 -0700 Subject: [PATCH 10/11] Validate opened containers Co-authored-by: Copilot --- src/windows/wslcsession/WSLCContainer.cpp | 18 ++++++++++++++++++ src/windows/wslcsession/WSLCContainer.h | 2 ++ src/windows/wslcsession/WSLCSession.cpp | 1 + src/windows/wslcsession/WSLCVolumes.cpp | 6 ++++++ src/windows/wslcsession/WSLCVolumes.h | 2 ++ 5 files changed, 29 insertions(+) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 228fe2c47..c3c1cf7b3 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -21,6 +21,7 @@ Module Name: #include "WSLCContainer.h" #include "WSLCProcess.h" #include "WSLCProcessIO.h" +#include "WSLCVolumes.h" using wsl::windows::common::COMServiceExecutionContext; using wsl::windows::common::docker_schema::ErrorResponse; @@ -1395,6 +1396,7 @@ std::unique_ptr WSLCContainerImpl::Open( const common::docker_schema::ContainerInfo& dockerContainer, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, + WSLCVolumes& volumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, @@ -1403,6 +1405,22 @@ std::unique_ptr WSLCContainerImpl::Open( // Extract container name from Docker's names list. std::string name = ExtractContainerName(dockerContainer.Names, dockerContainer.Id); + // Validate that all named volumes mounted by the container were successfully recovered + // by the volumes manager. If any are missing (e.g. backing VHD removed while the service was + // down), refuse to open the container so it cannot enter a broken state. + for (const auto& mount : dockerContainer.Mounts) + { + if (mount.Type == "volume" && !mount.Name.empty()) + { + THROW_HR_IF_MSG( + E_INVALIDARG, + !volumes.ContainsVolume(mount.Name), + "Cannot open container %hs: referenced volume '%hs' is not available", + dockerContainer.Id.c_str(), + mount.Name.c_str()); + } + } + auto labels(dockerContainer.Labels); auto metadataIt = labels.find(WSLCContainerMetadataLabel); diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index 26209bb08..d48b072b6 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -30,6 +30,7 @@ namespace wsl::windows::service::wslc { class WSLCContainer; class WSLCSession; +class WSLCVolumes; struct ContainerPortMapping { @@ -121,6 +122,7 @@ class WSLCContainerImpl const common::docker_schema::ContainerInfo& DockerContainer, WSLCSession& wslcSession, WSLCVirtualMachine& virtualMachine, + WSLCVolumes& Volumes, std::function&& OnDeleted, DockerEventTracker& EventTracker, DockerHTTPClient& DockerClient, diff --git a/src/windows/wslcsession/WSLCSession.cpp b/src/windows/wslcsession/WSLCSession.cpp index a457d7d22..17452e413 100644 --- a/src/windows/wslcsession/WSLCSession.cpp +++ b/src/windows/wslcsession/WSLCSession.cpp @@ -2543,6 +2543,7 @@ void WSLCSession::RecoverExistingContainers() dockerContainer, *this, m_virtualMachine.value(), + *m_volumes, std::bind(&WSLCSession::OnContainerDeleted, this, std::placeholders::_1), m_eventTracker.value(), m_dockerClient.value(), diff --git a/src/windows/wslcsession/WSLCVolumes.cpp b/src/windows/wslcsession/WSLCVolumes.cpp index 5dc47ce5f..4fff4dc4a 100644 --- a/src/windows/wslcsession/WSLCVolumes.cpp +++ b/src/windows/wslcsession/WSLCVolumes.cpp @@ -148,6 +148,12 @@ std::string WSLCVolumes::InspectVolume(const std::string& Name) const return it->second->Inspect(); } +bool WSLCVolumes::ContainsVolume(const std::string& Name) const +{ + auto lock = m_lock.lock_shared(); + return m_volumes.contains(Name); +} + void WSLCVolumes::OpenVolume(const std::string& VolumeName) { auto lock = m_lock.lock_exclusive(); diff --git a/src/windows/wslcsession/WSLCVolumes.h b/src/windows/wslcsession/WSLCVolumes.h index c3a5ca275..2f2996aab 100644 --- a/src/windows/wslcsession/WSLCVolumes.h +++ b/src/windows/wslcsession/WSLCVolumes.h @@ -44,6 +44,8 @@ class WSLCVolumes std::vector ListVolumes() const; std::string InspectVolume(_In_ const std::string& Name) const; + bool ContainsVolume(_In_ const std::string& Name) const; + private: void OpenVolume(_In_ const std::string& VolumeName); __requires_lock_held(m_lock) void OpenVolumeExclusiveLockHeld(const wsl::windows::common::docker_schema::Volume& vol); From 1a6181ac17c2a26215dee8929e66b62510aa1265 Mon Sep 17 00:00:00 2001 From: Kevin Vega <40717198+kvega005@users.noreply.github.com> Date: Fri, 24 Apr 2026 23:23:42 -0700 Subject: [PATCH 11/11] Update src/windows/wslcsession/DockerEventTracker.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/windows/wslcsession/DockerEventTracker.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/windows/wslcsession/DockerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp index a56673ddb..64d836239 100644 --- a/src/windows/wslcsession/DockerEventTracker.cpp +++ b/src/windows/wslcsession/DockerEventTracker.cpp @@ -16,7 +16,6 @@ Module Name: #include "WSLCVirtualMachine.h" #include -using wsl::windows::common::relay::MultiHandleWait; using wsl::windows::service::wslc::DockerEventTracker; using wsl::windows::service::wslc::DockerHTTPClient; using wsl::windows::service::wslc::WSLCVirtualMachine;