From 9e9ef6f145f996c880874da81f884e41a8b9a852 Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Tue, 21 Oct 2025 17:00:47 -0700 Subject: [PATCH] Fix edge cases around .vhd support (#13061) * Fix edge cases around .vhd support * PR feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * pr feedback * remove unneeded scope exit in unit test --------- Co-authored-by: Ben Hillis Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- localization/strings/en-US/Resources.resw | 27 ++++--- src/windows/common/WslClient.cpp | 26 +----- src/windows/common/wslutil.cpp | 7 ++ src/windows/common/wslutil.h | 2 + src/windows/service/exe/LxssUserSession.cpp | 48 ++++++++--- src/windows/service/exe/WslCoreFilesystem.cpp | 4 +- test/windows/SimpleTests.cpp | 4 +- test/windows/UnitTests.cpp | 79 +++++++++++++++++-- 8 files changed, 142 insertions(+), 55 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 61526d6..890dff2 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -466,7 +466,7 @@ Arguments for managing Windows Subsystem for Linux: Move the distribution to a new location. --set-sparse, -s <true|false> - Set the vhdx of distro to be sparse, allowing disk space to be automatically reclaimed. + Set the VHD of distro to be sparse, allowing disk space to be automatically reclaimed. --set-default-user <Username> Set the default user of the distribution. @@ -546,11 +546,11 @@ Arguments for managing distributions in Windows Subsystem for Linux: Specifies the version to use for the new distribution. --vhd - Specifies that the provided file is a .vhdx file, not a tar file. - This operation makes a copy of the .vhdx file at the specified install location. + Specifies that the provided file is a .vhd or .vhdx file, not a tar file. + This operation makes a copy of the VHD file at the specified install location. --import-in-place <Distro> <FileName> - Imports the specified .vhdx file as a new distribution. + Imports the specified VHD file as a new distribution. This virtual hard disk must be formatted with the ext4 filesystem type. --list, -l [Options] @@ -626,7 +626,7 @@ Build time: {} {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated - The custom kernel modules vhd in {} was not found: '{}'. + The custom kernel modules VHD in {} was not found: '{}'. {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated @@ -704,8 +704,13 @@ The system may need to be restarted so the changes can take effect. {Locked="--install "}{Locked="--no-distribution "}Command line arguments, file names and string inserts should not be translated - - The specified file must have the .vhdx file extension. + + The specified file must have the {} file extension. + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + + The specified file must have the {} or {} file extension. + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated The VmSwitch '{}' was not found. Available switches: {} @@ -998,7 +1003,7 @@ Falling back to NAT networking. See Docs - The operation could not be completed because the vhdx is currently in use. To force WSL to stop use: wsl.exe --shutdown + The operation could not be completed because the VHD is currently in use. To force WSL to stop use: wsl.exe --shutdown {Locked="--shutdown"}Command line arguments, file names and string inserts should not be translated @@ -1006,7 +1011,7 @@ Falling back to NAT networking. {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated - Sparse vhdx is supported on WSL2 only. + Sparse VHD is supported on WSL2 only. Running WSL as local system is not supported. @@ -1055,7 +1060,7 @@ Falling back to NAT networking. {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated - This looks like a VHDX file. Use --vhd to import a VHDX instead of a tar. + This looks like a VHD file. Use --vhd to import a VHD instead of a tar. {Locked="--vhd "}Command line arguments, file names and string inserts should not be translated @@ -1107,7 +1112,7 @@ Error code: {} Sparse VHD support is currently disabled due to potential data corruption. -To force a distribution to use a sparse vhd, please run: +To force a distribution to use a sparse VHD, please run: wsl.exe --manage <DistributionName> --set-sparse true --allow-unsafe {Locked="--manage "}{Locked="--set-sparse "}{Locked="--allow-unsafe"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/common/WslClient.cpp b/src/windows/common/WslClient.cpp index 04d167a..7c34582 100644 --- a/src/windows/common/WslClient.cpp +++ b/src/windows/common/WslClient.cpp @@ -275,14 +275,6 @@ int ExportDistribution(_In_ std::wstring_view commandLine) } else { - // If exporting to a vhd, ensure the filename ends with the vhdx file extension. - if (WI_IsFlagSet(flags, LXSS_EXPORT_DISTRO_FLAGS_VHD) && - !wsl::windows::common::string::IsPathComponentEqual(filePath.extension().native(), wsl::windows::common::wslutil::c_vhdxFileExtension)) - { - wsl::windows::common::wslutil::PrintMessage(wsl::shared::Localization::MessageRequiresVhdxFileExtension()); - return -1; - } - file.reset(CreateFileW( filePath.c_str(), GENERIC_WRITE, (FILE_SHARE_READ | FILE_SHARE_DELETE), nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); @@ -371,22 +363,10 @@ int ImportDistribution(_In_ std::wstring_view commandLine) } else { - bool isVhd = wsl::windows::common::string::IsPathComponentEqual( - filePath.extension().native(), wsl::windows::common::wslutil::c_vhdxFileExtension); - - if (WI_IsFlagSet(flags, LXSS_IMPORT_DISTRO_FLAGS_VHD)) + if (WI_IsFlagClear(flags, LXSS_IMPORT_DISTRO_FLAGS_VHD)) { - // If importing from a vhd, ensure the filename ends with the vhdx file extension. - if (!isVhd) - { - wsl::windows::common::wslutil::PrintMessage(wsl::shared::Localization::MessageRequiresVhdxFileExtension()); - return -1; - } - } - else - { - // Fail if we expect a tar, but the file name has the .vhdx extension. - if (isVhd) + // Fail if expecting a tar, but the file name has the .vhd or .vhdx extension. + if (wsl::windows::common::wslutil::IsVhdFile(filePath)) { wsl::windows::common::wslutil::PrintMessage(wsl::shared::Localization::MessagePassVhdFlag()); return -1; diff --git a/src/windows/common/wslutil.cpp b/src/windows/common/wslutil.cpp index a06f08f..ca87eff 100644 --- a/src/windows/common/wslutil.cpp +++ b/src/windows/common/wslutil.cpp @@ -1155,6 +1155,13 @@ bool wsl::windows::common::wslutil::IsRunningInMsix() return false; } } + +bool wsl::windows::common::wslutil::IsVhdFile(_In_ const std::filesystem::path& path) +{ + return wsl::windows::common::string::IsPathComponentEqual(path.extension().native(), c_vhdFileExtension) || + wsl::windows::common::string::IsPathComponentEqual(path.extension().native(), c_vhdxFileExtension); +} + std::vector wsl::windows::common::wslutil::ListRunningProcesses() { std::vector pids(1024); diff --git a/src/windows/common/wslutil.h b/src/windows/common/wslutil.h index 26b7a20..41d4b7c 100644 --- a/src/windows/common/wslutil.h +++ b/src/windows/common/wslutil.h @@ -122,6 +122,8 @@ void InitializeWil(); bool IsRunningInMsix(); +bool IsVhdFile(_In_ const std::filesystem::path& path); + bool IsVirtualMachinePlatformInstalled(); std::vector ListRunningProcesses(); diff --git a/src/windows/service/exe/LxssUserSession.cpp b/src/windows/service/exe/LxssUserSession.cpp index cb61109..d0dcc04 100644 --- a/src/windows/service/exe/LxssUserSession.cpp +++ b/src/windows/service/exe/LxssUserSession.cpp @@ -927,7 +927,7 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW std::filesystem::path newVhdPath = Location; RETURN_HR_IF(E_INVALIDARG, newVhdPath.empty()); - newVhdPath /= LXSS_VM_MODE_VHD_NAME; + newVhdPath /= distro.VhdFilePath.filename(); auto impersonate = wil::CoImpersonateClient(); @@ -952,7 +952,7 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW // Update the registry location registration.Write(Property::BasePath, Location); - registration.Write(Property::VhdFileName, LXSS_VM_MODE_VHD_NAME); + registration.Write(Property::VhdFileName, newVhdPath.filename().c_str()); revert.release(); @@ -1079,6 +1079,22 @@ HRESULT LxssUserSessionImpl::ExportDistribution(_In_opt_ LPCGUID DistroGuid, _In { const wil::unique_handle userToken = wsl::windows::common::security::GetUserToken(TokenImpersonation); auto runAsUser = wil::impersonate_token(userToken.get()); + + // Ensure the target file has the correct file extension. + if (GetFileType(FileHandle) == FILE_TYPE_DISK) + { + std::wstring exportPath; + THROW_IF_FAILED(wil::GetFinalPathNameByHandleW(FileHandle, exportPath)); + + const auto sourceFileExtension = configuration.VhdFilePath.extension().native(); + const auto targetFileExtension = std::filesystem::path(exportPath).extension().native(); + if (!wsl::windows::common::string::IsPathComponentEqual(sourceFileExtension, targetFileExtension)) + { + THROW_HR_WITH_USER_ERROR( + WSL_E_EXPORT_FAILED, wsl::shared::Localization::MessageRequiresFileExtension(sourceFileExtension.c_str())); + } + } + const wil::unique_hfile vhdFile(CreateFileW( configuration.VhdFilePath.c_str(), GENERIC_READ, (FILE_SHARE_READ | FILE_SHARE_DELETE), nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)); @@ -1258,13 +1274,9 @@ LxssUserSessionImpl::ImportDistributionInplace(_In_ LPCWSTR DistributionName, _I s_ValidateDistroName(DistributionName); - // Return an error if the path is not absolute or does not end in the .vhdx file extension. + // Return an error if the path is not absolute or does not have a valid VHD file extension. const std::filesystem::path path{VhdPath}; - RETURN_HR_IF( - E_INVALIDARG, - !path.is_absolute() || - (!wsl::windows::common::string::IsPathComponentEqual(path.extension().native(), wsl::windows::common::wslutil::c_vhdFileExtension) && - !wsl::windows::common::string::IsPathComponentEqual(path.extension().c_str(), wsl::windows::common::wslutil::c_vhdxFileExtension))); + RETURN_HR_IF(E_INVALIDARG, !path.is_absolute() || !wsl::windows::common::wslutil::IsVhdFile(path)); const wil::unique_hkey lxssKey = s_OpenLxssUserKey(); std::lock_guard lock(m_instanceLock); @@ -1448,6 +1460,24 @@ HRESULT LxssUserSessionImpl::RegisterDistribution( wil::CreateDirectoryDeep(distributionPath.c_str()); } + // If importing a vhd, determine if it is a .vhd or .vhdx. + std::wstring vhdName{LXSS_VM_MODE_VHD_NAME}; + if ((WI_IsFlagSet(Flags, LXSS_IMPORT_DISTRO_FLAGS_VHD)) && (GetFileType(FileHandle) == FILE_TYPE_DISK)) + { + std::wstring pathBuffer; + THROW_IF_FAILED(wil::GetFinalPathNameByHandleW(FileHandle, pathBuffer)); + + std::filesystem::path vhdPath{std::move(pathBuffer)}; + if (!wsl::windows::common::wslutil::IsVhdFile(vhdPath)) + { + using namespace wsl::windows::common::wslutil; + THROW_HR_WITH_USER_ERROR( + WSL_E_IMPORT_FAILED, wsl::shared::Localization::MessageRequiresFileExtensions(c_vhdFileExtension, c_vhdxFileExtension)); + } + + vhdName = vhdPath.filename(); + } + registration = DistributionRegistration::Create( lxssKey.get(), DistributionId, @@ -1457,7 +1487,7 @@ HRESULT LxssUserSessionImpl::RegisterDistribution( flags, LX_UID_ROOT, PackageFamilyName, - LXSS_VM_MODE_VHD_NAME, + vhdName.c_str(), WI_IsFlagClear(Flags, LXSS_IMPORT_DISTRO_FLAGS_NO_OOBE)); configuration = s_GetDistributionConfiguration(registration, DistributionName == nullptr); diff --git a/src/windows/service/exe/WslCoreFilesystem.cpp b/src/windows/service/exe/WslCoreFilesystem.cpp index 1f026dd..6fd79b3 100644 --- a/src/windows/service/exe/WslCoreFilesystem.cpp +++ b/src/windows/service/exe/WslCoreFilesystem.cpp @@ -71,9 +71,7 @@ void wsl::core::filesystem::CreateVhd(_In_ LPCWSTR target, _In_ ULONGLONG maximu wil::unique_handle wsl::core::filesystem::OpenVhd(_In_ LPCWSTR Path, _In_ VIRTUAL_DISK_ACCESS_MASK Mask) { - WI_ASSERT( - wsl::shared::string::IsEqual(std::filesystem::path{Path}.extension().c_str(), windows::common::wslutil::c_vhdFileExtension, true) || - wsl::shared::string::IsEqual(std::filesystem::path{Path}.extension().c_str(), windows::common::wslutil::c_vhdxFileExtension, true)); + WI_ASSERT(wsl::windows::common::wslutil::IsVhdFile(std::filesystem::path{Path})); // N.B. Specifying unknown for device and vendor means the system will determine the type of VHD. VIRTUAL_STORAGE_TYPE storageType{}; diff --git a/test/windows/SimpleTests.cpp b/test/windows/SimpleTests.cpp index 6adb571..5a72379 100644 --- a/test/windows/SimpleTests.cpp +++ b/test/windows/SimpleTests.cpp @@ -109,7 +109,7 @@ class SimpleTests std::format(L"{} {} {} {}", WSL_IMPORT_ARG, tempDistro, vhdDir.wstring(), tar.wstring()).c_str(), L"The operation completed successfully. \r\n", L"wsl: Sparse VHD support is currently disabled due to potential data corruption.\r\n" - L"To force a distribution to use a sparse vhd, please run:\r\n" + L"To force a distribution to use a sparse VHD, please run:\r\n" L"wsl.exe --manage --set-sparse true --allow-unsafe\r\n", 0); @@ -122,7 +122,7 @@ class SimpleTests ValidateOutput( std::format(L"{} {} {} {}", WSL_MANAGE_ARG, tempDistro, WSL_MANAGE_ARG_SET_SPARSE_OPTION_LONG, L"true").c_str(), L"Sparse VHD support is currently disabled due to potential data corruption.\r\n" - L"To force a distribution to use a sparse vhd, please run:\r\n" + L"To force a distribution to use a sparse VHD, please run:\r\n" L"wsl.exe --manage --set-sparse true --allow-unsafe\r\nError code: Wsl/Service/E_INVALIDARG\r\n", L"", -1); diff --git a/test/windows/UnitTests.cpp b/test/windows/UnitTests.cpp index 781221f..be832f7 100644 --- a/test/windows/UnitTests.cpp +++ b/test/windows/UnitTests.cpp @@ -1016,7 +1016,7 @@ class UnitTests L"Error code: Wsl/Service/RegisterDistro/ERROR_FILE_EXISTS\r\n"); commandLine = std::format(L"--import dummy {} {} --version {}", LXSST_IMPORT_DISTRO_TEST_DIR, vhdFileName, version); - validateOutput(commandLine.c_str(), L"This looks like a VHDX file. Use --vhd to import a VHDX instead of a tar.\r\n"); + validateOutput(commandLine.c_str(), L"This looks like a VHD file. Use --vhd to import a VHD instead of a tar.\r\n"); if (!LxsstuVmMode()) { @@ -1557,7 +1557,7 @@ Arguments for managing Windows Subsystem for Linux: Move the distribution to a new location. --set-sparse, -s - Set the vhdx of distro to be sparse, allowing disk space to be automatically reclaimed. + Set the VHD of distro to be sparse, allowing disk space to be automatically reclaimed. --set-default-user Set the default user of the distribution. @@ -1637,11 +1637,11 @@ Arguments for managing distributions in Windows Subsystem for Linux: Specifies the version to use for the new distribution. --vhd - Specifies that the provided file is a .vhdx file, not a tar file. - This operation makes a copy of the .vhdx file at the specified install location. + Specifies that the provided file is a .vhd or .vhdx file, not a tar file. + This operation makes a copy of the VHD file at the specified install location. --import-in-place - Imports the specified .vhdx file as a new distribution. + Imports the specified VHD file as a new distribution. This virtual hard disk must be formatted with the ext4 filesystem type. --list, -l [Options] @@ -2896,8 +2896,7 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND WslKeepAlive keepAlive; auto [out, _] = LxsstuLaunchWslAndCaptureOutput(L"--manage test_distro --resize 1500GB", -1); VERIFY_ARE_EQUAL( - L"The operation could not be completed because the vhdx is currently in use. To force WSL to stop use: " - L"wsl.exe " + L"The operation could not be completed because the VHD is currently in use. To force WSL to stop use: wsl.exe " L"--shutdown\r\nError code: Wsl/Service/WSL_E_DISTRO_NOT_STOPPED\r\n", out); } @@ -6276,5 +6275,71 @@ Error code: Wsl/InstallDistro/WSL_E_INVALID_JSON\r\n", VERIFY_ARE_EQUAL(out, L"755\n"); } + TEST_METHOD(ExportImportVhd) + { + WSL2_TEST_ONLY(); + + WslShutdown(); + + constexpr auto vhdPath = L"exported-test-distro.vhd"; + constexpr auto vhdxPath = L"exported-test-distro.vhdx"; + constexpr auto exportedVhdPath = L"exported-vhd.vhd"; + constexpr auto newDistroName = L"imported-test-distro"; + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + LOG_IF_WIN32_BOOL_FALSE(DeleteFile(vhdPath)); + LOG_IF_WIN32_BOOL_FALSE(DeleteFile(vhdxPath)); + LOG_IF_WIN32_BOOL_FALSE(DeleteFile(exportedVhdPath)); + LxsstuLaunchWsl(std::format(L"--unregister {}", newDistroName)); + }); + + // Attempt to export the distribution to a .vhd (should fail). + auto [out, err] = + LxsstuLaunchWslAndCaptureOutput(std::format(L"--export {} {} --format vhd", LXSS_DISTRO_NAME_TEST_L, vhdPath), -1); + VERIFY_ARE_EQUAL( + out, L"The specified file must have the .vhdx file extension.\r\nError code: Wsl/Service/WSL_E_EXPORT_FAILED\r\n"); + VERIFY_ARE_EQUAL(err, L""); + + // Export the distribution to a .vhdx. + std::tie(out, err) = + LxsstuLaunchWslAndCaptureOutput(std::format(L"--export {} {} --format vhd", LXSS_DISTRO_NAME_TEST_L, vhdxPath)); + VERIFY_ARE_EQUAL(out, L"The operation completed successfully. \r\n"); + VERIFY_ARE_EQUAL(err, L""); + + // Convert the .vhdx to .vhd. + LxsstuLaunchPowershellAndCaptureOutput(std::format(L"Convert-VHD -Path '{}' -DestinationPath '{}'", vhdxPath, vhdPath)); + + // Import a new distribution from the .vhd file. + std::tie(out, err) = + LxsstuLaunchWslAndCaptureOutput(std::format(L"--import {} {} {} --vhd", newDistroName, newDistroName, vhdPath)); + VERIFY_ARE_EQUAL(out, L"The operation completed successfully. \r\n"); + VERIFY_ARE_EQUAL(err, L""); + + // Export the newly imported distribution to another .vhd file. + std::tie(out, err) = LxsstuLaunchWslAndCaptureOutput(std::format(L"--export {} {} --format vhd", newDistroName, exportedVhdPath)); + VERIFY_ARE_EQUAL(out, L"The operation completed successfully. \r\n"); + VERIFY_ARE_EQUAL(err, L""); + + // Attempt to export to a .vhdx (should fail). + std::tie(out, err) = LxsstuLaunchWslAndCaptureOutput(std::format(L"--export {} {} --format vhd", newDistroName, vhdxPath), -1); + VERIFY_ARE_EQUAL( + out, L"The specified file must have the .vhd file extension.\r\nError code: Wsl/Service/WSL_E_EXPORT_FAILED\r\n"); + VERIFY_ARE_EQUAL(err, L""); + + // Attempt to import to a non VHD file. + auto tempFile = wsl::windows::common::filesystem::TempFile( + GENERIC_ALL, 0, CREATE_ALWAYS, wsl::windows::common::filesystem::TempFileFlags::None, L"txt"); + + tempFile.Handle.reset(); + + constexpr auto negativeVariationDistro = L"negative-variation-distro"; + std::tie(out, err) = LxsstuLaunchWslAndCaptureOutput( + std::format(L"--import {} {} {} --vhd", negativeVariationDistro, negativeVariationDistro, tempFile.Path), -1); + VERIFY_ARE_EQUAL( + out, + L"The specified file must have the .vhd or .vhdx file extension.\r\nError code: " + L"Wsl/Service/RegisterDistro/WSL_E_IMPORT_FAILED\r\n"); + VERIFY_ARE_EQUAL(err, L""); + } + }; // namespace UnitTests } // namespace UnitTests