Add support for drives larger than 2 TB (Closes: #1551)#1552
Add support for drives larger than 2 TB (Closes: #1551)#1552rdmark merged 1 commit intoPiSCSI:developfrom
Conversation
|
Although some Amiga code correctly supports such large drives, it's pretty clear that not all code (HDToolBox) does: |
rdmark
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This looks good at a code review level.
Have you tested building on a 32 bit system? We still want to support f.e. RPi 2 or Zero, and had some challenges with large ints in the past.
| uint64_t disk_blocks = GetBlockCount(); | ||
| if (disk_blocks > 0xFFFFFFFF) | ||
| disk_blocks = 0xFFFFFFFF; | ||
| SetInt32(buf, 4, static_cast<uint32_t>(disk_blocks)); |
There was a problem hiding this comment.
Did you see any side effects from "faking" the block size in the 6 byte ModeSense?
|
I have not tested building on a 32-bit system, but will work to set one up to do so. I'll report back where when that has been tested. As you likely know, Mode Sense (6) supports a 32-bit number for the maximum block count, where Mode Sense (10) supports a 64-bit number for the count. The Seagate SCSI Commands Reference Manual from 2016, has this to say about a "Short LBA mode parameter block descriptor" as returned by Mode Sense (6):
Similarly, the Read Capacity (10) command should return FFFFFFFFh if the capacity of the drive exceeds 2^32 sectors, and that a driver should then know to use Read Capacity (16) if it supports that command. Regarding side effects, yes, I did see a funny one. The Read Capacity commands specify the value returned as the last addressable sector on the device. This means that typically, an application which uses the value returned by that command is going to add 1 to it to get the actual number of blocks on a given device. If the program is not aware of Read Capacity (16), and is not using greater than 32-bit math, then it might report to the user that the total number of sectors on the drive as 0. I don't view this as a huge problem, however. In that case, the legacy driver wouldn't correctly support the capacity of the drive anyway, so providing that driver with a device that is 2 ^ 32 sectors or larger wouldn't make sense from the user perspective. |
|
I put together a 32-bit Pi and have confirmed that PiSCSI with these changes do compile clean under 32-bit. If you would like me to test the 32-bit system, it will take about 30 hours, as the system I use for that work is currently testing another piece of hardware. |
|
This is great, exactly what I wanted to know! Can you please add the Seagate reference manual quote (and related discussion) in the commit message? It really helps to have this in the commit log for future reference. I kicked off a compilation of your PR code on my RPi Zero w. Let's see if it finishes over night. |
Mode Sense (6) supports a 32-bit number for the maximum block count, where Mode Sense (10) supports a 64-bit number for the count. The Seagate SCSI Commands Reference Manual from 2016, has this to say about a "Short LBA mode parameter block descriptor" as returned by Mode Sense (6): > On a MODE SENSE command, if the number of logical blocks on the > medium exceeds the maximum value that is able to be specified in > the NUMBER OF LOGICAL BLOCKS field, the device server shall return > a value of FFFFFFFh. Similarly, the Read Capacity (10) command should return FFFFFFFFh if the capacity of the drive exceeds 2^32 sectors, and that a driver should then know to use Read Capacity (16) if it supports that command. There may be an unexpected side-effect if presenting a drive of more than 2^32 sectors to a legacy host that does not support devices that large. The Read Capacity commands specify the value returned as the last addressable sector on the device. This means that typically, an application which uses the value returned by that command is going to add 1 to it to get the actual number of blocks on a given device. If the program is not aware of Read Capacity (16), and is not using greater than 32-bit math, then it might report to the user that the total number of sectors on the drive as 0. I don't view this as a huge problem, however. In that case, the legacy driver wouldn't correctly support the capacity of the drive anyway, so providing that driver with a device that is 2 ^ 32 sectors or larger wouldn't make sense from the user perspective.
b7136d7 to
562365b
Compare
rdmark
left a comment
There was a problem hiding this comment.
Sanity test on an RPi Zero w was successful. LGTM!
No description provided.