Fixed bug #7873 : IO error reading file, The parameter is incorrect.

Author: hvlad

src/common/os/os_utils.h

@@ -68,6 +68,8 @@
 #define FLOCK flock
 #endif
 
+const ULONG DEFAULT_SECTOR_SIZE = 4096;
+
 namespace os_utils
 {
 
@@ -88,6 +90,8 @@ namespace os_utils
 	void setCloseOnExec(int fd);	// posix only
 	FILE* fopen(const char* pathname, const char* mode);
 
+	ULONG getPhysicalSectorSize(const Firebird::PathName& fileName);
+
 	// return a binary string that uniquely identifies the file
 #ifdef WIN_NT
 	void getUniqueFileId(HANDLE fd, Firebird::UCharBuffer& id);

src/common/os/posix/os_utils.cpp

@@ -399,6 +399,12 @@ FILE* fopen(const char* pathname, const char* mode)
 	return f;
 }
 
+ULONG getPhysicalSectorSize(const PathName& fileName)
+{
+	// return safe value
+	return DEFAULT_SECTOR_SIZE;
+}
+
 static void makeUniqueFileId(const struct STAT& statistics, UCharBuffer& id)
 {
 	const size_t len1 = sizeof(statistics.st_dev);

src/common/os/win32/os_utils.cpp

@@ -50,6 +50,7 @@
 
 #include <aclapi.h>
 #include <Winsock2.h>
+#include <winioctl.h>
 
 using namespace Firebird;
 
@@ -408,6 +409,47 @@ FILE* fopen(const char* pathname, const char* mode)
 	return ::fopen(pathname, mode);
 }
 
+ULONG getPhysicalSectorSize(const PathName& fileName)
+{
+	// Fallback to the safe value
+	ULONG ret = DEFAULT_SECTOR_SIZE;
+
+	// Device name could be set as \\.\PhysicalDrive0 or as \\.\a:
+	// The second form is used below for simplicity.
+
+	string deviceName = "\\\\.\\";
+	deviceName.append(fileName.c_str(), 2);
+
+	HANDLE hDevice = CreateFile(deviceName.c_str(),
+		0,
+		FILE_SHARE_READ | FILE_SHARE_WRITE,
+		NULL,
+		OPEN_EXISTING,
+		0, NULL);
+
+	if (hDevice != INVALID_HANDLE_VALUE)
+	{
+		STORAGE_PROPERTY_QUERY qry;
+		STORAGE_ACCESS_ALIGNMENT_DESCRIPTOR desc;
+		DWORD readSize = 0;
+
+		qry.PropertyId = StorageAccessAlignmentProperty;
+		qry.QueryType = PropertyStandardQuery;
+
+		if (DeviceIoControl(hDevice, IOCTL_STORAGE_QUERY_PROPERTY,
+			&qry, sizeof(qry),
+			&desc, sizeof(desc),
+			&readSize, NULL))
+		{
+			ret = desc.BytesPerPhysicalSector;
+		}
+
+		CloseHandle(hDevice);
+	}
+
+	return ret;
+}
+
 void getUniqueFileId(HANDLE fd, UCharBuffer& id)
 {
 	entryLoader.init();

src/jrd/jrd.cpp

@@ -7592,11 +7592,21 @@ static JAttachment* create_attachment(const PathName& alias_name,
 
 static void check_single_maintenance(thread_db* tdbb)
 {
-	UCHAR spare_memory[RAW_HEADER_SIZE + PAGE_ALIGNMENT];
-	UCHAR* header_page_buffer = FB_ALIGN(spare_memory, PAGE_ALIGNMENT);
+	Database* const dbb = tdbb->getDatabase();
+
+	const ULONG sectorSize = (dbb->dbb_flags & DBB_no_fs_cache) ?
+		os_utils::getPhysicalSectorSize(dbb->dbb_filename) :
+		PAGE_ALIGNMENT;
+
+	const ULONG headerSize = MAX(RAW_HEADER_SIZE, sectorSize);
+
+	HalfStaticArray<UCHAR, RAW_HEADER_SIZE + PAGE_ALIGNMENT> temp;
+	UCHAR* header_page_buffer = temp.getBuffer(headerSize + sectorSize);
+
+	header_page_buffer = FB_ALIGN(header_page_buffer, PAGE_ALIGNMENT);
 	Ods::header_page* const header_page = reinterpret_cast<Ods::header_page*>(header_page_buffer);
 
-	PIO_header(tdbb, header_page_buffer, RAW_HEADER_SIZE);
+	PIO_header(tdbb, header_page_buffer, headerSize);
 
 	if ((header_page->hdr_flags & Ods::hdr_shutdown_mask) == Ods::hdr_shutdown_single)
 	{

src/jrd/pag.cpp

@@ -1261,10 +1261,17 @@ void PAG_header_init(thread_db* tdbb)
 	// and unit of transfer is a multiple of physical disk
 	// sector for raw disk access.
 
-	UCHAR temp_buffer[RAW_HEADER_SIZE + PAGE_ALIGNMENT];
-	UCHAR* const temp_page = FB_ALIGN(temp_buffer, PAGE_ALIGNMENT);
+	const ULONG sectorSize = (dbb->dbb_flags & DBB_no_fs_cache) ?
+		os_utils::getPhysicalSectorSize(dbb->dbb_filename) :
+		PAGE_ALIGNMENT;
 
-	PIO_header(tdbb, temp_page, RAW_HEADER_SIZE);
+	const ULONG headerSize = MAX(RAW_HEADER_SIZE, sectorSize);
+
+	HalfStaticArray<UCHAR, RAW_HEADER_SIZE + PAGE_ALIGNMENT> temp;
+	UCHAR* temp_buffer = temp.getBuffer(headerSize + sectorSize);
+	UCHAR* const temp_page = FB_ALIGN(temp_buffer, sectorSize);
+
+	PIO_header(tdbb, temp_page, headerSize);
 	const header_page* header = (header_page*) temp_page;
 
 	if (header->hdr_header.pag_type != pag_header || header->hdr_sequence)

src/utilities/gstat/dba.epp

@@ -610,6 +610,10 @@ int gstat(Firebird::UtilSvc* uSvc)
 	expandDatabaseName(fileName, tempStr, NULL);
 	fileName = tempStr;
 
+	// If file will be opened with no buffering (direct IO), make sure that
+	// temp buffer is aligned on the disk physical sector size and IO size
+	// is multiply of sector size.
+
 	dba_fil* current = db_open(fileName.c_str(), fileName.length());
 
 	SCHAR temp[RAW_HEADER_SIZE];

src/utilities/nbackup/nbackup.cpp

@@ -79,12 +79,6 @@
 #define O_LARGEFILE 0
 #endif
 
-// How much we align memory when reading database header.
-// Sector alignment of memory is necessary to use unbuffered IO on Windows.
-// Actually, sectors may be bigger than 1K, but let's be consistent with
-// JRD regarding the matter for the moment.
-const FB_SIZE_T SECTOR_ALIGNMENT = PAGE_ALIGNMENT;
-
 using namespace Firebird;
 
 namespace
@@ -1199,7 +1193,7 @@ void NBackup::backup_database(int level, Guid& guid, const PathName& fname)
 	ULONG prev_scn = 0;
 	char prev_guid[GUID_BUFF_SIZE] = "";
 	char str_guid[GUID_BUFF_SIZE] = "";
-	Ods::pag* page_buff = NULL;
+
 	attach_database();
 	ULONG page_writes = 0, page_reads = 0;
 
@@ -1339,12 +1333,18 @@ void NBackup::backup_database(int level, Guid& guid, const PathName& fname)
 		open_database_scan();
 
 		// Read database header
-		char unaligned_header_buffer[RAW_HEADER_SIZE + SECTOR_ALIGNMENT];
 
-		auto header = reinterpret_cast<Ods::header_page*>(
-			FB_ALIGN(unaligned_header_buffer, SECTOR_ALIGNMENT));
+		const ULONG sectorSize = os_utils::getPhysicalSectorSize(dbname);
+		const ULONG headerSize = MAX(RAW_HEADER_SIZE, sectorSize);
+
+		Array<UCHAR> unaligned_header_buffer;
+		Ods::header_page* header = nullptr;
+		{ // scope
+			UCHAR* buf = unaligned_header_buffer.getBuffer(headerSize + sectorSize);
+			header = reinterpret_cast<Ods::header_page*>(FB_ALIGN(buf, sectorSize));
+		} // end scope
 
-		if (read_file(dbase, header, RAW_HEADER_SIZE) != RAW_HEADER_SIZE)
+		if (read_file(dbase, header, headerSize) != headerSize)
 			status_exception::raise(Arg::Gds(isc_nbackup_err_eofhdrdb) << dbname.c_str() << Arg::Num(1));
 
 		if (!Ods::isSupported(header))
@@ -1361,9 +1361,10 @@ void NBackup::backup_database(int level, Guid& guid, const PathName& fname)
 			status_exception::raise(Arg::Gds(isc_nbackup_db_notlock) << Arg::Num(header->hdr_flags));
 
 		Array<UCHAR> unaligned_page_buffer;
+		Ods::pag* page_buff = nullptr;
 		{ // scope
-			UCHAR* buf = unaligned_page_buffer.getBuffer(header->hdr_page_size + SECTOR_ALIGNMENT);
-			page_buff = reinterpret_cast<Ods::pag*>(FB_ALIGN(buf, SECTOR_ALIGNMENT));
+			UCHAR* buf = unaligned_page_buffer.getBuffer(header->hdr_page_size + sectorSize);
+			page_buff = reinterpret_cast<Ods::pag*>(FB_ALIGN(buf, sectorSize));
 		} // end scope
 
 		ULONG db_size = db_size_pages;
@@ -1430,8 +1431,8 @@ void NBackup::backup_database(int level, Guid& guid, const PathName& fname)
 		Array<UCHAR> unaligned_scns_buffer;
 		Ods::scns_page* scns = NULL, *scns_buf = NULL;
 		{ // scope
-			UCHAR* buf = unaligned_scns_buffer.getBuffer(header->hdr_page_size + SECTOR_ALIGNMENT);
-			scns_buf = reinterpret_cast<Ods::scns_page*>(FB_ALIGN(buf, SECTOR_ALIGNMENT));
+			UCHAR* buf = unaligned_scns_buffer.getBuffer(header->hdr_page_size + sectorSize);
+			scns_buf = reinterpret_cast<Ods::scns_page*>(FB_ALIGN(buf, sectorSize));
 		}
 
 		while (true)