Refactor + fix locking story

src/windows/service/exe/IVirtualMachineBackend.h

@@ -180,6 +180,13 @@ struct VmGuestListener
     GuestServicePort Port;
 };
 
+struct VmGuestListenerState
+{
+    VmGuestListener Listener;
+    wil::unique_socket Socket;
+    wil::unique_event CancellationEvent{wil::EventOptions::ManualReset};
+};
+
 struct VmProcessorRequest
 {
     std::uint32_t Count = 0;
@@ -518,8 +525,45 @@ public:
     virtual VmNetworkAttachment AddNetworkAdapter(const VmNetworkAdapterRequest& Request) = 0;
     virtual VmPortBinding BindPort(VmDeviceId Device, const VmPortBindingRequest& Request) = 0;
     virtual void UnbindPort(VmPortBindingId Binding) = 0;
+
+
+protected:
+    // Protects all mutable backend state, including the base listener registry. Callers must
+    // release it before performing blocking I/O or waiting for a callback.
+    mutable wil::srwlock m_lock;
+
+    // These helpers require m_lock to be held exclusively. ConfigureGuestListener runs while
+    // m_lock is held and must not re-enter another listener helper.
+    VmGuestListener RegisterGuestListenerLocked(const VmInstanceId& Identity, GuestServicePort Port);
+    wil::unique_socket AcceptGuestListenerConnection(VmListenerId Listener, const VmInstanceId& Identity) const;
+    std::shared_ptr<VmGuestListenerState> RemoveGuestListenerLocked(VmListenerId Listener, const VmInstanceId& Identity);
+    void CloseGuestListenersLocked(const VmInstanceId& Identity) noexcept;
+
+private:
+    virtual std::shared_ptr<VmGuestListenerState> ConfigureGuestListener(const VmGuestListener& Listener);
+
+    _Guarded_by_(m_lock) std::map<std::uint64_t, std::shared_ptr<VmGuestListenerState>> m_guestListeners;
+    _Guarded_by_(m_lock) std::uint64_t m_nextListenerId = 1;
 };
 
 VmPlatformCapabilities QueryVirtualMachineBackendCapabilities(BackendKind Kind);
 
-std::unique_ptr<IVirtualMachineBackend> CreateVirtualMachineBackend(BackendKind Kind, const VmCreateRequest& Request);
+std::unique_ptr<IVirtualMachineBackend> CreateVirtualMachineBackend(BackendKind Kind, const VmCreateRequest& Request);
+
+namespace wsl::windows::common::vm::validation {
+
+void ValidateFeature(VmFeatureRequest Request, PCWSTR Setting);
+void ValidateUnsupportedSelection(VmSelectionPolicy Policy);
+void ValidatePath(const std::filesystem::path& Path, PCWSTR Backend);
+const VmVirtualDiskSource& ValidateDiskRequest(const VmDiskRequest& Request, UINT32 MaximumDisks);
+void ValidateConsolePath(const std::filesystem::path& Path, PCWSTR Backend, HRESULT Error, bool RequireName);
+void ValidateName(std::wstring_view Name, PCWSTR Description);
+void ValidateResourceId(UINT64 Value, const GUID& VmId, const VmInstanceId& Owner);
+
+template <typename Tag>
+void ValidateResourceId(const VmResourceId<Tag>& Id, const VmInstanceId& Owner)
+{
+    ValidateResourceId(Id.Value, Id.Owner.VmId, Owner);
+}
+
+}

src/windows/service/exe/OpenVmmVirtualMachineBackend.cpp

@@ -23,6 +23,8 @@ Abstract:
 using wsl::windows::common::Context;
 using wsl::windows::common::ExecutionContext;
 
+namespace validation = wsl::windows::common::vm::validation;
+
 namespace {
 
 constexpr UINT64 c_memoryGranularity = 2ULL * 1024 * 1024;
@@ -38,53 +40,6 @@ void DestroyConfig(WslOpenVmmConfig* Config) noexcept
 
 using UniqueConfig = wil::unique_any<WslOpenVmmConfig*, decltype(&DestroyConfig), DestroyConfig>;
 
-void ValidateFeature(VmFeatureRequest Request, PCWSTR Setting)
-{
-    switch (Request)
-    {
-    case VmFeatureRequest::Disabled:
-    case VmFeatureRequest::Preferred:
-        return;
-    case VmFeatureRequest::Required:
-        THROW_HR_MSG(c_notSupported, "OpenVMM does not support the required %ls setting", Setting);
-    }
-
-    THROW_HR(E_INVALIDARG);
-}
-
-void ValidatePath(const std::filesystem::path& Path)
-{
-    THROW_HR_IF_MSG(
-        E_INVALIDARG,
-        Path.empty() || !Path.is_absolute() || Path.native().find(L'\0') != std::wstring::npos,
-        "OpenVMM requires an absolute, nonempty host path");
-}
-
-const VmVirtualDiskSource& ValidateDiskRequest(const VmDiskRequest& Request)
-{
-    const auto* source = std::get_if<VmVirtualDiskSource>(&Request.Source);
-    THROW_HR_IF(c_notSupported, source == nullptr);
-    ValidatePath(source->Path);
-    switch (source->Format)
-    {
-    case VmDiskFormat::Vhd:
-        THROW_HR_IF(E_INVALIDARG, _wcsicmp(source->Path.extension().c_str(), L".vhd") != 0);
-        break;
-    case VmDiskFormat::Vhdx:
-        THROW_HR_IF(E_INVALIDARG, _wcsicmp(source->Path.extension().c_str(), L".vhdx") != 0);
-        break;
-    default:
-        THROW_HR(E_INVALIDARG);
-    }
-
-    if (Request.Placement)
-    {
-        THROW_HR_IF(c_notSupported, Request.Placement->Address.Controller != 0 || Request.Placement->Address.Lun >= c_maximumDisks);
-    }
-
-    return *source;
-}
-
 wil::unique_hfile OpenBackingFile(const std::filesystem::path& Path, bool ReadOnly)
 {
     wil::unique_hfile file{CreateFileW(
@@ -98,13 +53,6 @@ wil::unique_hfile OpenBackingFile(const std::filesystem::path& Path, bool ReadOn
     return file;
 }
 
-void ValidateConsolePath(const std::filesystem::path& Path)
-{
-    ValidatePath(Path);
-    THROW_HR_IF_MSG(
-        c_notSupported, !Path.native().starts_with(L"\\\\.\\pipe\\"), "OpenVMM consoles require a caller-provided named pipe");
-}
-
 void DeleteOwnedFile(const std::filesystem::path& Path) noexcept
 {
     if (!Path.empty() && !DeleteFileW(Path.c_str()))
@@ -122,12 +70,6 @@ std::filesystem::path GetVsockListenerPath(const std::filesystem::path& VsockPat
     return std::format(L"{}_{:08x}-facb-11e6-bd58-64006a7986d3", VsockPath.native(), Port.Value);
 }
 
-void ValidateName(std::wstring_view Name, PCWSTR Description)
-{
-    THROW_HR_IF_MSG(
-        E_INVALIDARG, Name.empty() || Name.find(L'\0') != std::wstring_view::npos, "%ls must be nonempty and cannot contain NUL", Description);
-}
-
 void ValidateNetworkConfiguration(const VmUserModeNatNetwork& Configuration)
 {
     // The current C ABI accepts a NIC ID and MAC, but cannot override Consomme's network defaults.
@@ -163,12 +105,6 @@ std::wstring FormatIpAddress(const VmIpAddress& Address)
     return result;
 }
 
-template <typename Tag>
-void ValidateResourceId(const VmResourceId<Tag>& Id, const VmInstanceId& Owner)
-{
-    THROW_HR_IF(E_INVALIDARG, Id.Value == 0 || !IsEqualGUID(Id.Owner.VmId, Owner.VmId));
-}
-
 } // namespace
 
 OpenVmmVirtualMachineBackend::GuestListener::~GuestListener() noexcept
@@ -177,27 +113,13 @@ OpenVmmVirtualMachineBackend::GuestListener::~GuestListener() noexcept
     DeleteOwnedFile(Path);
 }
 
-void OpenVmmVirtualMachineBackend::CloseGuestListeners() noexcept
-{
-    WSL_LOG(
-        "OpenVmmCloseGuestListeners",
-        TraceLoggingValue(m_description.Identity.VmId, "vmId"),
-        TraceLoggingValue(m_guestListeners.size(), "listenerCount"));
-    for (const auto& entry : m_guestListeners)
-    {
-        LOG_IF_WIN32_BOOL_FALSE(SetEvent(entry.second->CancellationEvent.get()));
-    }
-
-    m_guestListeners.clear();
-}
-
 VmDescription wsl::windows::common::vm::openvmm::ValidateCreateRequest(const VmCreateRequest& Request)
 {
     THROW_HR_IF(E_INVALIDARG, IsEqualGUID(Request.VmId, GUID_NULL));
     THROW_HR_IF(E_INVALIDARG, Request.Processor.Count == 0 || Request.Memory.SizeBytes == 0);
     THROW_HR_IF_MSG(c_notSupported, wsl::shared::Arm64, "OpenVMM direct boot is currently supported only on x64");
-    ValidatePath(Request.Boot.KernelPath);
-    ValidatePath(Request.Boot.InitrdPath);
+    validation::ValidatePath(Request.Boot.KernelPath, L"OpenVMM Kernel Path");
+    validation::ValidatePath(Request.Boot.InitrdPath, L"OpenVMM Initrd Path");
     THROW_HR_IF(
         E_INVALIDARG,
         Request.Boot.GuestCommandLine.find(L'\0') != std::wstring::npos || Request.Boot.UserCommandLine.find(L'\0') != std::wstring::npos);
@@ -219,12 +141,12 @@ VmDescription wsl::windows::common::vm::openvmm::ValidateCreateRequest(const VmC
         c_notSupported,
         description.Memory.SizeBytes < c_memoryGranularity || Request.Memory.SizeBytes > c_maximumMemory,
         "OpenVMM currently supports memory sizes from 2 MiB to 4 GiB; memory is not silently capped");
-    ValidateFeature(Request.Processor.NestedVirtualization, L"nested virtualization");
-    ValidateFeature(Request.Processor.PerfmonPmu, L"PMU");
-    ValidateFeature(Request.Processor.PerfmonLbr, L"LBR");
-    ValidateFeature(Request.Memory.AllowOvercommit, L"memory overcommit");
-    ValidateFeature(Request.Memory.DeferredCommit, L"deferred memory commit");
-    ValidateFeature(Request.Memory.ColdDiscard, L"cold discard");
+    validation::ValidateFeature(Request.Processor.NestedVirtualization, L"nested virtualization");
+    validation::ValidateFeature(Request.Processor.PerfmonPmu, L"PMU");
+    validation::ValidateFeature(Request.Processor.PerfmonLbr, L"LBR");
+    validation::ValidateFeature(Request.Memory.AllowOvercommit, L"memory overcommit");
+    validation::ValidateFeature(Request.Memory.DeferredCommit, L"deferred memory commit");
+    validation::ValidateFeature(Request.Memory.ColdDiscard, L"cold discard");
 
     if (Request.CrashCapture)
     {
@@ -251,7 +173,7 @@ VmDescription wsl::windows::common::vm::openvmm::ValidateCreateRequest(const VmC
         {
             THROW_HR_IF(c_notSupported, serial->Port != 0);
             THROW_HR_IF(E_INVALIDARG, serialConfigured);
-            ValidateConsolePath(serial->NamedPipe);
+            validation::ValidateConsolePath(serial->NamedPipe, L"OpenVMM Serial Console", E_INVALIDARG, true);
             serialConfigured = true;
         }
         else
@@ -259,7 +181,7 @@ VmDescription wsl::windows::common::vm::openvmm::ValidateCreateRequest(const VmC
             const auto& virtio = std::get<VmVirtioConsole>(console.Device);
             THROW_HR_IF(c_notSupported, virtio.Port != 0 || !virtio.GuestName.empty());
             THROW_HR_IF(E_INVALIDARG, virtioConfigured);
-            ValidateConsolePath(virtio.NamedPipe);
+            validation::ValidateConsolePath(virtio.NamedPipe, L"OpenVMM Virtio Console", E_INVALIDARG, true);
             virtioConfigured = true;
         }
         description.Boot.Consoles.push_back(console);
@@ -271,7 +193,7 @@ VmDescription wsl::windows::common::vm::openvmm::ValidateCreateRequest(const VmC
     {
         THROW_HR_IF(E_INVALIDARG, disk.Key.empty() || description.BootDisks.contains(disk.Key));
         description.BootDisks.emplace(disk.Key, VmDiskAttachment{});
-        ValidateDiskRequest(disk.Disk);
+        validation::ValidateDiskRequest(disk.Disk, c_maximumDisks);
         if (disk.Disk.Placement)
         {
             const auto& placement = *disk.Disk.Placement;
@@ -304,7 +226,7 @@ VmDescription wsl::windows::common::vm::openvmm::ValidateCreateRequest(const VmC
     THROW_HR_IF_MSG(c_notSupported, Request.NetworkAdapters.size() > 1, "OpenVMM supports one creation-time network adapter");
     for (const auto& adapter : Request.NetworkAdapters)
     {
-        ValidateName(adapter.Tag, L"OpenVMM network adapter tag");
+        validation::ValidateName(adapter.Tag, L"OpenVMM network adapter tag");
         ValidateNetworkConfiguration(adapter.Configuration);
         GUID nicId{};
         THROW_IF_FAILED(CoCreateGuid(&nicId));
@@ -327,8 +249,9 @@ OpenVmmVirtualMachineBackend::~OpenVmmVirtualMachineBackend() noexcept
     const auto startTimeMs = GetTickCount64();
     WSL_LOG("OpenVmmDestroyVmBegin", TraceLoggingValue(m_description.Identity.VmId, "vmId"));
     {
+        // TODO the locking story here isn't fully fleshed out; consider potential deadlocks and proper order of resource cleanup
         auto lock = m_lock.lock_exclusive();
-        CloseGuestListeners();
+        CloseGuestListenersLocked(m_description.Identity);
     }
     if (m_processWait)
     {
@@ -542,7 +465,7 @@ void CALLBACK OpenVmmVirtualMachineBackend::OnProcessExit(PTP_CALLBACK_INSTANCE,
     LOG_IF_WIN32_BOOL_FALSE(SetEvent(backend.m_exitEvent.get()));
     LOG_IF_WIN32_BOOL_FALSE(SetEvent(backend.m_operationCancellationEvent.get()));
     auto lock = backend.m_lock.lock_exclusive();
-    backend.CloseGuestListeners();
+    backend.CloseGuestListenersLocked(backend.m_description.Identity);
 }
 
 VmPlatformCapabilities OpenVmmVirtualMachineBackend::QueryCapabilities()
@@ -659,7 +582,7 @@ void OpenVmmVirtualMachineBackend::Terminate()
     m_fileSystemDevices.clear();
     m_portBindings.clear();
     m_networkAdapters.clear();
-    CloseGuestListeners();
+    CloseGuestListenersLocked(m_description.Identity);
     WSL_LOG(
         "OpenVmmTerminateVmEnd",
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
@@ -669,6 +592,8 @@ void OpenVmmVirtualMachineBackend::Terminate()
 void OpenVmmVirtualMachineBackend::CancelPendingOperations() noexcept
 {
     WSL_LOG("OpenVmmCancelPendingOperations", TraceLoggingValue(m_description.Identity.VmId, "vmId"));
+    LOG_IF_WIN32_BOOL_FALSE(SetEvent(m_operationCancellationEvent.get()));
+    auto lock = m_lock.lock_exclusive();
     if (m_vm)
     {
         const auto result = WslOpenVmmVmCancelRequests(m_vm.get());
@@ -678,28 +603,19 @@ void OpenVmmVirtualMachineBackend::CancelPendingOperations() noexcept
             TraceLoggingHResult(result, "result"));
         LOG_IF_FAILED(result);
     }
-    LOG_IF_WIN32_BOOL_FALSE(SetEvent(m_operationCancellationEvent.get()));
-    auto lock = m_lock.lock_exclusive();
-    CloseGuestListeners();
+    CloseGuestListenersLocked(m_description.Identity);
 }
 
-VmGuestListener OpenVmmVirtualMachineBackend::CreateGuestListener(GuestServicePort Port)
+std::shared_ptr<VmGuestListenerState> OpenVmmVirtualMachineBackend::ConfigureGuestListener(const VmGuestListener& Listener)
 {
     WSL_LOG(
         "OpenVmmCreateGuestListenerBegin",
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
-        TraceLoggingValue(Port.Value, "port"));
-    auto lock = m_lock.lock_exclusive();
-    THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_vm);
-    THROW_HR_IF(E_BOUNDS, m_nextListenerId == UINT64_MAX);
-    for (const auto& entry : m_guestListeners)
-    {
-        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), entry.second->Listener.Port.Value == Port.Value);
-    }
+        TraceLoggingValue(Listener.Port.Value, "port"));
 
     auto listener = std::make_shared<GuestListener>();
-    listener->Listener = {{m_description.Identity, m_nextListenerId}, Port};
-    listener->Path = GetVsockListenerPath(m_vsockPath, Port);
+    listener->Listener = Listener;
+    listener->Path = GetVsockListenerPath(m_vsockPath, Listener.Port);
     DeleteOwnedFile(listener->Path);
 
     listener->Socket.reset(::socket(AF_UNIX, SOCK_STREAM, 0));
@@ -716,44 +632,24 @@ VmGuestListener OpenVmmVirtualMachineBackend::CreateGuestListener(GuestServicePo
         static_cast<DWORD>(WSAGetLastError()),
         bind(listener->Socket.get(), reinterpret_cast<const sockaddr*>(&address), sizeof(address)) == SOCKET_ERROR);
     THROW_WIN32_IF(static_cast<DWORD>(WSAGetLastError()), listen(listener->Socket.get(), SOMAXCONN) == SOCKET_ERROR);
-
-    const auto result = listener->Listener;
-    const auto inserted = m_guestListeners.emplace(result.Id.Value, std::move(listener)).second;
-    WI_ASSERT(inserted);
-    ++m_nextListenerId;
     WSL_LOG(
         "OpenVmmCreateGuestListenerEnd",
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
-        TraceLoggingValue(result.Id.Value, "listenerId"),
-        TraceLoggingValue(Port.Value, "port"));
-    return result;
+        TraceLoggingValue(listener->Listener.Id.Value, "listenerId"),
+        TraceLoggingValue(Listener.Port.Value, "port"));
+    return listener;
+}
+
+VmGuestListener OpenVmmVirtualMachineBackend::CreateGuestListener(GuestServicePort Port)
+{
+    auto lock = m_lock.lock_exclusive();
+    THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_vm);
+    return RegisterGuestListenerLocked(m_description.Identity, Port);
 }
 
 wil::unique_socket OpenVmmVirtualMachineBackend::AcceptGuestConnection(VmListenerId Listener)
 {
-    WSL_LOG(
-        "OpenVmmAcceptGuestConnectionBegin",
-        TraceLoggingValue(m_description.Identity.VmId, "vmId"),
-        TraceLoggingValue(Listener.Value, "listenerId"));
-    THROW_HR_IF(E_INVALIDARG, Listener.Value == 0 || !IsEqualGUID(Listener.Owner.VmId, m_description.Identity.VmId));
-
-    std::shared_ptr<GuestListener> listener;
-    {
-        auto lock = m_lock.lock_shared();
-        const auto entry = m_guestListeners.find(Listener.Value);
-        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), entry == m_guestListeners.end());
-        listener = entry->second;
-    }
-
-    auto socket = wsl::windows::common::socket::CancellableAccept(listener->Socket.get(), INFINITE, listener->CancellationEvent.get());
-    WSL_LOG(
-        "OpenVmmAcceptGuestConnectionEnd",
-        TraceLoggingValue(m_description.Identity.VmId, "vmId"),
-        TraceLoggingValue(Listener.Value, "listenerId"),
-        TraceLoggingValue(listener->Listener.Port.Value, "port"),
-        TraceLoggingHResult(socket ? S_OK : E_ABORT, "result"));
-    THROW_HR_IF(E_ABORT, !socket);
-    return std::move(*socket);
+    return AcceptGuestListenerConnection(Listener, m_description.Identity);
 }
 
 wil::unique_socket OpenVmmVirtualMachineBackend::ConnectGuest(GuestServicePort Port)
@@ -814,16 +710,8 @@ wil::unique_socket OpenVmmVirtualMachineBackend::ConnectGuest(GuestServicePort P
 
 void OpenVmmVirtualMachineBackend::CloseGuestListener(VmListenerId Listener)
 {
-    WSL_LOG(
-        "OpenVmmCloseGuestListener",
-        TraceLoggingValue(m_description.Identity.VmId, "vmId"),
-        TraceLoggingValue(Listener.Value, "listenerId"));
-    THROW_HR_IF(E_INVALIDARG, Listener.Value == 0 || !IsEqualGUID(Listener.Owner.VmId, m_description.Identity.VmId));
     auto lock = m_lock.lock_exclusive();
-    const auto entry = m_guestListeners.find(Listener.Value);
-    THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), entry == m_guestListeners.end());
-    THROW_IF_WIN32_BOOL_FALSE(SetEvent(entry->second->CancellationEvent.get()));
-    m_guestListeners.erase(entry);
+    RemoveGuestListenerLocked(Listener, m_description.Identity);
 }
 
 VmDiskAttachment OpenVmmVirtualMachineBackend::AttachDisk(const VmDiskRequest& Request)
@@ -833,7 +721,7 @@ VmDiskAttachment OpenVmmVirtualMachineBackend::AttachDisk(const VmDiskRequest& R
         "OpenVmmAttachDiskBegin",
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
         TraceLoggingValue(Request.ReadOnly, "readOnly"));
-    const auto& source = ValidateDiskRequest(Request);
+    const auto& source = validation::ValidateDiskRequest(Request, c_maximumDisks);
     auto backingFile = OpenBackingFile(source.Path, Request.ReadOnly);
     auto lock = m_lock.lock_exclusive();
     THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_vm);
@@ -915,7 +803,7 @@ VmFileSystemDevice OpenVmmVirtualMachineBackend::CreateFileSystemDevice(const Vm
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
         TraceLoggingValue(Request.Transport.Tag.c_str(), "tag"),
         TraceLoggingValue(static_cast<UINT32>(Request.Transport.Layout), "layout"));
-    ValidateName(Request.Transport.Tag, L"OpenVMM virtio-fs tag");
+    validation::ValidateName(Request.Transport.Tag, L"OpenVMM virtio-fs tag");
     THROW_HR_IF_MSG(
         c_notSupported,
         Request.Transport.Layout != VmVirtioFsLayout::SingleShare,
@@ -949,8 +837,8 @@ VmFileSystemShare OpenVmmVirtualMachineBackend::AddFileSystemShare(VmDeviceId De
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
         TraceLoggingValue(Device.Value, "deviceId"),
         TraceLoggingValue(Request.ReadOnly, "readOnly"));
-    ValidateResourceId(Device, m_description.Identity);
-    ValidatePath(Request.HostPath);
+    validation::ValidateResourceId(Device, m_description.Identity);
+    validation::ValidatePath(Request.HostPath, L"OpenVMM virtio-fs host path");
     THROW_HR_IF_MSG(
         E_INVALIDARG, !Request.Name.empty(), "A single-share OpenVMM virtio-fs device does not accept a child share name");
     THROW_HR_IF_MSG(c_notSupported, !Request.Options.MountOptions.empty(), "OpenVMM does not support virtio-fs mount options");
@@ -997,7 +885,7 @@ void OpenVmmVirtualMachineBackend::RemoveFileSystemShare(VmShareId Share)
         "OpenVmmRemoveFileSystemShareBegin",
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
         TraceLoggingValue(Share.Value, "shareId"));
-    ValidateResourceId(Share, m_description.Identity);
+    validation::ValidateResourceId(Share, m_description.Identity);
     auto lock = m_lock.lock_exclusive();
     THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_vm);
     const auto share = m_fileSystemShares.find(Share.Value);
@@ -1035,7 +923,7 @@ VmPortBinding OpenVmmVirtualMachineBackend::BindPort(VmDeviceId Device, const Vm
         TraceLoggingValue(static_cast<UINT32>(Request.Protocol), "protocol"),
         TraceLoggingValue(Request.Listen.Port, "hostPort"),
         TraceLoggingValue(Request.GuestPort, "guestPort"));
-    ValidateResourceId(Device, m_description.Identity);
+    validation::ValidateResourceId(Device, m_description.Identity);
     THROW_HR_IF_MSG(
         c_notSupported, Request.Listen.Port == 0, "OpenVMM cannot report the allocated port for a dynamic host port binding");
     THROW_HR_IF(E_INVALIDARG, Request.GuestPort == 0);
@@ -1090,7 +978,7 @@ void OpenVmmVirtualMachineBackend::UnbindPort(VmPortBindingId Binding)
         "OpenVmmUnbindPortBegin",
         TraceLoggingValue(m_description.Identity.VmId, "vmId"),
         TraceLoggingValue(Binding.Value, "bindingId"));
-    ValidateResourceId(Binding, m_description.Identity);
+    validation::ValidateResourceId(Binding, m_description.Identity);
     auto lock = m_lock.lock_exclusive();
     THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_vm);
     const auto binding = m_portBindings.find(Binding.Value);

src/windows/service/exe/OpenVmmVirtualMachineBackend.h

@@ -69,20 +69,19 @@ private:
     static void DestroyVm(WslOpenVmmVm* Vm) noexcept;
     using UniqueVm = wil::unique_any<WslOpenVmmVm*, decltype(&DestroyVm), DestroyVm>;
 
+    std::shared_ptr<VmGuestListenerState> ConfigureGuestListener(const VmGuestListener& Listener) override;
+
     struct AttachedDisk
     {
         VmDiskAttachment Attachment;
         wil::unique_hfile BackingFile;
     };
 
-    struct GuestListener
+    struct GuestListener : VmGuestListenerState
     {
         ~GuestListener() noexcept;
 
-        VmGuestListener Listener;
-        wil::unique_socket Socket;
         std::filesystem::path Path;
-        wil::unique_event CancellationEvent{wil::EventOptions::ManualReset};
     };
 
     struct FileSystemDevice
@@ -110,15 +109,9 @@ private:
         std::wstring HostAddress;
     };
 
-    wil::srwlock m_lock;
-    _Requires_lock_held_(m_lock)
-    void CloseGuestListeners() noexcept;
-
     VmDescription m_description{};
     _Guarded_by_(m_lock) std::map<std::uint64_t, AttachedDisk> m_attachedDisks;
     _Guarded_by_(m_lock) std::uint64_t m_nextDiskId = 1;
-    _Guarded_by_(m_lock) std::map<std::uint64_t, std::shared_ptr<GuestListener>> m_guestListeners;
-    _Guarded_by_(m_lock) std::uint64_t m_nextListenerId = 1;
     _Guarded_by_(m_lock) std::map<std::uint64_t, FileSystemDevice> m_fileSystemDevices;
     _Guarded_by_(m_lock) std::map<std::uint64_t, FileSystemShare> m_fileSystemShares;
     _Guarded_by_(m_lock) std::map<std::uint64_t, NetworkAdapter> m_networkAdapters;
@@ -126,7 +119,7 @@ private:
     _Guarded_by_(m_lock) std::uint64_t m_nextDeviceId = 1;
     _Guarded_by_(m_lock) std::uint64_t m_nextShareId = 1;
     _Guarded_by_(m_lock) std::uint64_t m_nextPortBindingId = 1;
-    UniqueVm m_vm;
+    _Guarded_by_(m_lock) UniqueVm m_vm;
     wil::unique_handle m_process;
     wil::unique_handle m_job;
     std::vector<wil::unique_hfile> m_backingFiles;

src/windows/service/exe/VirtualMachineBackend.cpp

@@ -13,9 +13,176 @@ Abstract:
 --*/
 
 #include "precomp.h"
+#include "hvsocket.hpp"
+#include "socket.hpp"
 #include "IVirtualMachineBackend.h"
 #include "OpenVmmVirtualMachineBackend.h"
 
+namespace {
+
+constexpr HRESULT c_notSupported = HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED);
+
+}
+
+
+void wsl::windows::common::vm::validation::ValidateFeature(VmFeatureRequest Request, PCWSTR Setting)
+{
+    switch (Request)
+    {
+    case VmFeatureRequest::Disabled:
+    case VmFeatureRequest::Preferred:
+        return;
+    case VmFeatureRequest::Required:
+        THROW_HR_MSG(c_notSupported, "OpenVMM does not support the required %ls setting", Setting);
+    }
+
+    THROW_HR(E_INVALIDARG);
+}
+
+void wsl::windows::common::vm::validation::ValidateUnsupportedSelection(VmSelectionPolicy Policy)
+{
+    THROW_HR_IF(E_INVALIDARG, Policy != VmSelectionPolicy::Required && Policy != VmSelectionPolicy::Preferred);
+    THROW_HR_IF(c_notSupported, Policy == VmSelectionPolicy::Required);
+}
+
+void wsl::windows::common::vm::validation::ValidatePath(const std::filesystem::path& Path, PCWSTR Backend)
+{
+    THROW_HR_IF_MSG(
+        E_INVALIDARG,
+        Path.empty() || !Path.is_absolute() || Path.native().find(L'\0') != std::wstring::npos,
+        "%ls requires an absolute, nonempty host path",
+        Backend);
+}
+
+const VmVirtualDiskSource& wsl::windows::common::vm::validation::ValidateDiskRequest(const VmDiskRequest& Request, UINT32 MaximumDisks)
+{
+    const auto* source = std::get_if<VmVirtualDiskSource>(&Request.Source);
+    THROW_HR_IF(c_notSupported, source == nullptr);
+    ValidatePath(source->Path, L"OpenVMM");
+    switch (source->Format)
+    {
+    case VmDiskFormat::Vhd:
+        THROW_HR_IF(E_INVALIDARG, _wcsicmp(source->Path.extension().c_str(), L".vhd") != 0);
+        break;
+    case VmDiskFormat::Vhdx:
+        THROW_HR_IF(E_INVALIDARG, _wcsicmp(source->Path.extension().c_str(), L".vhdx") != 0);
+        break;
+    default:
+        THROW_HR(E_INVALIDARG);
+    }
+
+    if (Request.Placement)
+    {
+        THROW_HR_IF(c_notSupported, Request.Placement->Address.Controller != 0 || Request.Placement->Address.Lun >= MaximumDisks);
+    }
+
+    return *source;
+}
+
+void wsl::windows::common::vm::validation::ValidateConsolePath(
+    const std::filesystem::path& Path, PCWSTR Backend, HRESULT Error, bool RequireName)
+{
+    ValidatePath(Path, Backend);
+    THROW_HR_IF_MSG(
+        Error,
+        !Path.native().starts_with(L"\\\\.\\pipe\\") || (RequireName && Path.filename().empty()),
+        "%ls consoles require a caller-provided named pipe",
+        Backend);
+}
+
+void wsl::windows::common::vm::validation::ValidateName(std::wstring_view Name, PCWSTR Description)
+{
+    THROW_HR_IF_MSG(
+        E_INVALIDARG, Name.empty() || Name.find(L'\0') != std::wstring_view::npos, "%ls must be nonempty and cannot contain NUL", Description);
+}
+
+void wsl::windows::common::vm::validation::ValidateResourceId(UINT64 Value, const GUID& VmId, const VmInstanceId& Owner)
+{
+    THROW_HR_IF(E_INVALIDARG, Value == 0 || !IsEqualGUID(VmId, Owner.VmId));
+}
+
+VmGuestListener IVirtualMachineBackend::RegisterGuestListenerLocked(const VmInstanceId& Identity, GuestServicePort Port)
+{
+    THROW_HR_IF(E_BOUNDS, m_nextListenerId == UINT64_MAX);
+    for (const auto& entry : m_guestListeners)
+    {
+        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), entry.second->Listener.Port.Value == Port.Value);
+    }
+
+    const VmGuestListener listener{{Identity, m_nextListenerId}, Port};
+    auto state = ConfigureGuestListener(listener);
+    THROW_HR_IF(E_UNEXPECTED, !state);
+    THROW_HR_IF(E_UNEXPECTED, state->Listener.Id.Value != listener.Id.Value || state->Listener.Port.Value != listener.Port.Value);
+
+    const auto inserted = m_guestListeners.emplace(listener.Id.Value, std::move(state)).second;
+    WI_ASSERT(inserted);
+    ++m_nextListenerId;
+    return listener;
+}
+
+wil::unique_socket IVirtualMachineBackend::AcceptGuestListenerConnection(VmListenerId Listener, const VmInstanceId& Identity) const
+{
+    WSL_LOG(
+        "OpenVmmAcceptGuestConnectionBegin",
+        TraceLoggingValue(Identity.VmId, "vmId"),
+        TraceLoggingValue(Listener.Value, "listenerId"));
+    THROW_HR_IF(E_INVALIDARG, Listener.Value == 0 || !IsEqualGUID(Listener.Owner.VmId, Identity.VmId));
+    std::shared_ptr<VmGuestListenerState> listener;
+    {
+        auto lock = m_lock.lock_shared();
+        const auto entry = m_guestListeners.find(Listener.Value);
+        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), entry == m_guestListeners.end());
+        listener = entry->second;
+    }
+
+    auto socket = wsl::windows::common::socket::CancellableAccept(listener->Socket.get(), INFINITE, listener->CancellationEvent.get());
+    WSL_LOG(
+        "OpenVmmAcceptGuestConnectionEnd",
+        TraceLoggingValue(Identity.VmId, "vmId"),
+        TraceLoggingValue(Listener.Value, "listenerId"),
+        TraceLoggingValue(listener->Listener.Port.Value, "port"),
+        TraceLoggingHResult(socket ? S_OK : E_ABORT, "result"));
+    THROW_HR_IF(E_ABORT, !socket);
+    return std::move(*socket);
+}
+
+std::shared_ptr<VmGuestListenerState> IVirtualMachineBackend::RemoveGuestListenerLocked(VmListenerId Listener, const VmInstanceId& Identity)
+{
+    WSL_LOG(
+        "OpenVmmCloseGuestListener",
+        TraceLoggingValue(Identity.VmId, "vmId"),
+        TraceLoggingValue(Listener.Value, "listenerId"));
+    THROW_HR_IF(E_INVALIDARG, Listener.Value == 0 || !IsEqualGUID(Listener.Owner.VmId, Identity.VmId));
+    const auto entry = m_guestListeners.find(Listener.Value);
+    THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), entry == m_guestListeners.end());
+    THROW_IF_WIN32_BOOL_FALSE(SetEvent(entry->second->CancellationEvent.get()));
+    auto result = std::move(entry->second);
+    m_guestListeners.erase(entry);
+    return result;
+}
+
+void IVirtualMachineBackend::CloseGuestListenersLocked(const VmInstanceId& Identity) noexcept
+{
+    WSL_LOG(
+        "OpenVmmCloseGuestListeners",
+        TraceLoggingValue(Identity.VmId, "vmId"),
+        TraceLoggingValue(m_guestListeners.size(), "listenerCount"));
+    for (const auto& entry : m_guestListeners)
+    {
+        LOG_IF_WIN32_BOOL_FALSE(SetEvent(entry.second->CancellationEvent.get()));
+    }
+
+    m_guestListeners.clear();
+}
+
+std::shared_ptr<VmGuestListenerState> IVirtualMachineBackend::ConfigureGuestListener(const VmGuestListener& Listener)
+{
+    auto state = std::make_shared<VmGuestListenerState>();
+    state->Listener = Listener;
+    state->Socket = wsl::windows::common::hvsocket::Listen(Listener.Id.Owner.VmId, Listener.Port.Value);
+    return state;
+}
+
 std::unique_ptr<IVirtualMachineBackend> CreateVirtualMachineBackend(BackendKind Kind, const VmCreateRequest& Request)
 {
     switch (Kind)