[scsi_cd] ASSERT FAILED: actualLength == fLength

[vidrep]

hrev54195 x86_64

Insert blank CD into optical drive Crash Photo attached

[waddlesplash]

This is a new assert I added in IORequest being triggered. CC korli: I don't know very much about scsi_cd; I will take a look tomorrow evening, but if you have time before then, I think you know more than I do?

[korli]

The problem seems to be in IOCache.cpp. The request length is capped at the device capacity, but the vecs are not adjusted, hence the assert fails.

[korli]

Maybe something like this?

diff --git a/src/system/kernel/device_manager/IOCache.cpp 
b/src/system/kernel/device_manager/IOCache.cpp
index 7cd834b8a5..a7e1660dd6 100644
--- a/src/system/kernel/device_manager/IOCache.cpp
+++ b/src/system/kernel/device_manager/IOCache.cpp
@@ -529,7 +529,8 @@ IOCache::_TransferPages(size_t firstPage, size_t pageCount, bool isWrite,
 
        // prepare the I/O vecs
        size_t vecCount = 0;
-       size_t endPage = firstPage + pageCount;
+       size_t endPage = std::min(firstPage + pageCount,
+               (size_t)((fDeviceCapacity + B_PAGE_SIZE - 1) / B_PAGE_SIZE));
        phys_addr_t vecsEndAddress = 0;
        for (size_t i = firstPage; i < endPage; i++) {
                phys_addr_t pageAddress

[waddlesplash]

Wouldn't it be clearer to use ROUNDDOWN()? Otherwise, looks good.

[waddlesplash]

Actually, on closer inspection, it appears some other functions are already trimming endPage(), so perhaps the ones that are not, should be?

[mark_mcs]

I went a slightly different way with this as I didn't think we could guarantee the storage medium was an exact multiple of B_PAGE_SIZE. I'm not too familiar with the format but aren't ISO 9660 sectors around 2KB (with metadata)?

I've been using this:

diff --git a/src/system/kernel/device_manager/IOCache.cpp b/src/system/kernel/device_manager/IOCache.cpp
index 7cd834b8a5..5ce71436b5 100644
--- a/src/system/kernel/device_manager/IOCache.cpp
+++ b/src/system/kernel/device_manager/IOCache.cpp
@@ -545,6 +545,12 @@ IOCache::_TransferPages(size_t firstPage, size_t pageCount, bool isWrite,
 		}
 	}
 
+	// don't try to read past the end of the device just to fill a page.
+	// this makes sure that sum(fVecs[].length) == requestLength.
+	const generic_size_t lastPageFill = requestLength & (B_PAGE_SIZE-1);
+	if (lastPageFill > 0)
+		fVecs[vecCount-1].length -= B_PAGE_SIZE - lastPageFill;
+
 	// create a request for the transfer
 	IORequest request;
 	status_t error = request.Init(firstPageOffset, fVecs, vecCount,

[korli]

Your way is definitely better. Thanks, I tested, adjusted and pushed for review: ​https://review.haiku-os.org/c/haiku/+/3451

[waddlesplash]

Fix merged in hrev54752.