Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen volume search logic to simply ignore volumes that have errors #55

Merged
merged 4 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ File clones on Windows do not actually allocate space on-drive for the clone. Th

[![NuGet version (CopyOnWrite)](https://img.shields.io/nuget/v/CopyOnWrite?style=plastic)](https://www.nuget.org/packages/CopyOnWrite)

* 0.4.2 November 2024: Add STATUS_VHD_INVALID_STATE and ERROR_NO_SUCH_DEVICE handling on getting volume information
* 0.4.2 November 2024: Loosened error handling getting volume information, any volume resulting in an error is ignored. This aids continued failures finding new unique error types based on al lthe various disk subsystems and drivers out there.
* 0.4.1 October 2024: Add ERROR_DEV_NOT_EXIST handling on volume enumeration
* 0.4.0 October 2024: Remove async versions of `CloneFile` as the implementation did not use overlapped I/O anyway. CoW support is releasing in the Server 2025 and Win11 24H2 wave, built into the `CopyFile` API suite and on by default for Dev Drive and ReFS, so overlapped I/O in this library will never be implemented. Resolves https://github.com/microsoft/CopyOnWrite/issues/50
* 0.3.12 October 2024: Add ERROR_DEV_NOT_EXIST handling on getting free disk space
Expand Down
2 changes: 1 addition & 1 deletion lib/Windows/NativeMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public struct BY_HANDLE_FILE_INFORMATION
public uint FileIndexHigh;
public uint FileIndexLow;

public long FileSize => ((long)FileSizeHigh << 32) | FileSizeLow;
public readonly long FileSize => ((long)FileSizeHigh << 32) | FileSizeLow;
}

[DllImport("Kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)]
Expand Down
6 changes: 3 additions & 3 deletions lib/Windows/VolumeEnumerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public IEnumerable<VolumePaths> GetVolumesAndVolumePaths()

NativeMethods.QueryDosDevice(drive, lpszDosDeviceNames, singleDriveMappingSize);
string? mappedToVolume = Marshal.PtrToStringUni(lpszDosDeviceNames);
if (mappedToVolume != null)
if (mappedToVolume is not null)
{
if (mappedToVolume.StartsWith(@"\??\", StringComparison.Ordinal))
{
Expand Down Expand Up @@ -219,10 +219,10 @@ private bool GetNextVolume(out string? volumeName)
var sb = new StringBuilder(bufferLen);
volumeName = null;

if (_findHandle == null)
if (_findHandle is null)
{
_findHandle = NativeMethods.FindFirstVolume(sb, bufferLen);
if (_findHandle != null && !_findHandle.IsInvalid)
if (_findHandle is not null && !_findHandle.IsInvalid)
{
volumeName = sb.ToString();
return true;
Expand Down
2 changes: 1 addition & 1 deletion lib/Windows/VolumeInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace Microsoft.CopyOnWrite.Windows;

internal class VolumeInfo
internal sealed class VolumeInfo
{
public VolumeInfo(string primaryDriveLetterRoot, string volumeName, bool supportsCoW, long clusterSize)
{
Expand Down
40 changes: 12 additions & 28 deletions lib/Windows/VolumeInfoCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ internal VolumeInfoCache(IList<(VolumePaths, VolumeInfo?)> volumesAndMountedPath
_driveLetterSubPathsSortedInReverseOrder = new SubPathAndVolume[26][];
for (int i = 0; i < _driveLetterSubPathsSortedInReverseOrder.Length; i++)
{
_driveLetterSubPathsSortedInReverseOrder[i] = Array.Empty<SubPathAndVolume>();
_driveLetterSubPathsSortedInReverseOrder[i] = [];
}

foreach ((VolumePaths volumePaths, VolumeInfo? volumeInfo) in volumesAndMountedPaths)
Expand All @@ -65,7 +65,7 @@ internal VolumeInfoCache(IList<(VolumePaths, VolumeInfo?)> volumesAndMountedPath
// Typical case is for one volume to be mounted at one drive letter root path.
if (existingPaths.Length == 0)
{
_driveLetterSubPathsSortedInReverseOrder[letterIndex] = new[] { newRef };
_driveLetterSubPathsSortedInReverseOrder[letterIndex] = [newRef];
}
else
{
Expand All @@ -82,7 +82,7 @@ internal VolumeInfoCache(IList<(VolumePaths, VolumeInfo?)> volumesAndMountedPath
}
}

public VolumeInfo GetVolumeForPath(string path)
public VolumeInfo? GetVolumeForPath(string path)
{
// Look up paths by drive letter to reduce the size of the resulting path array to search.
int driveLetterIndex = IndexFromDriveLetter(path[0]);
Expand All @@ -91,23 +91,18 @@ public VolumeInfo GetVolumeForPath(string path)
// Paths are sorted in reverse order to get longer paths ahead of shorter paths for prefix matching.
// For cases where volumes are mounted under other volumes, e.g. a D: ReFS drive mounted
// under D:\ReFS, we want to match the deeper path.
foreach (SubPathAndVolume spv in subPathsAndVolumes.Where(spv => spv.Volume is not null))
foreach (SubPathAndVolume spv in subPathsAndVolumes)
{
if (path.IsSubpathOf(spv.SubPath))
{
return spv.Volume!;
return spv.Volume;
}
}

throw new ArgumentException($"No known volume information for '{path}'. " +
"If the drive was added recently you may need to recreate the filesystem cache.");
}

private const int ERROR_NOT_READY = 21;
private const int ERROR_INVALID_PARAMETER = 87;
private const int ERROR_UNRECOGNIZED_VOLUME = 1005;
private const int FVE_E_LOCKED_VOLUME = unchecked((int)0x80310000);

private static VolumeInfo? GetVolumeInfo(VolumePaths volumePaths)
{
bool result = NativeMethods.GetVolumeInformation(
Expand All @@ -121,29 +116,18 @@ public VolumeInfo GetVolumeForPath(string path)
0);
if (!result)
{
int lastErr = Marshal.GetLastWin32Error();

// Ignore:
// Ignore volumes that have errors.
// Caveat: We don't have a callback to inform the caller that we cannot enumerate a volume,
// which means CoW will simply invisibly not work. However, attempts to list individual
// error codes proved difficult and prone to creating tons of new PRs after various deployments.
//
// Examples of things we are ignoring here:
// - Some SD Card readers show a drive letter even when empty.
// - BitLocker can have a volume locked.
// - Access denied can imply a volume needing escalated privilege to get its metadata,
// sometimes indicating a Windows container volume.
// - Not found can occur as a timing issue on some machines.
if (lastErr == ERROR_UNRECOGNIZED_VOLUME ||
lastErr == ERROR_NOT_READY ||
lastErr == ERROR_INVALID_PARAMETER ||
lastErr == FVE_E_LOCKED_VOLUME ||
lastErr == NativeMethods.ERROR_ACCESS_DENIED ||
lastErr == NativeMethods.ERROR_FILE_NOT_FOUND ||
lastErr == NativeMethods.ERROR_DEV_NOT_EXIST ||
lastErr == NativeMethods.ERROR_NO_SUCH_DEVICE ||
lastErr == NativeMethods.STATUS_VHD_INVALID_STATE)
{
return null;
}

NativeMethods.ThrowSpecificIoException(lastErr,
$"Failed retrieving volume information for {volumePaths.PrimaryDriveRootPath} with winerror {lastErr}");
return null;
}

result = NativeMethods.GetDiskFreeSpace(
Expand Down
16 changes: 8 additions & 8 deletions lib/Windows/WindowsCopyOnWriteFilesystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destinati
}

// Must be in the same volume.
VolumeInfo sourceVolume = _volumeInfoCache.GetVolumeForPath(resolvedSource);
VolumeInfo destVolume = _volumeInfoCache.GetVolumeForPath(resolvedDestination);
if (!ReferenceEquals(sourceVolume, destVolume))
VolumeInfo? sourceVolume = _volumeInfoCache.GetVolumeForPath(resolvedSource);
VolumeInfo? destVolume = _volumeInfoCache.GetVolumeForPath(resolvedDestination);
if (sourceVolume is null || destVolume is null || !ReferenceEquals(sourceVolume, destVolume))
{
return false;
}
Expand All @@ -62,8 +62,8 @@ public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory, bool p
return false;
}

VolumeInfo volumeInfo = _volumeInfoCache.GetVolumeForPath(resolvedSource);
return volumeInfo.SupportsCoW;
VolumeInfo? volumeInfo = _volumeInfoCache.GetVolumeForPath(resolvedSource);
return volumeInfo?.SupportsCoW ?? false;
}

/// <inheritdoc />
Expand All @@ -84,10 +84,10 @@ public void CloneFile(string source, string destination, CloneFlags cloneFlags)
throw new NotSupportedException($"Source path '{source}' is not in the correct format");
}

VolumeInfo sourceVolume = _volumeInfoCache.GetVolumeForPath(resolvedSource);
if (!sourceVolume.SupportsCoW)
VolumeInfo? sourceVolume = _volumeInfoCache.GetVolumeForPath(resolvedSource);
if (sourceVolume is null || !sourceVolume.SupportsCoW)
{
throw new NotSupportedException($@"Drive volume {sourceVolume.PrimaryDriveLetterRoot} does not support copy-on-write clone links, i.e. is not formatted with ReFS");
throw new NotSupportedException($@"Drive volume {sourceVolume?.PrimaryDriveLetterRoot} does not support copy-on-write clone links, i.e. is not formatted with ReFS");
}

// Get an open file handle to the source file. We use FILE_FLAGS_NO_BUFFERING here
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/DisposableTempDirectory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void Dispose()
}

// Restore the previous current directory if necessary
if (_previousCurrentDirectory != null)
if (_previousCurrentDirectory is not null)
{
Environment.CurrentDirectory = _previousCurrentDirectory;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/WindowsReFsDriveSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public static WindowsReFsDriveSession Create(string relativeTestRootDir, bool fo
public void Dispose()
{
Directory.Delete(Path.GetDirectoryName(TestRootDir)!, recursive: true);
if (_removeVhdScriptPath != null)
if (_removeVhdScriptPath is not null)
{
RunPowershellScript(_removeVhdScriptPath, $"{_vhdDriveLetter}");
}
Expand Down
Loading