From 6be21bbdbc4549e9dd737b68e96c150495263369 Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Tue, 25 Jun 2013 18:39:07 +0100 Subject: [PATCH] [core] more drive handling improvements * Use retries when locking the volume * Wait for volume availability after partitioning * Don't halt on errors that can be ignored * Other small improvements and fixes * The above should help with addressing #122 --- src/drive.c | 64 ++++++++++++++++++++++++++-------- src/format.c | 97 +++++++++++++++++++++++++++------------------------- src/rufus.h | 10 +++--- src/rufus.rc | 10 +++--- 4 files changed, 111 insertions(+), 70 deletions(-) diff --git a/src/drive.c b/src/drive.c index af817788..2ad0a634 100644 --- a/src/drive.c +++ b/src/drive.c @@ -56,6 +56,7 @@ extern BOOL enable_fixed_disks; */ static HANDLE GetHandle(char* Path, BOOL bWriteAccess, BOOL bLockDrive) { + int i; DWORD size; HANDLE hDrive = INVALID_HANDLE_VALUE; @@ -72,10 +73,17 @@ static HANDLE GetHandle(char* Path, BOOL bWriteAccess, BOOL bLockDrive) uprintf("Caution: Opened drive %s for write access\n", Path); } - if ((bLockDrive) && (!DeviceIoControl(hDrive, FSCTL_LOCK_VOLUME, NULL, 0, NULL, 0, &size, NULL))) { + if (bLockDrive) { + for (i = 0; i < DRIVE_ACCESS_RETRIES; i++) { + if (DeviceIoControl(hDrive, FSCTL_LOCK_VOLUME, NULL, 0, NULL, 0, &size, NULL)) + goto out; + if (IS_ERROR(FormatStatus)) // User cancel + break; + Sleep(DRIVE_ACCESS_TIMEOUT/DRIVE_ACCESS_RETRIES); + } + // If we reached this section, either we didn't manage to get a lock or the user cancelled uprintf("Could not get exclusive access to device %s: %s\n", Path, WindowsErrorString()); safe_closehandle(hDrive); - goto out; } out: @@ -114,7 +122,8 @@ HANDLE GetPhysicalHandle(DWORD DriveIndex, BOOL bWriteAccess, BOOL bLockDrive) // See http://msdn.microsoft.com/en-us/library/cc542456.aspx // The returned string is allocated and must be freed // TODO: a drive may have multiple volumes - should we handle those? -char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash) +#define suprintf(...) if (!bSilent) uprintf(__VA_ARGS__) +char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash, BOOL bSilent) { BOOL success = FALSE; char volume_name[MAX_PATH]; @@ -134,13 +143,13 @@ char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash) if (i == 0) { hVolume = FindFirstVolumeA(volume_name, sizeof(volume_name)); if (hVolume == INVALID_HANDLE_VALUE) { - uprintf("Could not access first GUID volume: %s\n", WindowsErrorString()); + suprintf("Could not access first GUID volume: %s\n", WindowsErrorString()); goto out; } } else { if (!FindNextVolumeA(hVolume, volume_name, sizeof(volume_name))) { if (GetLastError() != ERROR_NO_MORE_FILES) { - uprintf("Could not access next GUID volume: %s\n", WindowsErrorString()); + suprintf("Could not access next GUID volume: %s\n", WindowsErrorString()); } goto out; } @@ -149,7 +158,7 @@ char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash) // Sanity checks len = safe_strlen(volume_name); if ((len <= 1) || (safe_strnicmp(volume_name, volume_start, 4) != 0) || (volume_name[len-1] != '\\')) { - uprintf("'%s' is not a GUID volume name\n", volume_name); + suprintf("'%s' is not a GUID volume name\n", volume_name); continue; } @@ -162,27 +171,27 @@ char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash) volume_name[len-1] = 0; if (QueryDosDeviceA(&volume_name[4], path, sizeof(path)) == 0) { - uprintf("Failed to get device path for GUID volume '%s': %s\n", volume_name, WindowsErrorString()); + suprintf("Failed to get device path for GUID volume '%s': %s\n", volume_name, WindowsErrorString()); continue; } for (j=0; (j 32 GB FAT32 static BOOL FormatFAT32(DWORD DriveIndex) { BOOL r = FALSE; @@ -369,17 +368,14 @@ static BOOL FormatFAT32(DWORD DriveIndex) PrintStatus(0, TRUE, "Formatting (Large FAT32)..."); VolumeId = GetVolumeID(); - // Open the drive (volume should already be locked) - hLogicalVolume = GetLogicalHandle(DriveIndex, TRUE, FALSE); + // Open the drive and lock it + hLogicalVolume = GetLogicalHandle(DriveIndex, TRUE, TRUE); if (IS_ERROR(FormatStatus)) goto out; if ((hLogicalVolume == INVALID_HANDLE_VALUE) || (hLogicalVolume == NULL)) die("Invalid logical volume handle\n", ERROR_INVALID_HANDLE); - // Make sure we get exclusive access - if (!UnmountVolume(hLogicalVolume)) { - FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_ACCESS_DENIED; - goto out; - } + // Try to disappear the volume while we're formatting it + UnmountVolume(hLogicalVolume); // Work out drive params if (!DeviceIoControl (hLogicalVolume, IOCTL_DISK_GET_DRIVE_GEOMETRY, NULL, 0, &dgDrive, @@ -583,7 +579,7 @@ static BOOL FormatFAT32(DWORD DriveIndex) // Handle must be closed for SetVolumeLabel to work safe_closehandle(hLogicalVolume); PrintStatus(0, TRUE, "Setting Label (This may take while)..."); - VolumeName = GetLogicalName(DriveIndex, TRUE); + VolumeName = GetLogicalName(DriveIndex, TRUE, TRUE); wVolumeName = utf8_to_wchar(VolumeName); if ((wVolumeName == NULL) || (!SetVolumeLabelW(wVolumeName, wLabel))) { uprintf("Could not set label: %s\n", WindowsErrorString()); @@ -622,7 +618,7 @@ static BOOL FormatDrive(DWORD DriveIndex) GetWindowTextA(hFileSystem, FSType, ARRAYSIZE(FSType)); safe_sprintf(format_status, ARRAYSIZE(format_status), "Formatting (%s)...", FSType); PrintStatus(0, TRUE, format_status); - VolumeName = GetLogicalName(DriveIndex, FALSE); + VolumeName = GetLogicalName(DriveIndex, FALSE, TRUE); wVolumeName = utf8_to_wchar(VolumeName); if (wVolumeName == NULL) { uprintf("Could not read volume name\n"); @@ -822,6 +818,7 @@ BOOL WriteRufusMBR(FILE *fp) static BOOL WriteMBR(HANDLE hPhysicalDrive) { BOOL r = FALSE; + DWORD size; int dt, fs; unsigned char* buf = NULL; size_t SecSize = SelectedDrive.Geometry.BytesPerSector; @@ -891,6 +888,9 @@ static BOOL WriteMBR(HANDLE hPhysicalDrive) } } + // Tell the system we've updated the disk properties + DeviceIoControl(hPhysicalDrive, IOCTL_DISK_UPDATE_PROPERTIES, NULL, 0, NULL, 0, &size, NULL ); + out: safe_free(buf); return r; @@ -1095,14 +1095,15 @@ out: */ static BOOL MountVolume(char* drive_name, char *drive_guid) { - char mounted_guid[50]; + char mounted_guid[52]; // You need at least 51 characters on XP if (!SetVolumeMountPointA(drive_name, drive_guid)) { // If the OS was faster than us at remounting the drive, this operation can fail // with ERROR_DIR_NOT_EMPTY. If that's the case, just check that mountpoints match if (GetLastError() == ERROR_DIR_NOT_EMPTY) { if (!GetVolumeNameForVolumeMountPointA(drive_name, mounted_guid, sizeof(mounted_guid))) { - uprintf("%s already mounted, but volume GUID could not be checked\n", drive_name); + uprintf("%s already mounted, but volume GUID could not be checked: %s\n", + drive_name, WindowsErrorString()); return FALSE; } if (safe_strcmp(drive_guid, mounted_guid) != 0) { @@ -1123,7 +1124,7 @@ static BOOL MountVolume(char* drive_name, char *drive_guid) */ static BOOL RemountVolume(char* drive_name) { - char drive_guid[50]; + char drive_guid[51]; if (GetVolumeNameForVolumeMountPointA(drive_name, drive_guid, sizeof(drive_guid))) { if (DeleteVolumeMountPointA(drive_name)) { @@ -1198,6 +1199,7 @@ DWORD WINAPI CloseFormatPromptThread(LPVOID param) { * Unlock the volume. * Close the volume handle. */ +#define CHECK_FOR_USER_CANCEL if (IS_ERROR(FormatStatus)) goto out DWORD WINAPI FormatThread(LPVOID param) { int r, pt, bt, fs, dt; @@ -1243,7 +1245,6 @@ DWORD WINAPI FormatThread(LPVOID param) uprintf("Will use '%c:' as volume mountpoint\n", drive_name[0]); // ...but we need a lock to the logical drive to be able to write anything to it - // TODO: Use a retry loop for the lock hLogicalVolume = GetLogicalHandle(DriveIndex, FALSE, TRUE); if (hLogicalVolume == INVALID_HANDLE_VALUE) { uprintf("Could not lock volume\n"); @@ -1252,11 +1253,10 @@ DWORD WINAPI FormatThread(LPVOID param) } else if (hLogicalVolume == NULL) { // NULL is returned for cases where the drive is not yet partitioned uprintf("Drive does not appear to be partitioned\n"); - } else { - if (!UnmountVolume(hLogicalVolume)) - uprintf("Trying to continue regardless...\n"); + } else if (!UnmountVolume(hLogicalVolume)) { + uprintf("Trying to continue regardless...\n"); } - if (FormatStatus) goto out; // Check for user cancel + CHECK_FOR_USER_CANCEL; PrintStatus(0, TRUE, "Analyzing existing boot records...\n"); AnalyzeMBR(hPhysicalDrive); @@ -1276,7 +1276,7 @@ DWORD WINAPI FormatThread(LPVOID param) if (IsChecked(IDC_BADBLOCKS)) { do { // create a log file for bad blocks report. Since %USERPROFILE% may - // have localised characters, we use the UTF-8 API. + // have localized characters, we use the UTF-8 API. userdir = getenvU("USERPROFILE"); safe_strcpy(logfile, MAX_PATH, userdir); safe_free(userdir); @@ -1296,9 +1296,8 @@ DWORD WINAPI FormatThread(LPVOID param) if (!BadBlocks(hPhysicalDrive, SelectedDrive.DiskSize, SelectedDrive.Geometry.BytesPerSector, ComboBox_GetCurSel(hNBPasses)+1, &report, log_fd)) { uprintf("Bad blocks: Check failed.\n"); - if (!FormatStatus) - FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)| - APPERR(ERROR_BADBLOCKS_FAILURE); + if (!IS_ERROR(FormatStatus)) + FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|APPERR(ERROR_BADBLOCKS_FAILURE); ClearMBRGPT(hPhysicalDrive, SelectedDrive.DiskSize, SelectedDrive.Geometry.BytesPerSector); fclose(log_fd); _unlink(logfile); @@ -1333,8 +1332,7 @@ DWORD WINAPI FormatThread(LPVOID param) goto out; } } - // Close the (unmounted) volume before formatting, but keep the lock - // According to MS this relinquishes the lock, so... + // Close the (unmounted) volume before formatting if (hLogicalVolume != NULL) { PrintStatus(0, TRUE, "Closing existing volume...\n"); if (!CloseHandle(hLogicalVolume)) { @@ -1351,12 +1349,12 @@ DWORD WINAPI FormatThread(LPVOID param) // before repartitioning. Else, all kind of bad things can happen. if (!ClearMBRGPT(hPhysicalDrive, SelectedDrive.DiskSize, SelectedDrive.Geometry.BytesPerSector)) { uprintf("unable to zero MBR/GPT\n"); - if (!FormatStatus) + if (!IS_ERROR(FormatStatus)) FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_WRITE_FAULT; goto out; } - if (FormatStatus) goto out; // Check for user cancel UpdateProgress(OP_ZERO_MBR, -1.0f); + CHECK_FOR_USER_CANCEL; CreateThread(NULL, 0, CloseFormatPromptThread, NULL, 0, NULL); if (!CreatePartition(hPhysicalDrive, pt, fs, (pt==PARTITION_STYLE_MBR)&&(bt==BT_UEFI))) { @@ -1365,9 +1363,11 @@ DWORD WINAPI FormatThread(LPVOID param) } UpdateProgress(OP_PARTITION, -1.0f); - // Add a delay after partitioning to be safe - // TODO: Use a retry loop and test the lock - Sleep(2000); + // Wait for the logical drive we just created to appear + uprintf("Waiting for logical drive to reappear...\n"); + Sleep(200); + WaitForLogical(DriveIndex); // We try to continue even if this fails, just in case + CHECK_FOR_USER_CANCEL; // If FAT32 is requested and we have a large drive (>32 GB) use // large FAT32 format, else use MS's FormatEx. @@ -1379,7 +1379,22 @@ DWORD WINAPI FormatThread(LPVOID param) goto out; } - guid_volume = GetLogicalName(DriveIndex, TRUE); + // Thanks to Microsoft, we must fix the MBR AFTER the drive has been formatted + if (pt == PARTITION_STYLE_MBR) { + PrintStatus(0, TRUE, "Writing master boot record..."); + if (!WriteMBR(hPhysicalDrive)) { + if (!IS_ERROR(FormatStatus)) + FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_WRITE_FAULT; + goto out; + } + UpdateProgress(OP_FIX_MBR, -1.0f); + } + Sleep(200); + WaitForLogical(DriveIndex); + // Try to continue + CHECK_FOR_USER_CANCEL; + + guid_volume = GetLogicalName(DriveIndex, TRUE, TRUE); if (guid_volume == NULL) { uprintf("Could not get GUID volume name\n"); FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_NO_VOLUME_ID; @@ -1387,22 +1402,12 @@ DWORD WINAPI FormatThread(LPVOID param) } uprintf("Found volume GUID %s\n", guid_volume); - if (pt == PARTITION_STYLE_MBR) { - PrintStatus(0, TRUE, "Writing master boot record..."); - if (!WriteMBR(hPhysicalDrive)) { - if (!FormatStatus) - FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_WRITE_FAULT; - goto out; - } - UpdateProgress(OP_FIX_MBR, -1.0f); - } - if (FormatStatus) goto out; // Check for user cancel - if (!MountVolume(drive_name, guid_volume)) { uprintf("Could not remount %s on %s: %s\n", guid_volume, drive_name, WindowsErrorString()); FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|APPERR(ERROR_CANT_MOUNT_VOLUME); goto out; } + CHECK_FOR_USER_CANCEL; if (IsChecked(IDC_BOOT)) { if (bt == BT_UEFI) { @@ -1425,7 +1430,7 @@ DWORD WINAPI FormatThread(LPVOID param) // [0x00000456] The media in the drive may have changed PrintStatus(0, TRUE, "Writing partition boot record..."); if (!WritePBR(hLogicalVolume)) { - if (!FormatStatus) + if (!IS_ERROR(FormatStatus)) FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_WRITE_FAULT; goto out; } @@ -1440,21 +1445,21 @@ DWORD WINAPI FormatThread(LPVOID param) if (IsChecked(IDC_SET_ICON)) SetAutorun(drive_name); } - if (FormatStatus) goto out; // Check for user cancel + CHECK_FOR_USER_CANCEL; // We issue a complete remount of the filesystem at on account of: // - Ensuring the file explorer properly detects that the volume was updated // - Ensuring that an NTFS system will be reparsed so that it becomes bootable if (!RemountVolume(drive_name)) goto out; - if (FormatStatus) goto out; // Check for user cancel + CHECK_FOR_USER_CANCEL; if (IsChecked(IDC_BOOT)) { if ((dt == DT_WINME) || (dt == DT_FREEDOS)) { UpdateProgress(OP_DOS, -1.0f); PrintStatus(0, TRUE, "Copying DOS files..."); if (!ExtractDOS(drive_name)) { - if (!FormatStatus) + if (!IS_ERROR(FormatStatus)) FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_CANNOT_COPY; goto out; } @@ -1464,7 +1469,7 @@ DWORD WINAPI FormatThread(LPVOID param) PrintStatus(0, TRUE, "Copying ISO files..."); drive_name[2] = 0; if (!ExtractISO(iso_path, drive_name, FALSE)) { - if (!FormatStatus) + if (!IS_ERROR(FormatStatus)) FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_CANNOT_COPY; goto out; } diff --git a/src/rufus.h b/src/rufus.h index 004317ad..2ebc08ea 100644 --- a/src/rufus.h +++ b/src/rufus.h @@ -36,6 +36,8 @@ #define STR_NO_LABEL "NO_LABEL" #define RUFUS_CANCELBOX_TITLE APPLICATION_NAME " - Cancellation" #define RUFUS_BLOCKING_IO_TITLE APPLICATION_NAME " - Flushing buffers" +#define DRIVE_ACCESS_TIMEOUT 10000 // How long we should retry drive access (in ms) +#define DRIVE_ACCESS_RETRIES 20 // How many times we should retry #define DRIVE_INDEX_MIN 0x00000080 #define DRIVE_INDEX_MAX 0x000000C0 #define MAX_DRIVES (DRIVE_INDEX_MAX - DRIVE_INDEX_MIN) @@ -60,7 +62,6 @@ #define IsChecked(CheckBox_ID) (IsDlgButtonChecked(hMainDialog, CheckBox_ID) == BST_CHECKED) #define safe_free(p) do {free((void*)p); p = NULL;} while(0) -#define safe_closehandle(h) do {if (h != INVALID_HANDLE_VALUE) {CloseHandle(h); h = INVALID_HANDLE_VALUE;}} while(0) #define safe_min(a, b) min((size_t)(a), (size_t)(b)) #define safe_strcp(dst, dst_max, src, count) do {memcpy(dst, src, safe_min(count, dst_max)); \ ((char*)dst)[safe_min(count, dst_max)-1] = 0;} while(0) @@ -71,8 +72,8 @@ #define safe_stricmp(str1, str2) _stricmp(((str1==NULL)?"":str1), ((str2==NULL)?"":str2)) #define safe_strncmp(str1, str2, count) strncmp(((str1==NULL)?"":str1), ((str2==NULL)?"":str2), count) #define safe_strnicmp(str1, str2, count) _strnicmp(((str1==NULL)?"":str1), ((str2==NULL)?"":str2), count) -#define safe_closehandle(h) do {if (h != INVALID_HANDLE_VALUE) {CloseHandle(h); h = INVALID_HANDLE_VALUE;}} while(0) -#define safe_unlockclose(h) do {if (h != INVALID_HANDLE_VALUE) {UnlockDrive(h); CloseHandle(h); h = INVALID_HANDLE_VALUE;}} while(0) +#define safe_closehandle(h) do {if ((h != INVALID_HANDLE_VALUE) && (h != NULL)) {CloseHandle(h); h = INVALID_HANDLE_VALUE;}} while(0) +#define safe_unlockclose(h) do {if ((h != INVALID_HANDLE_VALUE) && (h != NULL)) {UnlockDrive(h); CloseHandle(h); h = INVALID_HANDLE_VALUE;}} while(0) #define safe_sprintf(dst, count, ...) do {_snprintf(dst, count, __VA_ARGS__); (dst)[(count)-1] = 0; } while(0) #define safe_strlen(str) ((((char*)str)==NULL)?0:strlen(str)) #define safe_strdup _strdup @@ -302,7 +303,8 @@ extern BOOL InstallSyslinux(DWORD drive_index, char drive_letter); DWORD WINAPI FormatThread(void* param); extern char* GetPhysicalName(DWORD DriveIndex); extern HANDLE GetPhysicalHandle(DWORD DriveIndex, BOOL bWriteAccess, BOOL bLockDrive); -extern char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash); +extern BOOL WaitForLogical(DWORD DriveIndex); +extern char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash, BOOL bSilent); extern HANDLE GetLogicalHandle(DWORD DriveIndex, BOOL bWriteAccess, BOOL bLockDrive); extern char GetDriveLetter(DWORD DriveIndex); extern char GetUnusedDriveLetter(void); diff --git a/src/rufus.rc b/src/rufus.rc index c40966f1..453b18e6 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -30,7 +30,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 206, 329 STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | DS_CENTER | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_APPWINDOW -CAPTION "Rufus v1.3.4.258" +CAPTION "Rufus v1.3.4.259" FONT 8, "MS Shell Dlg", 400, 0, 0x1 BEGIN DEFPUSHBUTTON "Start",IDC_START,94,291,50,14 @@ -278,8 +278,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 1,3,4,258 - PRODUCTVERSION 1,3,4,258 + FILEVERSION 1,3,4,259 + PRODUCTVERSION 1,3,4,259 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -296,13 +296,13 @@ BEGIN BEGIN VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "1.3.4.258" + VALUE "FileVersion", "1.3.4.259" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "© 2011-2013 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html" VALUE "OriginalFilename", "rufus.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "1.3.4.258" + VALUE "ProductVersion", "1.3.4.259" END END BLOCK "VarFileInfo"