Skip to content

Commit f4b67e3

Browse files
benhillisBen Hillis
andauthored
cleanup: switch drvfs.cpp to use common MountUtil helper (#13890)
* cleanup: switch drvfs.cpp to use common MountUtil helper * adjust mount.drvfs error logging * adjust mount.drvfs error logging --------- Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
1 parent cb8bc9a commit f4b67e3

File tree

3 files changed

+34
-53
lines changed

3 files changed

+34
-53
lines changed

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ bin/
4444
*.nupkg
4545
build/
4646
generated/
47-
Microsoft.WSL.PluginApi.nuspec
47+
*.nuspec
4848
test/linux/unit_tests/wsl_unit_tests
4949
*.dir/
5050
UserConfig.cmake

src/linux/init/drvfs.cpp

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,7 @@ int MountWithRetry(const char* Source, const char* Target, const char* FsType, c
220220
221221
Routine Description:
222222
223-
This routine will perform a mount using the /bin/mount binary, and will
224-
retry it if it fails.
225-
226-
N.B. This routine is used for virtio-9p and virtiofs which can transiently
227-
fail to mount if the PCI device isn't ready yet.
223+
This routine performs a mount with retry logic for DrvFs filesystems.
228224
229225
Arguments:
230226
@@ -246,52 +242,19 @@ Return Value:
246242

247243
try
248244
{
249-
int Result;
250-
try
245+
//
246+
// Verify the target directory exists before mounting.
247+
//
248+
249+
int Result = access(Target, F_OK);
250+
if (Result < 0)
251251
{
252-
bool PrintWarning = true;
253-
wsl::shared::retry::RetryWithTimeout<void>(
254-
[&]() { THROW_LAST_ERROR_IF(mountutil::MountFilesystem(Source, Target, FsType, Options) < 0); },
255-
std::chrono::milliseconds{100},
256-
std::chrono::seconds{2},
257-
[&]() {
258-
// For virtio-9p, there are two errors that could indicate the PCI device is not ready:
259-
// EBUSY - Returned if all devices with the tag are already in use.
260-
// ENOENT - Returned if there are no devices with the tag, which can happen after the VM first boots.
261-
// In this case, check if the target exists; if it doesn't, ENOENT is for that and there's no reason to retry.
262-
//
263-
// For virtiofs, EINVAL will be returned if the tag is not ready.
264-
auto savedErrno = wil::ResultFromCaughtException();
265-
if (strcmp(FsType, PLAN9_FS_TYPE) == 0)
266-
{
267-
if (errno != EBUSY && (errno != ENOENT || access(Target, F_OK) != 0))
268-
{
269-
errno = savedErrno;
270-
return false;
271-
}
272-
}
273-
else if ((strcmp(FsType, VIRTIO_FS_TYPE) == 0) && (errno != EINVAL))
274-
{
275-
return false;
276-
}
277-
278-
if (PrintWarning)
279-
{
280-
LOG_WARNING("mount: waiting for virtio device {}", Source);
281-
PrintWarning = false;
282-
}
283-
284-
return true;
285-
});
286-
287-
Result = 0;
252+
LOG_STDERR(errno);
288253
}
289-
catch (...)
254+
else
290255
{
291-
errno = wil::ResultFromCaughtException();
292-
auto parsed = mountutil::MountParseFlags(Options);
293-
LOG_ERROR("mount({}, {}, {}, {:#x}, {}) failed: {}", Source, Target, FsType, parsed.MountFlags, parsed.StringOptions.c_str(), strerror(errno));
294-
Result = -1;
256+
auto Parsed = mountutil::MountParseFlags(Options);
257+
Result = UtilMount(Source, Target, FsType, Parsed.MountFlags, Parsed.StringOptions.c_str(), std::chrono::seconds{2});
295258
}
296259

297260
if (ExitCode)

src/linux/init/util.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,8 +1706,10 @@ Return Value:
17061706
//
17071707
// Mount the device to the mount point.
17081708
//
1709-
// N.B. The mount operation is only retried if the mount source does not yet exist,
1710-
// which can happen when hot-added devices are not yet available in the guest.
1709+
// N.B. The mount operation is retried if:
1710+
// - The mount source does not yet exist (hot-added devices)
1711+
// - For Plan9 (9p): device is busy or not found
1712+
// - For VirtioFS: invalid tag (device not ready)
17111713
//
17121714

17131715
try
@@ -1720,7 +1722,23 @@ Return Value:
17201722
TimeoutSeconds.value(),
17211723
[&]() {
17221724
errno = wil::ResultFromCaughtException();
1723-
return errno == ENOENT || errno == ENXIO || errno == EIO;
1725+
1726+
// Generic device not ready errors
1727+
if (errno == ENXIO || errno == EIO || errno == ENOENT)
1728+
{
1729+
return true;
1730+
}
1731+
1732+
// Filesystem-specific device readiness errors
1733+
if (Type != nullptr)
1734+
{
1735+
if ((strcmp(Type, PLAN9_FS_TYPE) == 0 && errno == EBUSY) || (strcmp(Type, VIRTIO_FS_TYPE) == 0 && errno == EINVAL))
1736+
{
1737+
return true;
1738+
}
1739+
}
1740+
1741+
return false;
17241742
});
17251743
}
17261744
else
@@ -1731,7 +1749,7 @@ Return Value:
17311749
catch (...)
17321750
{
17331751
errno = wil::ResultFromCaughtException();
1734-
LOG_ERROR("mount({}, {}, {}, 0x{}x, {}) failed {}", Source, Target, Type, MountFlags, Options, errno);
1752+
LOG_ERROR("mount({}, {}, {}, {:#x}, {}) failed {}", Source, Target, Type, MountFlags, Options, errno);
17351753
return -errno;
17361754
}
17371755

0 commit comments

Comments
 (0)