Skip to content

Commit 9e9ef6f

Browse files
benhillisBen HillisCopilot
authored
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 <benhill@ntdev.microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent c1b77b1 commit 9e9ef6f

File tree

8 files changed

+142
-55
lines changed

8 files changed

+142
-55
lines changed

‎localization/strings/en-US/Resources.resw‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ Arguments for managing Windows Subsystem for Linux:
466466
Move the distribution to a new location.
467467

468468
--set-sparse, -s &lt;true|false&gt;
469-
Set the vhdx of distro to be sparse, allowing disk space to be automatically reclaimed.
469+
Set the VHD of distro to be sparse, allowing disk space to be automatically reclaimed.
470470

471471
--set-default-user &lt;Username&gt;
472472
Set the default user of the distribution.
@@ -546,11 +546,11 @@ Arguments for managing distributions in Windows Subsystem for Linux:
546546
Specifies the version to use for the new distribution.
547547

548548
--vhd
549-
Specifies that the provided file is a .vhdx file, not a tar file.
550-
This operation makes a copy of the .vhdx file at the specified install location.
549+
Specifies that the provided file is a .vhd or .vhdx file, not a tar file.
550+
This operation makes a copy of the VHD file at the specified install location.
551551

552552
--import-in-place &lt;Distro&gt; &lt;FileName&gt;
553-
Imports the specified .vhdx file as a new distribution.
553+
Imports the specified VHD file as a new distribution.
554554
This virtual hard disk must be formatted with the ext4 filesystem type.
555555

556556
--list, -l [Options]
@@ -626,7 +626,7 @@ Build time: {}</value>
626626
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
627627
</data>
628628
<data name="MessageCustomKernelModulesNotFound" xml:space="preserve">
629-
<value>The custom kernel modules vhd in {} was not found: '{}'.</value>
629+
<value>The custom kernel modules VHD in {} was not found: '{}'.</value>
630630
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
631631
</data>
632632
<data name="MessageCustomSystemDistroError" xml:space="preserve">
@@ -704,8 +704,13 @@ The system may need to be restarted so the changes can take effect.</value>
704704
<comment>{Locked="--install "}{Locked="--no-distribution
705705
"}Command line arguments, file names and string inserts should not be translated</comment>
706706
</data>
707-
<data name="MessageRequiresVhdxFileExtension" xml:space="preserve">
708-
<value>The specified file must have the .vhdx file extension.</value>
707+
<data name="MessageRequiresFileExtension" xml:space="preserve">
708+
<value>The specified file must have the {} file extension.</value>
709+
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
710+
</data>
711+
<data name="MessageRequiresFileExtensions" xml:space="preserve">
712+
<value>The specified file must have the {} or {} file extension.</value>
713+
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
709714
</data>
710715
<data name="MessageVmSwitchNotFound" xml:space="preserve">
711716
<value>The VmSwitch '{}' was not found. Available switches: {}</value>
@@ -998,15 +1003,15 @@ Falling back to NAT networking.</value>
9981003
<value>See Docs</value>
9991004
</data>
10001005
<data name="MessageVhdInUse" xml:space="preserve">
1001-
<value>The operation could not be completed because the vhdx is currently in use. To force WSL to stop use: wsl.exe --shutdown</value>
1006+
<value>The operation could not be completed because the VHD is currently in use. To force WSL to stop use: wsl.exe --shutdown</value>
10021007
<comment>{Locked="--shutdown"}Command line arguments, file names and string inserts should not be translated</comment>
10031008
</data>
10041009
<data name="MessageInvalidBoolean" xml:space="preserve">
10051010
<value>{} is not a valid boolean, &lt;true|false&gt;</value>
10061011
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
10071012
</data>
10081013
<data name="MessageSparseVhdWsl2Only" xml:space="preserve">
1009-
<value>Sparse vhdx is supported on WSL2 only.</value>
1014+
<value>Sparse VHD is supported on WSL2 only.</value>
10101015
</data>
10111016
<data name="MessageLocalSystemNotSupported" xml:space="preserve">
10121017
<value>Running WSL as local system is not supported.</value>
@@ -1055,7 +1060,7 @@ Falling back to NAT networking.</value>
10551060
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
10561061
</data>
10571062
<data name="MessagePassVhdFlag" xml:space="preserve">
1058-
<value>This looks like a VHDX file. Use --vhd to import a VHDX instead of a tar.</value>
1063+
<value>This looks like a VHD file. Use --vhd to import a VHD instead of a tar.</value>
10591064
<comment>{Locked="--vhd "}Command line arguments, file names and string inserts should not be translated</comment>
10601065
</data>
10611066
<data name="MessageDistroStoreInstallFailed" xml:space="preserve">
@@ -1107,7 +1112,7 @@ Error code: {}</value>
11071112
</data>
11081113
<data name="MessageSparseVhdDisabled" xml:space="preserve">
11091114
<value>Sparse VHD support is currently disabled due to potential data corruption.
1110-
To force a distribution to use a sparse vhd, please run:
1115+
To force a distribution to use a sparse VHD, please run:
11111116
wsl.exe --manage &lt;DistributionName&gt; --set-sparse true --allow-unsafe</value>
11121117
<comment>{Locked="--manage "}{Locked="--set-sparse "}{Locked="--allow-unsafe"}Command line arguments, file names and string inserts should not be translated</comment>
11131118
</data>

‎src/windows/common/WslClient.cpp‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,6 @@ int ExportDistribution(_In_ std::wstring_view commandLine)
275275
}
276276
else
277277
{
278-
// If exporting to a vhd, ensure the filename ends with the vhdx file extension.
279-
if (WI_IsFlagSet(flags, LXSS_EXPORT_DISTRO_FLAGS_VHD) &&
280-
!wsl::windows::common::string::IsPathComponentEqual(filePath.extension().native(), wsl::windows::common::wslutil::c_vhdxFileExtension))
281-
{
282-
wsl::windows::common::wslutil::PrintMessage(wsl::shared::Localization::MessageRequiresVhdxFileExtension());
283-
return -1;
284-
}
285-
286278
file.reset(CreateFileW(
287279
filePath.c_str(), GENERIC_WRITE, (FILE_SHARE_READ | FILE_SHARE_DELETE), nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr));
288280

@@ -371,22 +363,10 @@ int ImportDistribution(_In_ std::wstring_view commandLine)
371363
}
372364
else
373365
{
374-
bool isVhd = wsl::windows::common::string::IsPathComponentEqual(
375-
filePath.extension().native(), wsl::windows::common::wslutil::c_vhdxFileExtension);
376-
377-
if (WI_IsFlagSet(flags, LXSS_IMPORT_DISTRO_FLAGS_VHD))
378-
{
379-
// If importing from a vhd, ensure the filename ends with the vhdx file extension.
380-
if (!isVhd)
381-
{
382-
wsl::windows::common::wslutil::PrintMessage(wsl::shared::Localization::MessageRequiresVhdxFileExtension());
383-
return -1;
384-
}
385-
}
386-
else
366+
if (WI_IsFlagClear(flags, LXSS_IMPORT_DISTRO_FLAGS_VHD))
387367
{
388-
// Fail if we expect a tar, but the file name has the .vhdx extension.
389-
if (isVhd)
368+
// Fail if expecting a tar, but the file name has the .vhd or .vhdx extension.
369+
if (wsl::windows::common::wslutil::IsVhdFile(filePath))
390370
{
391371
wsl::windows::common::wslutil::PrintMessage(wsl::shared::Localization::MessagePassVhdFlag());
392372
return -1;

‎src/windows/common/wslutil.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,13 @@ bool wsl::windows::common::wslutil::IsRunningInMsix()
11551155
return false;
11561156
}
11571157
}
1158+
1159+
bool wsl::windows::common::wslutil::IsVhdFile(_In_ const std::filesystem::path& path)
1160+
{
1161+
return wsl::windows::common::string::IsPathComponentEqual(path.extension().native(), c_vhdFileExtension) ||
1162+
wsl::windows::common::string::IsPathComponentEqual(path.extension().native(), c_vhdxFileExtension);
1163+
}
1164+
11581165
std::vector<DWORD> wsl::windows::common::wslutil::ListRunningProcesses()
11591166
{
11601167
std::vector<DWORD> pids(1024);

‎src/windows/common/wslutil.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ void InitializeWil();
122122

123123
bool IsRunningInMsix();
124124

125+
bool IsVhdFile(_In_ const std::filesystem::path& path);
126+
125127
bool IsVirtualMachinePlatformInstalled();
126128

127129
std::vector<DWORD> ListRunningProcesses();

‎src/windows/service/exe/LxssUserSession.cpp‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW
927927
std::filesystem::path newVhdPath = Location;
928928
RETURN_HR_IF(E_INVALIDARG, newVhdPath.empty());
929929

930-
newVhdPath /= LXSS_VM_MODE_VHD_NAME;
930+
newVhdPath /= distro.VhdFilePath.filename();
931931

932932
auto impersonate = wil::CoImpersonateClient();
933933

@@ -952,7 +952,7 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW
952952

953953
// Update the registry location
954954
registration.Write(Property::BasePath, Location);
955-
registration.Write(Property::VhdFileName, LXSS_VM_MODE_VHD_NAME);
955+
registration.Write(Property::VhdFileName, newVhdPath.filename().c_str());
956956

957957
revert.release();
958958

@@ -1079,6 +1079,22 @@ HRESULT LxssUserSessionImpl::ExportDistribution(_In_opt_ LPCGUID DistroGuid, _In
10791079
{
10801080
const wil::unique_handle userToken = wsl::windows::common::security::GetUserToken(TokenImpersonation);
10811081
auto runAsUser = wil::impersonate_token(userToken.get());
1082+
1083+
// Ensure the target file has the correct file extension.
1084+
if (GetFileType(FileHandle) == FILE_TYPE_DISK)
1085+
{
1086+
std::wstring exportPath;
1087+
THROW_IF_FAILED(wil::GetFinalPathNameByHandleW(FileHandle, exportPath));
1088+
1089+
const auto sourceFileExtension = configuration.VhdFilePath.extension().native();
1090+
const auto targetFileExtension = std::filesystem::path(exportPath).extension().native();
1091+
if (!wsl::windows::common::string::IsPathComponentEqual(sourceFileExtension, targetFileExtension))
1092+
{
1093+
THROW_HR_WITH_USER_ERROR(
1094+
WSL_E_EXPORT_FAILED, wsl::shared::Localization::MessageRequiresFileExtension(sourceFileExtension.c_str()));
1095+
}
1096+
}
1097+
10821098
const wil::unique_hfile vhdFile(CreateFileW(
10831099
configuration.VhdFilePath.c_str(), GENERIC_READ, (FILE_SHARE_READ | FILE_SHARE_DELETE), nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr));
10841100

@@ -1258,13 +1274,9 @@ LxssUserSessionImpl::ImportDistributionInplace(_In_ LPCWSTR DistributionName, _I
12581274

12591275
s_ValidateDistroName(DistributionName);
12601276

1261-
// Return an error if the path is not absolute or does not end in the .vhdx file extension.
1277+
// Return an error if the path is not absolute or does not have a valid VHD file extension.
12621278
const std::filesystem::path path{VhdPath};
1263-
RETURN_HR_IF(
1264-
E_INVALIDARG,
1265-
!path.is_absolute() ||
1266-
(!wsl::windows::common::string::IsPathComponentEqual(path.extension().native(), wsl::windows::common::wslutil::c_vhdFileExtension) &&
1267-
!wsl::windows::common::string::IsPathComponentEqual(path.extension().c_str(), wsl::windows::common::wslutil::c_vhdxFileExtension)));
1279+
RETURN_HR_IF(E_INVALIDARG, !path.is_absolute() || !wsl::windows::common::wslutil::IsVhdFile(path));
12681280

12691281
const wil::unique_hkey lxssKey = s_OpenLxssUserKey();
12701282
std::lock_guard lock(m_instanceLock);
@@ -1448,6 +1460,24 @@ HRESULT LxssUserSessionImpl::RegisterDistribution(
14481460
wil::CreateDirectoryDeep(distributionPath.c_str());
14491461
}
14501462

1463+
// If importing a vhd, determine if it is a .vhd or .vhdx.
1464+
std::wstring vhdName{LXSS_VM_MODE_VHD_NAME};
1465+
if ((WI_IsFlagSet(Flags, LXSS_IMPORT_DISTRO_FLAGS_VHD)) && (GetFileType(FileHandle) == FILE_TYPE_DISK))
1466+
{
1467+
std::wstring pathBuffer;
1468+
THROW_IF_FAILED(wil::GetFinalPathNameByHandleW(FileHandle, pathBuffer));
1469+
1470+
std::filesystem::path vhdPath{std::move(pathBuffer)};
1471+
if (!wsl::windows::common::wslutil::IsVhdFile(vhdPath))
1472+
{
1473+
using namespace wsl::windows::common::wslutil;
1474+
THROW_HR_WITH_USER_ERROR(
1475+
WSL_E_IMPORT_FAILED, wsl::shared::Localization::MessageRequiresFileExtensions(c_vhdFileExtension, c_vhdxFileExtension));
1476+
}
1477+
1478+
vhdName = vhdPath.filename();
1479+
}
1480+
14511481
registration = DistributionRegistration::Create(
14521482
lxssKey.get(),
14531483
DistributionId,
@@ -1457,7 +1487,7 @@ HRESULT LxssUserSessionImpl::RegisterDistribution(
14571487
flags,
14581488
LX_UID_ROOT,
14591489
PackageFamilyName,
1460-
LXSS_VM_MODE_VHD_NAME,
1490+
vhdName.c_str(),
14611491
WI_IsFlagClear(Flags, LXSS_IMPORT_DISTRO_FLAGS_NO_OOBE));
14621492

14631493
configuration = s_GetDistributionConfiguration(registration, DistributionName == nullptr);

‎src/windows/service/exe/WslCoreFilesystem.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,7 @@ void wsl::core::filesystem::CreateVhd(_In_ LPCWSTR target, _In_ ULONGLONG maximu
7171

7272
wil::unique_handle wsl::core::filesystem::OpenVhd(_In_ LPCWSTR Path, _In_ VIRTUAL_DISK_ACCESS_MASK Mask)
7373
{
74-
WI_ASSERT(
75-
wsl::shared::string::IsEqual(std::filesystem::path{Path}.extension().c_str(), windows::common::wslutil::c_vhdFileExtension, true) ||
76-
wsl::shared::string::IsEqual(std::filesystem::path{Path}.extension().c_str(), windows::common::wslutil::c_vhdxFileExtension, true));
74+
WI_ASSERT(wsl::windows::common::wslutil::IsVhdFile(std::filesystem::path{Path}));
7775

7876
// N.B. Specifying unknown for device and vendor means the system will determine the type of VHD.
7977
VIRTUAL_STORAGE_TYPE storageType{};

‎test/windows/SimpleTests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class SimpleTests
109109
std::format(L"{} {} {} {}", WSL_IMPORT_ARG, tempDistro, vhdDir.wstring(), tar.wstring()).c_str(),
110110
L"The operation completed successfully. \r\n",
111111
L"wsl: Sparse VHD support is currently disabled due to potential data corruption.\r\n"
112-
L"To force a distribution to use a sparse vhd, please run:\r\n"
112+
L"To force a distribution to use a sparse VHD, please run:\r\n"
113113
L"wsl.exe --manage <DistributionName> --set-sparse true --allow-unsafe\r\n",
114114
0);
115115

@@ -122,7 +122,7 @@ class SimpleTests
122122
ValidateOutput(
123123
std::format(L"{} {} {} {}", WSL_MANAGE_ARG, tempDistro, WSL_MANAGE_ARG_SET_SPARSE_OPTION_LONG, L"true").c_str(),
124124
L"Sparse VHD support is currently disabled due to potential data corruption.\r\n"
125-
L"To force a distribution to use a sparse vhd, please run:\r\n"
125+
L"To force a distribution to use a sparse VHD, please run:\r\n"
126126
L"wsl.exe --manage <DistributionName> --set-sparse true --allow-unsafe\r\nError code: Wsl/Service/E_INVALIDARG\r\n",
127127
L"",
128128
-1);

0 commit comments

Comments
 (0)